[PATCH 5 of 8] mq: make patchheader .plainmode more reliable

Mads Kiilerich mads at kiilerich.com
Wed Sep 24 18:54:52 CDT 2014


On 09/24/2014 03:35 AM, Pierre-Yves David wrote:
>
>
> On 09/23/2014 06:00 PM, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski at unity3d.com>
>> # Date 1411225616 -7200
>> #      Sat Sep 20 17:06:56 2014 +0200
>> # Node ID 39cc9d328c6e61f631ae4f96b5bb0702f3145d39
>> # Parent  889f7da574226e636d689fa4dd425b2b1fbb5ac5
>> mq: make patchheader .plainmode more reliable
>>
>> Instead of having to make extra checks whenever we use .plainmode, 
>> let the
>> initial value consider the actual patch header content.
>>
>> diff --git a/hgext/mq.py b/hgext/mq.py
>> --- a/hgext/mq.py
>> +++ b/hgext/mq.py
>> @@ -202,7 +202,11 @@ class patchheader(object):
>>           self.nodeid = nodeid
>>           self.branch = branch
>>           self.haspatch = diffstart > 1
>> -        self.plainmode = plainmode
>> +        self.plainmode = (plainmode or
>> +                          '# HG changeset patch' not in 
>> self.comments and
>> +                          util.any(c.startswith('Date: ') or
>> +                                   c.startswith('From: ')
>> +                                   for c in self.comments))
>
> 5 lines long "oneliner"‽ Can you use temporary variable to get this 
> readable ?
>

Yes, it is an expression spanning 5 lines. What is the problem?

Plainmode is when the plainmode parameters says so or when it not is a 
HG patch and it has Date or From fields. Exactly what the expression says.

It could of course be written differently and more verbose and more 
inefficient to read and execute.

Would you prefer a more imperative 6-liner that is more verbose in 
explaining "how" but hides the "why"?

         self.plainmode = plainmode
         if not plainmode:
             if '# HG changeset patch' not in self.comments:
                 self.plainmode = util.any(c.startswith('Date: ') or
                                           c.startswith('From: ')
                                           for c in self.comments)

What could be put in temporary variables? Unconditionally looping 
through the comments 3 times would be so obviously inefficient that it 
would be hard to look through that and read what happens.

(/me very much dislikes the pattern we use in some places where we 
introduce a redundant variable just to avoid wrapping a line.)

/Mads
not just being polemic but really not seeing how it could be made more 
readable


More information about the Mercurial-devel mailing list