| Submitter | Denis Laxalde |
|---|---|
| Date | Oct. 4, 2017, 3:03 p.m. |
| Message ID | <3d8ed7dafee4676b4992.1507129439@sh77.tls.logilab.fr> |
| Download | mbox | patch |
| Permalink | /patch/24498/ |
| State | Accepted |
| Headers | show |
Comments
On Wed, Oct 04, 2017 at 05:03:59PM +0200, Denis Laxalde wrote: > # HG changeset patch > # User Denis Laxalde <denis.laxalde@logilab.fr> > # Date 1506442667 -7200 > # Tue Sep 26 18:17:47 2017 +0200 > # Node ID 3d8ed7dafee4676b49922a3dde0edf1b1fad63ec > # Parent 2fd06499dc8e6a5a784b1334b925c289d7b54e4e > # Available At http://hg.logilab.org/users/dlaxalde/hg > # hg pull http://hg.logilab.org/users/dlaxalde/hg -r 3d8ed7dafee4 > # EXP-Topic followlines-cli > patch: rename "header" variable into "hdr" in diff() I've taken this one, and discarded the rest of the stack per Yuya's review on patch 2. > > The "header" variable was hiding the eponymous class, hence preventing its > usage. > > diff --git a/mercurial/patch.py b/mercurial/patch.py > --- a/mercurial/patch.py > +++ b/mercurial/patch.py > @@ -2316,13 +2316,13 @@ def diff(repo, node1=None, node2=None, m > > copy, if not empty, should contain mappings {dst@y: src@x} of copy > information.''' > - for header, hunks in diffhunks(repo, node1=node1, node2=node2, match=match, > - changes=changes, opts=opts, > - losedatafn=losedatafn, prefix=prefix, > - relroot=relroot, copy=copy): > + for hdr, hunks in diffhunks(repo, node1=node1, node2=node2, match=match, > + changes=changes, opts=opts, > + losedatafn=losedatafn, prefix=prefix, > + relroot=relroot, copy=copy): > text = ''.join(sum((list(hlines) for hrange, hlines in hunks), [])) > - if header and (text or len(header) > 1): > - yield '\n'.join(header) + '\n' > + if hdr and (text or len(hdr) > 1): > + yield '\n'.join(hdr) + '\n' > if text: > yield text > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler a écrit : > On Wed, Oct 04, 2017 at 05:03:59PM +0200, Denis Laxalde wrote: >> # HG changeset patch >> # User Denis Laxalde <denis.laxalde@logilab.fr> >> # Date 1506442667 -7200 >> # Tue Sep 26 18:17:47 2017 +0200 >> # Node ID 3d8ed7dafee4676b49922a3dde0edf1b1fad63ec >> # Parent 2fd06499dc8e6a5a784b1334b925c289d7b54e4e >> # Available At http://hg.logilab.org/users/dlaxalde/hg >> # hg pull http://hg.logilab.org/users/dlaxalde/hg -r 3d8ed7dafee4 >> # EXP-Topic followlines-cli >> patch: rename "header" variable into "hdr" in diff() > > I've taken this one, and discarded the rest of the stack per Yuya's > review on patch 2. Unfortunately, this doesn't help since this patch is no longer in the v2 I sent on Friday: https://patchwork-demo.mercurial-scm.org/project/hg/list/?series=298 Also, it will probably merge-conflict with the other ones. So I'd say you can drop it... >> >> The "header" variable was hiding the eponymous class, hence preventing its >> usage. >> >> diff --git a/mercurial/patch.py b/mercurial/patch.py >> --- a/mercurial/patch.py >> +++ b/mercurial/patch.py >> @@ -2316,13 +2316,13 @@ def diff(repo, node1=None, node2=None, m >> >> copy, if not empty, should contain mappings {dst@y: src@x} of copy >> information.''' >> - for header, hunks in diffhunks(repo, node1=node1, node2=node2, match=match, >> - changes=changes, opts=opts, >> - losedatafn=losedatafn, prefix=prefix, >> - relroot=relroot, copy=copy): >> + for hdr, hunks in diffhunks(repo, node1=node1, node2=node2, match=match, >> + changes=changes, opts=opts, >> + losedatafn=losedatafn, prefix=prefix, >> + relroot=relroot, copy=copy): >> text = ''.join(sum((list(hlines) for hrange, hlines in hunks), [])) >> - if header and (text or len(header) > 1): >> - yield '\n'.join(header) + '\n' >> + if hdr and (text or len(hdr) > 1): >> + yield '\n'.join(hdr) + '\n' >> if text: >> yield text >> >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@mercurial-scm.org >> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Tue, 10 Oct 2017 08:52:36 +0200, Denis Laxalde wrote: > Augie Fackler a écrit : > > On Wed, Oct 04, 2017 at 05:03:59PM +0200, Denis Laxalde wrote: > >> # HG changeset patch > >> # User Denis Laxalde <denis.laxalde@logilab.fr> > >> # Date 1506442667 -7200 > >> # Tue Sep 26 18:17:47 2017 +0200 > >> # Node ID 3d8ed7dafee4676b49922a3dde0edf1b1fad63ec > >> # Parent 2fd06499dc8e6a5a784b1334b925c289d7b54e4e > >> # Available At http://hg.logilab.org/users/dlaxalde/hg > >> # hg pull http://hg.logilab.org/users/dlaxalde/hg -r 3d8ed7dafee4 > >> # EXP-Topic followlines-cli > >> patch: rename "header" variable into "hdr" in diff() > > > > I've taken this one, and discarded the rest of the stack per Yuya's > > review on patch 2. > > Unfortunately, this doesn't help since this patch is no longer in the v2 > I sent on Friday: > > https://patchwork-demo.mercurial-scm.org/project/hg/list/?series=298 > > Also, it will probably merge-conflict with the other ones. So I'd say > you can drop it.. I think the patch itself is good to go since shadowing globals is generally a bad idea. Can you send V3 if they conflict?
Patch
diff --git a/mercurial/patch.py b/mercurial/patch.py --- a/mercurial/patch.py +++ b/mercurial/patch.py @@ -2316,13 +2316,13 @@ def diff(repo, node1=None, node2=None, m copy, if not empty, should contain mappings {dst@y: src@x} of copy information.''' - for header, hunks in diffhunks(repo, node1=node1, node2=node2, match=match, - changes=changes, opts=opts, - losedatafn=losedatafn, prefix=prefix, - relroot=relroot, copy=copy): + for hdr, hunks in diffhunks(repo, node1=node1, node2=node2, match=match, + changes=changes, opts=opts, + losedatafn=losedatafn, prefix=prefix, + relroot=relroot, copy=copy): text = ''.join(sum((list(hlines) for hrange, hlines in hunks), [])) - if header and (text or len(header) > 1): - yield '\n'.join(header) + '\n' + if hdr and (text or len(hdr) > 1): + yield '\n'.join(hdr) + '\n' if text: yield text