| Submitter | Matt Mackall |
|---|---|
| Date | July 20, 2015, 8:07 p.m. |
| Message ID | <1437422876.19409.294.camel@selenic.com> |
| Download | mbox | patch |
| Permalink | /patch/10044/ |
| State | Not Applicable |
| Delegated to: | Matt Mackall |
| Headers | show |
Comments
On Mon, Jul 20, 2015 at 1:07 PM, Matt Mackall <mpm@selenic.com> wrote: > 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@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: > Sorry. I found this last week and I considered it stable worthy because it prevents bustage of {diffstat}. I went with the more involved fix because, well, it felt more proper than a hack that would only get refactored post 3.5 anyway. > > 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. > Unfortunately, this patch breaks on filenames with spaces. (This is also a deficiency of the Git patch format.)
On Mon, 2015-07-20 at 14:13 -0700, Gregory Szorc wrote: > On Mon, Jul 20, 2015 at 1:07 PM, Matt Mackall <mpm@selenic.com> wrote: > > > 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@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: > > > > Sorry. I found this last week and I considered it stable worthy because it > prevents bustage of {diffstat}. I'm not saying fixing this issue is not stable-worthy. I'm saying any sort of significant moving of stuff around to make the fix pretty is not how we do things in stable. We only care about user benefit/churn and "pretty" is bad for that equation. > Unfortunately, this patch breaks on filenames with spaces. (This is also a > deficiency of the Git patch format.) Do we not have any of those in the test suite? Because my change passes tests. But also, if we're generating git diffs that aren't parseable by git.. we have a bigger problem.
On 07/21/2015 12:06 PM, Matt Mackall wrote: > On Mon, 2015-07-20 at 14:13 -0700, Gregory Szorc wrote: >> On Mon, Jul 20, 2015 at 1:07 PM, Matt Mackall <mpm@selenic.com> wrote: >> >>> 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@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: >>> >> >> Sorry. I found this last week and I considered it stable worthy because it >> prevents bustage of {diffstat}. > > I'm not saying fixing this issue is not stable-worthy. I'm saying any > sort of significant moving of stuff around to make the fix pretty is not > how we do things in stable. We only care about user benefit/churn and > "pretty" is bad for that equation. > >> Unfortunately, this patch breaks on filenames with spaces. (This is also a >> deficiency of the Git patch format.) > > Do we not have any of those in the test suite? Because my change passes > tests. But also, if we're generating git diffs that aren't parseable by > git.. we have a bigger problem. So, what is the general state of this ? 1) did you had a stable worthy issue? 2) if so was it fixed on stable or should we expect a fix in the next 48h? (urg:-/) 3) What should I mark this series on patchwork?
On Wed, Jul 29, 2015 at 1:14 PM, Pierre-Yves David < pierre-yves.david@ens-lyon.org> wrote: > > > On 07/21/2015 12:06 PM, Matt Mackall wrote: > >> On Mon, 2015-07-20 at 14:13 -0700, Gregory Szorc wrote: >> >>> On Mon, Jul 20, 2015 at 1:07 PM, Matt Mackall <mpm@selenic.com> wrote: >>> >>> 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@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: >>>> >>>> >>> Sorry. I found this last week and I considered it stable worthy because >>> it >>> prevents bustage of {diffstat}. >>> >> >> I'm not saying fixing this issue is not stable-worthy. I'm saying any >> sort of significant moving of stuff around to make the fix pretty is not >> how we do things in stable. We only care about user benefit/churn and >> "pretty" is bad for that equation. >> >> Unfortunately, this patch breaks on filenames with spaces. (This is also >>> a >>> deficiency of the Git patch format.) >>> >> >> Do we not have any of those in the test suite? Because my change passes >> tests. But also, if we're generating git diffs that aren't parseable by >> git.. we have a bigger problem. >> > > So, what is the general state of this ? > > 1) did you had a stable worthy issue? > 2) if so was it fixed on stable or should we expect a fix in the next 48h? > (urg:-/) > 3) What should I mark this series on patchwork? While I think this bug fix is stable worthy, I have little to no inclination to restructure this in the next 48 hours. I'm not exactly sure what qualifies as a stable-appropriate patch and I'd rather not incur the extra work to refactor this before and after 3.5. I'll just send again after the freeze. Although, it sounds like there may be more discussion about the general approach. So I dunno.
On Wed, 2015-07-29 at 20:57 -0700, Gregory Szorc wrote: > On Wed, Jul 29, 2015 at 1:14 PM, Pierre-Yves David < > pierre-yves.david@ens-lyon.org> wrote: > > > > > > > On 07/21/2015 12:06 PM, Matt Mackall wrote: > > > >> On Mon, 2015-07-20 at 14:13 -0700, Gregory Szorc wrote: > >> > >>> On Mon, Jul 20, 2015 at 1:07 PM, Matt Mackall <mpm@selenic.com> wrote: > >>> > >>> 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@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: > >>>> > >>>> > >>> Sorry. I found this last week and I considered it stable worthy because > >>> it > >>> prevents bustage of {diffstat}. > >>> > >> > >> I'm not saying fixing this issue is not stable-worthy. I'm saying any > >> sort of significant moving of stuff around to make the fix pretty is not > >> how we do things in stable. We only care about user benefit/churn and > >> "pretty" is bad for that equation. > >> > >> Unfortunately, this patch breaks on filenames with spaces. (This is also > >>> a > >>> deficiency of the Git patch format.) > >>> > >> > >> Do we not have any of those in the test suite? Because my change passes > >> tests. But also, if we're generating git diffs that aren't parseable by > >> git.. we have a bigger problem. > >> > > > > So, what is the general state of this ? > > > > 1) did you had a stable worthy issue? > > 2) if so was it fixed on stable or should we expect a fix in the next 48h? > > (urg:-/) > > 3) What should I mark this series on patchwork? > > > While I think this bug fix is stable worthy, I have little to no > inclination to restructure this in the next 48 hours. I'm not exactly sure > what qualifies as a stable-appropriate patch and I'd rather not incur the > extra work to refactor this before and after 3.5. I'll just send again > after the freeze. Although, it sounds like there may be more discussion > about the general approach. So I dunno. (adding sid0) We still have the important unanswered question: "If the hack of allowing a/ to be optional in regexes isn't valid because git diffs rely on them to handle filenames with spaces, then don't we actually have to back out the change that allowed --no-prefix with --git?" Or put more directly: "Do hg's --no-prefix --git diffs... actually work with git? If not, THAT is the bug and the --stat issue is irrelevant."
Patch
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"