| Submitter | via Mercurial-devel |
|---|---|
| Date | Jan. 12, 2017, 12:10 p.m. |
| Message ID | <M14D1YP5Br61saWo-xh-eCFvhq1OxO7vB00TCQyFu64qQZWQPrXHMpsgh654AbcPxjjPcko_3rulyal-r-CcQHuPrABGE4k5ewlD5fBDw9A=@protonmail.com> |
| Download | mbox | patch |
| Permalink | /patch/18211/ |
| State | Accepted |
| Headers | show |
Comments
Matthieu Laneuville via Mercurial-devel <mercurial-devel@mercurial-scm.org> writes: > # HG changeset patch > # User Matthieu Laneuville <mlaneuville@protonmail.com> > # Date 1484222815 -32400 > # Thu Jan 12 21:06:55 2017 +0900 > # Node ID c0030823eee2d2f320fe4b7e9d8ec90d05485980 > # Parent 493935e0327a2dca0987a702bf06fa89b62e6a5c > templatekw: force noprefix=False to insure diffstat consistency (issue4755) > > The result of diffstatdata should not depend on having noprefix set or not, as > was reported in issue 4755. Forcing noprefix to false on call makes sure the > parser receives the diff in the correct format and returns the proper result. > > Another way to fix this would have been to change the regular expressions in > path.diffstatdata(), but that would have introduced many unecessary special > cases. This reasoning and the test look good to me.
On Thu, Jan 12, 2017 at 4:10 AM, Matthieu Laneuville via Mercurial-devel < mercurial-devel@mercurial-scm.org> wrote: > # HG changeset patch > # User Matthieu Laneuville <mlaneuville@protonmail.com> > # Date 1484222815 -32400 > # Thu Jan 12 21:06:55 2017 +0900 > # Node ID c0030823eee2d2f320fe4b7e9d8ec90d05485980 > # Parent 493935e0327a2dca0987a702bf06fa89b62e6a5c > templatekw: force noprefix=False to insure diffstat consistency (issue4755) > > The result of diffstatdata should not depend on having noprefix set or > not, as > was reported in issue 4755. Forcing noprefix to false on call makes sure > the > parser receives the diff in the correct format and returns the proper > result. > > Another way to fix this would have been to change the regular expressions > in > path.diffstatdata(), but that would have introduced many unecessary special > cases. > For whoever reviews this, previous attempts at fixing this were at: * https://www.mercurial-scm.org/pipermail/mercurial-devel/ 2016-April/083448.html * https://www.mercurial-scm.org/pipermail/mercurial-devel/2015 -December/077214.html (The threads have interesting discussion.) The approach in this patch seems right. I'm wondering why those of us that tried before didn't see this solution. It seems so... obvious. > > diff -r 493935e0327a -r c0030823eee2 mercurial/templatekw.py > --- a/mercurial/templatekw.py Tue Jan 10 06:59:31 2017 +0800 > +++ b/mercurial/templatekw.py Thu Jan 12 21:06:55 2017 +0900 > @@ -299,7 +299,7 @@ > """String. Statistics of changes with the following format: > "modified files: +added/-removed lines" > """ > - stats = patch.diffstatdata(util.iterlines(ctx.diff())) > + stats = patch.diffstatdata(util.iterlines(ctx.diff(noprefix=False))) > maxname, maxtotal, adds, removes, binary = patch.diffstatsum(stats) > return '%s: +%s/-%s' % (len(stats), adds, removes) > > diff -r 493935e0327a -r c0030823eee2 tests/test-import.t > --- a/tests/test-import.t Tue Jan 10 06:59:31 2017 +0800 > +++ b/tests/test-import.t Thu Jan 12 21:06:55 2017 +0900 > @@ -1517,6 +1517,12 @@ > | > o initial [Babar] 2: +8/-0 > > +Adding those config options should not change the output of diffstat. > Bugfix #4755. > + > + $ hg log -r . --template '{diffstat}\n' > + 1: +1/-0 > + $ hg log -r . --template '{diffstat}\n' --config diff.git=1 --config > diff.noprefix=1 > + 1: +1/-0 > > Importing with some success and some errors: > > > > > > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > >
On Fri, 13 Jan 2017 13:05:16 -0800, Gregory Szorc wrote: > On Thu, Jan 12, 2017 at 4:10 AM, Matthieu Laneuville via Mercurial-devel < > mercurial-devel@mercurial-scm.org> wrote: > > > # HG changeset patch > > # User Matthieu Laneuville <mlaneuville@protonmail.com> > > # Date 1484222815 -32400 > > # Thu Jan 12 21:06:55 2017 +0900 > > # Node ID c0030823eee2d2f320fe4b7e9d8ec90d05485980 > > # Parent 493935e0327a2dca0987a702bf06fa89b62e6a5c > > templatekw: force noprefix=False to insure diffstat consistency (issue4755) > > > > The result of diffstatdata should not depend on having noprefix set or > > not, as > > was reported in issue 4755. Forcing noprefix to false on call makes sure > > the > > parser receives the diff in the correct format and returns the proper > > result. > > > > Another way to fix this would have been to change the regular expressions > > in > > path.diffstatdata(), but that would have introduced many unecessary special > > cases. > > > > For whoever reviews this, previous attempts at fixing this were at: > > * https://www.mercurial-scm.org/pipermail/mercurial-devel/ > 2016-April/083448.html > * https://www.mercurial-scm.org/pipermail/mercurial-devel/2015 > -December/077214.html > > (The threads have interesting discussion.) > > The approach in this patch seems right. I'm wondering why those of us that > tried before didn't see this solution. It seems so... obvious. We'd take the tri-state bool path for --no- flag option, e40343ce9c4c, per cost/benefit ratio. > > diff -r 493935e0327a -r c0030823eee2 mercurial/templatekw.py > > --- a/mercurial/templatekw.py Tue Jan 10 06:59:31 2017 +0800 > > +++ b/mercurial/templatekw.py Thu Jan 12 21:06:55 2017 +0900 > > @@ -299,7 +299,7 @@ > > """String. Statistics of changes with the following format: > > "modified files: +added/-removed lines" > > """ > > - stats = patch.diffstatdata(util.iterlines(ctx.diff())) > > + stats = patch.diffstatdata(util.iterlines(ctx.diff(noprefix=False))) > > maxname, maxtotal, adds, removes, binary = patch.diffstatsum(stats) > > return '%s: +%s/-%s' % (len(stats), adds, removes) Matthieu, the patch content appears to be totally corrupted because of your mailer. Can you resend it by using patchbomb or pushgate? https://www.mercurial-scm.org/wiki/ContributingChanges#Emailing_patches Or maybe we can import the last attempt made by Piotr. https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-April/083449.html
> On Jan 12, 2017, at 7:10 AM, Matthieu Laneuville via Mercurial-devel <mercurial-devel@mercurial-scm.org> wrote: > > # HG changeset patch > # User Matthieu Laneuville <mlaneuville@protonmail.com <mailto:mlaneuville@protonmail.com>> > # Date 1484222815 -32400 > # Thu Jan 12 21:06:55 2017 +0900 > # Node ID c0030823eee2d2f320fe4b7e9d8ec90d05485980 > # Parent 493935e0327a2dca0987a702bf06fa89b62e6a5c > templatekw: force noprefix=False to insure diffstat consistency (issue4755) Queued this, thanks. Congratulations on your first Mercurial patch! (It looks like your patch got some whitespace damage in-flight that required manual fixups, which suggests to me you might have pasted this into your mail client, which has an annoying tendency to break things. I recommend one of the methods listed on https://www.mercurial-scm.org/wiki/ContributingChanges for any future patches.)
> On Jan 14, 2017, at 1:28 AM, Yuya Nishihara <yuya@tcha.org> wrote: > >>> diff -r 493935e0327a -r c0030823eee2 mercurial/templatekw.py >>> --- a/mercurial/templatekw.py Tue Jan 10 06:59:31 2017 +0800 >>> +++ b/mercurial/templatekw.py Thu Jan 12 21:06:55 2017 +0900 >>> @@ -299,7 +299,7 @@ >>> """String. Statistics of changes with the following format: >>> "modified files: +added/-removed lines" >>> """ >>> - stats = patch.diffstatdata(util.iterlines(ctx.diff())) >>> + stats = patch.diffstatdata(util.iterlines(ctx.diff(noprefix=False))) >>> maxname, maxtotal, adds, removes, binary = patch.diffstatsum(stats) >>> return '%s: +%s/-%s' % (len(stats), adds, removes) > > Matthieu, the patch content appears to be totally corrupted because of your > mailer. Can you resend it by using patchbomb or pushgate? I managed to get it to apply with only minor surgery. No resend required.
Patch
diff -r 493935e0327a -r c0030823eee2 mercurial/templatekw.py --- a/mercurial/templatekw.py Tue Jan 10 06:59:31 2017 +0800 +++ b/mercurial/templatekw.py Thu Jan 12 21:06:55 2017 +0900 @@ -299,7 +299,7 @@ """String. Statistics of changes with the following format: "modified files: +added/-removed lines" """ - stats = patch.diffstatdata(util.iterlines(ctx.diff())) + stats = patch.diffstatdata(util.iterlines(ctx.diff(noprefix=False))) maxname, maxtotal, adds, removes, binary = patch.diffstatsum(stats) return '%s: +%s/-%s' % (len(stats), adds, removes) diff -r 493935e0327a -r c0030823eee2 tests/test-import.t --- a/tests/test-import.t Tue Jan 10 06:59:31 2017 +0800 +++ b/tests/test-import.t Thu Jan 12 21:06:55 2017 +0900 @@ -1517,6 +1517,12 @@ | o initial [Babar] 2: +8/-0 +Adding those config options should not change the output of diffstat. Bugfix #4755. + + $ hg log -r . --template '{diffstat}\n' + 1: +1/-0 + $ hg log -r . --template '{diffstat}\n' --config diff.git=1 --config diff.noprefix=1 + 1: +1/-0 Importing with some success and some errors: