[PATCH 5 of 5] cleanup: replace False identity testing with an explicit token object

Pierre-Yves David pierre-yves.david at ens-lyon.org
Wed May 11 08:38:02 EDT 2016



On 05/11/2016 02:34 PM, Augie Fackler wrote:
>> On May 11, 2016, at 3:38 AM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>>
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
>> # Date 1462469571 -7200
>> #      Thu May 05 19:32:51 2016 +0200
>> # Node ID b62cf5924c3b5684cb0693fe4186dc607dcc5b50
>> # Parent  685102426f222a6746960a905e2392cc2c4607da
>> # EXP-Topic cleanup
>> cleanup: replace False identity testing with an explicit token object
>>
>> The recommended way to check default value (when None is not as option) is a
>> token object. Identity testing to integer is less explicit and not guaranteed to
>> work in all implementations.
>>
>> diff -r 685102426f22 -r b62cf5924c3b mercurial/dirstate.py
>> --- a/mercurial/dirstate.py	Wed May 11 09:31:47 2016 +0200
>> +++ b/mercurial/dirstate.py	Thu May 05 19:32:51 2016 +0200
>> @@ -74,6 +74,8 @@ def _trypending(root, vfs, filename):
>>                  raise
>>      return (vfs(filename), False)
>>
>> +_token = object()
>> +
>> class dirstate(object):
>>
>>      def __init__(self, opener, ui, root, validate):
>> @@ -688,12 +690,12 @@ class dirstate(object):
>>          self._pl = (parent, nullid)
>>          self._dirty = True
>>
>> -    def write(self, tr=False):
>> +    def write(self, tr=_token):
>>          if not self._dirty:
>>              return
>>
>>          filename = self._filename
>> -        if tr is False: # not explicitly specified
>> +        if tr is _token: # not explicitly specified
> I just saw some code (seriously, I tripped over it about at the same time as I saw this patch) in localrepo.py that explicitly calls dirstate.write(None). Is that a bug? It looks like a bug. I suspect this might want to just look for a truthy value to sniff out clients that are doing the wrong thing?
>
> (if not, we should maybe have some documentation that states why internal users are allowed to skip the transaction here)

Dirstate change -must- collaborate with the transaction if there is one. 
However, if there is no currently open transaction, it wont.

-- 
Pierre-Yves


More information about the Mercurial-devel mailing list