[PATCH RFC] patch: add index line for diff output

[Pierre-Yves David]

On 12/31/2016 10:45 PM, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean at farley.io>
> # Date 1483220517 21600
> #      Sat Dec 31 15:41:57 2016 -0600
> # Node ID e461e74f54822fef345ffb79bc806e32f03e93fe
> # Parent  bd762c3d68423f7fa9e5e2b97425ed7108e0b8fc
> patch: add index line for diff output

TL;DR: let's make it a config option.

It is usually recommended to include example of output change for such 
patches.

The change seems to from:

 > diff --git a/mercurial/patch.py b/mercurial/patch.py
 > --- a/mercurial/patch.py
 > +++ b/mercurial/patch.py
 > @@ -2499,10 +2499,19 @@ def trydiff(repo, revs, ctx1, ctx2, modi

to

 > diff --git a/mercurial/patch.py b/mercurial/patch.py
 > index 376266343330..333637393631 100644
 > --- a/mercurial/patch.py
 > +++ b/mercurial/patch.py


> This helps highlighting in third-party diff coloring (which assumes git
> output). Is there any reason not to always output this as git does?

Fun fact: we apparently have been doing git style diff wrong for over 
10- years (and yes, the doc seems to confirm the 'index' line is required).

I'm not very enthusiastic at the idea of updating our output to include 
this. Part of that come from the fact we have been -not- including it 
for the past 10 years so updating that output seems a bit bold. In 
addition, Mercurial does not put the idea of "index" or filenode hash 
forward much and from a user point of view I don't see too much value in 
changing that.

So I would rather us to not touch the default output of 'hg diff --git'

> The upside is that third-party tools don't need to change. The downside
> is all the churn in the tests.

Helping third party tool to work with Mercurial is definitely a large 
win. In addition, being able to actually produce valid git diff seems 
"useful".

As we have a diff config section already have a [diff] section with tens 
of config knob there I suggest we add one for this. From reading "git 
diff" help there is a "full" variant of that index line. What about:

   [diff]
   extendedheader.index = [none,short,full]

> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -2499,10 +2499,19 @@ def trydiff(repo, revs, ctx1, ctx2, modi
>              text = mdiff.b85diff(content1, content2)
>              if text:
>                  header.append('index %s..%s' %
>                                (gitindex(content1), gitindex(content2)))
>          else:
> +            if opts.git:
> +                flag = flag1
> +                if flag is None:
> +                    flag = flag2
> +                header.append('index %s..%s %s' %
> +                              (short(gitindex(content1)),
> +                               short(gitindex(content2)),
> +                               gitmode[flag]))
> +
>              text = mdiff.unidiff(content1, date1,
>                                   content2, date2,
>                                   path1, path2, opts=opts)
>          if header and (text or len(header) > 1):
>              yield '\n'.join(header) + '\n'
>
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> index 376266343330..333637393631 100644
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -2499,10 +2499,19 @@ def trydiff(repo, revs, ctx1, ctx2, modi
>              text = mdiff.b85diff(content1, content2)
>              if text:
>                  header.append('index %s..%s' %
>                                (gitindex(content1), gitindex(content2)))
>          else:
> +            if opts.git:
> +                flag = flag1
> +                if flag is None:
> +                    flag = flag2
> +                header.append('index %s..%s %s' %
> +                              (short(gitindex(content1)),
> +                               short(gitindex(content2)),
> +                               gitmode[flag]))
> +
>              text = mdiff.unidiff(content1, date1,
>                                   content2, date2,
>                                   path1, path2, opts=opts)
>          if header and (text or len(header) > 1):
>              yield '\n'.join(header) + '\n'
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

-- 
Pierre-Yves David