Patchwork cmdutil: rename '_deleted' to 'deleted' since it is used

login
register
mail settings
Submitter Martin von Zweigbergk
Date Oct. 3, 2014, 6:50 p.m.
Message ID <27a6af7fdd92f97cd587.1412362229@handduk2.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/6095/
State Changes Requested
Headers show

Comments

Martin von Zweigbergk - Oct. 3, 2014, 6:50 p.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@gmail.com>
# Date 1412362128 25200
#      Fri Oct 03 11:48:48 2014 -0700
# Node ID 27a6af7fdd92f97cd587faae9502c044b292fb83
# Parent  78c916f24dd99a56e4c29153a5df3bd7d1c40edd
cmdutil: rename '_deleted' to 'deleted' since it is used

Underscore seems to be used as a prefix in some places to indicate
that a variable is not used, so avoid using it for a variable that is
used.
Pierre-Yves David - Oct. 3, 2014, 7:48 p.m.
On 10/03/2014 01:50 PM, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@gmail.com>
> # Date 1412362128 25200
> #      Fri Oct 03 11:48:48 2014 -0700
> # Node ID 27a6af7fdd92f97cd587faae9502c044b292fb83
> # Parent  78c916f24dd99a56e4c29153a5df3bd7d1c40edd
> cmdutil: rename '_deleted' to 'deleted' since it is used
>
> Underscore seems to be used as a prefix in some places to indicate
> that a variable is not used, so avoid using it for a variable that is
> used.


No thanks, The variable is only used in the direct vicinity for a quick 
computation. The other have a great destiny much lower in the rest of 
this hug function.
Matt Mackall - Oct. 3, 2014, 8:09 p.m.
On Fri, 2014-10-03 at 14:48 -0500, Pierre-Yves David wrote:
> 
> On 10/03/2014 01:50 PM, Martin von Zweigbergk wrote:
> > # HG changeset patch
> > # User Martin von Zweigbergk <martinvonz@gmail.com>
> > # Date 1412362128 25200
> > #      Fri Oct 03 11:48:48 2014 -0700
> > # Node ID 27a6af7fdd92f97cd587faae9502c044b292fb83
> > # Parent  78c916f24dd99a56e4c29153a5df3bd7d1c40edd
> > cmdutil: rename '_deleted' to 'deleted' since it is used
> >
> > Underscore seems to be used as a prefix in some places to indicate
> > that a variable is not used, so avoid using it for a variable that is
> > used.
> 
> 
> No thanks, The variable is only used in the direct vicinity for a quick 
> computation. The other have a great destiny much lower in the rest of 
> this hug function.

Totally confusing to me until I realized that 'hug' is probably a
reference to the 'hugs' Haskell compiler and that he's manually
implementing static single assignment.

(https://en.wikipedia.org/wiki/Static_single_assignment_form)
Pierre-Yves David - Oct. 3, 2014, 8:10 p.m.
On 10/03/2014 03:09 PM, Matt Mackall wrote:
> On Fri, 2014-10-03 at 14:48 -0500, Pierre-Yves David wrote:
>>
>> On 10/03/2014 01:50 PM, Martin von Zweigbergk wrote:
>>> # HG changeset patch
>>> # User Martin von Zweigbergk <martinvonz@gmail.com>
>>> # Date 1412362128 25200
>>> #      Fri Oct 03 11:48:48 2014 -0700
>>> # Node ID 27a6af7fdd92f97cd587faae9502c044b292fb83
>>> # Parent  78c916f24dd99a56e4c29153a5df3bd7d1c40edd
>>> cmdutil: rename '_deleted' to 'deleted' since it is used
>>>
>>> Underscore seems to be used as a prefix in some places to indicate
>>> that a variable is not used, so avoid using it for a variable that is
>>> used.
>>
>>
>> No thanks, The variable is only used in the direct vicinity for a quick
>> computation. The other have a great destiny much lower in the rest of
>> this hug function.
>
> Totally confusing to me until I realized that 'hug' is probably a
> reference to the 'hugs' Haskell compiler and that he's manually
> implementing static single assignment.
>
> (https://en.wikipedia.org/wiki/Static_single_assignment_form)

s/hug/huge/
Mads Kiilerich - Oct. 6, 2014, 8:21 p.m.
On 10/03/2014 09:48 PM, Pierre-Yves David wrote:
>
>
> On 10/03/2014 01:50 PM, Martin von Zweigbergk wrote:
>> # HG changeset patch
>> # User Martin von Zweigbergk <martinvonz@gmail.com>
>> # Date 1412362128 25200
>> #      Fri Oct 03 11:48:48 2014 -0700
>> # Node ID 27a6af7fdd92f97cd587faae9502c044b292fb83
>> # Parent  78c916f24dd99a56e4c29153a5df3bd7d1c40edd
>> cmdutil: rename '_deleted' to 'deleted' since it is used
>>
>> Underscore seems to be used as a prefix in some places to indicate
>> that a variable is not used, so avoid using it for a variable that is
>> used.
>
>
> No thanks, The variable is only used in the direct vicinity for a 
> quick computation. The other have a great destiny much lower in the 
> rest of this hug function.

What??? That do not make sense to me. I could kind of understand it if 
the _ was added (in the end) to make the name unique - a bit like how we 
use a and a' in math. Or if it was a binding done at module or class 
level and you wanted to keep it "private". But here you use the _ prefix 
is to designate that this great variable is less important than other 
great variables in this function? ERR. If this variable is unimportant 
in an unique and important way then it deserves a name that describes that.

/Mads
Pierre-Yves David - Oct. 6, 2014, 8:25 p.m.
On 10/06/2014 01:21 PM, Mads Kiilerich wrote:
> On 10/03/2014 09:48 PM, Pierre-Yves David wrote:
>>
>>
>> On 10/03/2014 01:50 PM, Martin von Zweigbergk wrote:
>>> # HG changeset patch
>>> # User Martin von Zweigbergk <martinvonz@gmail.com>
>>> # Date 1412362128 25200
>>> #      Fri Oct 03 11:48:48 2014 -0700
>>> # Node ID 27a6af7fdd92f97cd587faae9502c044b292fb83
>>> # Parent  78c916f24dd99a56e4c29153a5df3bd7d1c40edd
>>> cmdutil: rename '_deleted' to 'deleted' since it is used
>>>
>>> Underscore seems to be used as a prefix in some places to indicate
>>> that a variable is not used, so avoid using it for a variable that is
>>> used.
>>
>>
>> No thanks, The variable is only used in the direct vicinity for a
>> quick computation. The other have a great destiny much lower in the
>> rest of this hug function.
>
> What??? That do not make sense to me. I could kind of understand it if
> the _ was added (in the end) to make the name unique - a bit like how we
> use a and a' in math. Or if it was a binding done at module or class
> level and you wanted to keep it "private". But here you use the _ prefix
> is to designate that this great variable is less important than other
> great variables in this function? ERR. If this variable is unimportant
> in an unique and important way then it deserves a name that describes that

Unlike all other variable, this old is never used in the huge table used 
in the middle of the revert function. Go read the function for details.
Mads Kiilerich - Oct. 6, 2014, 8:30 p.m.
On 10/06/2014 10:25 PM, Pierre-Yves David wrote:
>
>
> On 10/06/2014 01:21 PM, Mads Kiilerich wrote:
>> On 10/03/2014 09:48 PM, Pierre-Yves David wrote:
>>>
>>>
>>> On 10/03/2014 01:50 PM, Martin von Zweigbergk wrote:
>>>> # HG changeset patch
>>>> # User Martin von Zweigbergk <martinvonz@gmail.com>
>>>> # Date 1412362128 25200
>>>> #      Fri Oct 03 11:48:48 2014 -0700
>>>> # Node ID 27a6af7fdd92f97cd587faae9502c044b292fb83
>>>> # Parent  78c916f24dd99a56e4c29153a5df3bd7d1c40edd
>>>> cmdutil: rename '_deleted' to 'deleted' since it is used
>>>>
>>>> Underscore seems to be used as a prefix in some places to indicate
>>>> that a variable is not used, so avoid using it for a variable that is
>>>> used.
>>>
>>>
>>> No thanks, The variable is only used in the direct vicinity for a
>>> quick computation. The other have a great destiny much lower in the
>>> rest of this hug function.
>>
>> What??? That do not make sense to me. I could kind of understand it if
>> the _ was added (in the end) to make the name unique - a bit like how we
>> use a and a' in math. Or if it was a binding done at module or class
>> level and you wanted to keep it "private". But here you use the _ prefix
>> is to designate that this great variable is less important than other
>> great variables in this function? ERR. If this variable is unimportant
>> in an unique and important way then it deserves a name that describes 
>> that
>
> Unlike all other variable, this old is never used in the huge table 
> used in the middle of the revert function. Go read the function for 
> details.

If it is unique because it is old then call it something with old.
If it is unique because it is tmp then call it something with tmp.
If it is unique because it is _ then call it something with _.

/Mads

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2520,7 +2520,7 @@ 
         modified = set(changes[0])
         added    = set(changes[1])
         removed  = set(changes[2])
-        _deleted = set(changes[3])
+        deleted  = set(changes[3])
         unknown  = set(changes[4])
         unknown.update(changes[5])
         clean    = set(changes[6])
@@ -2530,8 +2530,8 @@ 
         smf = set(mf)
 
         # determine the exact nature of the deleted changesets
-        deladded = _deleted - smf
-        deleted = _deleted - deladded
+        deladded = deleted - smf
+        deleted = deleted - deladded
 
         # We need to account for the state of file in the dirstate
         #