| 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
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.
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)
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/
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
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.
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 #