[PATCH 2 of 3 STABLE] context: ability to manipulate diff feature opt-ins

[Matt Mackall]

On Mon, 2015-07-20 at 20:18 +0900, Yuya Nishihara wrote:
> On Sat, 18 Jul 2015 14:24:30 -0700, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc at gmail.com>
> > # Date 1437254506 25200
> > #      Sat Jul 18 14:21:46 2015 -0700
> > # Node ID b6ab3a470c0196159e8931c8c6dd4b5c4a52a4f6
> > # Parent  d88519184df5f9538b4f9294bee7a595d5dce6d2
> > context: ability to manipulate diff feature opt-ins
> > 
> > Before this patch, basectx.diff respected all diff options. Sometimes
> > consumers want to opt out of certain classes of diff features.
> > 
> > Change the function to call patch.difffeatureopts instead of
> > diffallopts and allow the various features to be opted out of via
> > arguments.
> > 
> > diff --git a/mercurial/context.py b/mercurial/context.py
> > --- a/mercurial/context.py
> > +++ b/mercurial/context.py
> > @@ -268,15 +268,23 @@ class basectx(object):
> >                                include, exclude, default,
> >                                auditor=r.auditor, ctx=self,
> >                                listsubrepos=listsubrepos, badfn=badfn)
> >  
> > -    def diff(self, ctx2=None, match=None, **opts):
> > -        """Returns a diff generator for the given contexts and matcher"""
> > +    def diff(self, ctx2=None, match=None, allowgit=True, allowwhitespace=True,
> > +             allowformatchanging=True, **opts):
> > +        """Returns a diff generator for the given contexts and matcher.
> > +
> > +        The various allow* arguments control whether the diff features under
> > +        that category are respected. See patch.difffeatureopts.
> > +        """
> >          if ctx2 is None:
> >              ctx2 = self.p1()
> >          if ctx2 is not None:
> >              ctx2 = self._repo[ctx2]
> > -        diffopts = patch.diffopts(self._repo.ui, opts)
> > +        diffopts = patch.difffeatureopts(self._repo.ui, opts=opts,
> > +                                         git=allowgit,
> > +                                         whitespace=allowwhitespace,
> > +                                         formatchanging=allowformatchanging)
> >          return patch.diff(self._repo, ctx2, self, match=match, opts=diffopts)
> 
> I'm noob about this API, but I think patch.diff*opts() exists to pack various
> diff-related options into one argument. It seems this change goes the opposite
> direction.

Yep. Also, it bears repeating: I really don't want _patch series_ on
stable. The explicit goal of stable is to maximize benefit/churn. If you
can't fix something in stable in one simple patch, it may be too much
churn. I don't see any reason why our diffstat code should be picky
about prefixes, so I think we should instead teach it that prefixes are
optional:

diff -r f8aead51aec0 mercurial/patch.py
--- a/mercurial/patch.py	Sun Jul 19 18:11:18 2015 +0200
+++ b/mercurial/patch.py	Mon Jul 20 15:02:19 2015 -0500
@@ -15,7 +15,7 @@
 import base85, mdiff, scmutil, util, diffhelpers, copies, encoding, error
 import pathutil
 
-gitre = re.compile('diff --git a/(.*) b/(.*)')
+gitre = re.compile('diff --git (?:a/)(.*) (?:b/)(.*)')
 tabsplitter = re.compile(r'(\t+|[^\t]+)')
 
 class PatchError(Exception):
@@ -324,7 +324,7 @@
     gitpatches = []
     for line in lr:
         line = line.rstrip(' \r\n')
-        if line.startswith('diff --git a/'):
+        if line.startswith('diff --git '):
             m = gitre.match(line)
             if m:
                 if gp:
@@ -814,7 +814,7 @@
 class header(object):
     """patch header
     """
-    diffgit_re = re.compile('diff --git a/(.*) b/(.*)$')
+    diffgit_re = re.compile('diff --git (?:a/)(.*) (?:b/)(.*)$')
     diff_re = re.compile('diff -r .* (.*)$')
     allhunks_re = re.compile('(?:index|deleted file) ')
     pretty_re = re.compile('(?:new file|deleted file) ')
@@ -1752,7 +1752,7 @@
                 emitfile = False
                 yield 'file', (afile, bfile, h, gp and gp.copy() or None)
             yield 'hunk', h
-        elif x.startswith('diff --git a/'):
+        elif x.startswith('diff --git '):
             m = gitre.match(x.rstrip(' \r\n'))
             if not m:
                 continue
@@ -2476,7 +2476,7 @@
             addresult()
             # set numbers to 0 anyway when starting new file
             adds, removes, isbinary = 0, 0, False
-            if line.startswith('diff --git a/'):
+            if line.startswith('diff --git '):
                 filename = gitre.search(line).group(2)
             elif line.startswith('diff -r'):
                 # format: "diff -r ... -r ... filename"

This patch may go a little too far since I just did a search and
replace, but we should probably also teach the patcher how to apply
prefixless patches.

-- 
Mathematics is the supreme nostalgia of our time.