mdiff: prepare mdiff to be used for run-tests to replace unidiff
Needs ReviewPublic

Authored by sangeet259 on Fri, May 10, 1:28 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

Patch 1/2

This patch adds some functions to mdiff.py that will be called from run-tests.py
which will be added in the second part of this patch series.

The future changes to run-tests.py will enable one to use mdiif
to diff rather than unidiff whenever the mercurial is installed in the system.

hg diff uses mdiff which which gives simpler and more readable diffs than unidiff

Why do I need to split the patches into two ?
The reason to split what would have been a single patch into two is because
for the next patch to be able to use the mdiff during tests,
this revision has to be there in the system's mercurial installation.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped
sangeet259 created this revision.Fri, May 10, 1:28 PM

The future changes to run-tests.py will enable one to use mdiif
to diff rather than unidiff whenever the mercurial is installed in the system.

Perhaps a naive question, but why is that desirable? How will I (as someone running tests) notice?

The future changes to run-tests.py will enable one to use mdiif
to diff rather than unidiff whenever the mercurial is installed in the system.

Perhaps a naive question, but why is that desirable? How will I (as someone running tests) notice?

@martinvonz
This patch is a breakdown of the earlier patch https://phab.mercurial-scm.org/D5514

There I have detailed why is this required.
In short, the diff output of tests is a bit ugly compared to what hg diff does, which is much nicer.

I am thinking of putting that descriptive commit message in the patch 2/2.

The future changes to run-tests.py will enable one to use mdiif
to diff rather than unidiff whenever the mercurial is installed in the system.

Perhaps a naive question, but why is that desirable? How will I (as someone running tests) notice?

@martinvonz
This patch is a breakdown of the earlier patch https://phab.mercurial-scm.org/D5514

Oh, nice! I sometimes use run-tests.py -i and just accept an unreadable diff because I know that hg diff might make it easier to read. I had not understood why :)

There I have detailed why is this required.
In short, the diff output of tests is a bit ugly compared to what hg diff does, which is much nicer.

I am thinking of putting that descriptive commit message in the patch 2/2.

Perhaps also adding a "which gives simpler diffs" or something like that to this patch?

sangeet259 edited the summary of this revision. (Show Details)Fri, May 10, 2:15 PM

Oh, nice! I sometimes use run-tests.py -i and just accept an unreadable diff because I know that hg diff might make it easier to read. I had not understood why :)

Glad to know you will be helped :)

Added some description to the commit message!

pulkit added a subscriber: pulkit.Tue, May 14, 1:07 PM

Why do I need to split the patches into two ?
The reason to split what would have been a single patch into two is because
for the next patch to be able to use the mdiff during tests,
this revision has to be there in the system's mercurial installation.

I didn't get the last line. Can you explain more?

I think both can be folded in one and should be good.

In D6356#92644, @pulkit wrote:

Why do I need to split the patches into two ?
The reason to split what would have been a single patch into two is because
for the next patch to be able to use the mdiff during tests,
this revision has to be there in the system's mercurial installation.

I didn't get the last line. Can you explain more?

I think both can be folded in one and should be good.

@pulkit
Well the next patch imports mercurial and then calls mdff.new_diff . If I combine those two patches then you can not see the change as while testing D6359 , if there mdiff doesn't already have new_diff
it will always fall back to unified_diff as happened with @durin42 , while he was trying to see the alternate path as he trying to test D5514 .

What I am trying to do here is, once D6356 is has added new_diff to mdiff, you can then have this revision installed in your system or wherever one tests this, and then check D6359 to see the alternate path it follows.

Patch 1/2

we don't add that to commit messages.

mercurial/mdiff.py
531

may be worth to rename it to prepare_mdiff_input.

540

We can do

date = datetime.datetime.now().strftime("%a %b %d %y %H:%M:%S %Y %z")
return "".join(expected), date, "".join(output), date, diffopts(noprefix=True)
544

I was unable to find any getdiff in this file.

552

nit: return itertools.chain.from_iterable(hunklines)

557

backwards compatibility with what?

562

nit: return process_mdiff(uheaders, hunks)

In D6356#92644, @pulkit wrote:

Why do I need to split the patches into two ?
The reason to split what would have been a single patch into two is because
for the next patch to be able to use the mdiff during tests,
this revision has to be there in the system's mercurial installation.

I didn't get the last line. Can you explain more?

I think both can be folded in one and should be good.

@pulkit
Well the next patch imports mercurial and then calls mdff.new_diff . If I combine those two patches then you can not see the change as while testing D6359 , if there mdiff doesn't already have new_diff
it will always fall back to unified_diff as happened with @durin42 , while he was trying to see the alternate path as he trying to test D5514 .

What I am trying to do here is, once D6356 is has added new_diff to mdiff, you can then have this revision installed in your system or wherever one tests this, and then check D6359 to see the alternate path it follows.

I was trying to test it and unable to get this to work both with a single patch and two splitted patches.

My understanding is that the 1st patch should be present in the mercurial installed in the system, which is what you said right? If that's correct, we can go ahead and have this whole as one patch as I don't think people usually do install system hg on patch to patch basis.