Patchwork [1,of,4] patch: rename "header" variable into "hdr" in diff()

login
register
mail settings
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

Denis Laxalde - Oct. 4, 2017, 3:03 p.m.
# 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()

The "header" variable was hiding the eponymous class, hence preventing its
usage.
Augie Fackler - Oct. 10, 2017, 1:31 a.m.
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
Denis Laxalde - Oct. 10, 2017, 6:52 a.m.
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
Yuya Nishihara - Oct. 10, 2017, 1:18 p.m.
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