Patchwork [18,of,19] workingctx: customize _matchstatus for parentworking case

login
register
mail settings
Submitter Sean Farley
Date May 15, 2014, 9:16 p.m.
Message ID <a7bf5d5ab5a5671c5362.1400188596@laptop.local>
Download mbox | patch
Permalink /patch/4777/
State Changes Requested
Headers show

Comments

Sean Farley - May 15, 2014, 9:16 p.m.
# HG changeset patch
# User Sean Farley <sean.michael.farley@gmail.com>
# Date 1398346348 18000
#      Thu Apr 24 08:32:28 2014 -0500
# Node ID a7bf5d5ab5a5671c5362389d52a2e3dd438be49c
# Parent  0770226fad0f8961a755cb516c9e0604c7a4209a
workingctx: customize _matchstatus for parentworking case
Pierre-Yves David - May 16, 2014, 12:24 a.m.
On 05/15/2014 02:16 PM, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1398346348 18000
> #      Thu Apr 24 08:32:28 2014 -0500
> # Node ID a7bf5d5ab5a5671c5362389d52a2e3dd438be49c
> # Parent  0770226fad0f8961a755cb516c9e0604c7a4209a
> workingctx: customize _matchstatus for parentworking case

There is some strange magic going on here. Please elaborate.

>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -1318,10 +1318,26 @@ class workingctx(committablectx):
>               s = super(workingctx, self)._generatestatus(other, s, match,
>                                                           listignored, listclean,
>                                                           listunknown)
>           return s
>
> +    def _matchstatus(self, other, s, match, listignored, listclean,
> +                     listunknown):
> +        """return different match.bad if other isn't parent"""

As said before, the black magic in strong in this area. You want a full 
featured docstring.

Reminder: doc-string works as changeset description

   short oneliner

   longer details
   one multiple line


> +        match = super(workingctx, self)._matchstatus(other, s, match,
> +                                                     listignored, listclean,
> +                                                     listunknown)
> +        if other != self._repo['.']:
> +            def bad(f, msg):
> +                # 'f' may be a directory pattern from 'match.files()',
> +                # so 'f not in ctx1' is not enough
> +                if f not in other and f not in other.dirs():
> +                    self._repo.ui.warn('%s: %s\n' %
> +                                       (self._repo.dirstate.pathto(f), msg))

Drops `self._repo.dirstate.pathto(f)` in a variable so that you do not 
needs the two line for displaying the messages
Sean Farley - May 16, 2014, 1:13 a.m.
Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:

> On 05/15/2014 02:16 PM, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley@gmail.com>
>> # Date 1398346348 18000
>> #      Thu Apr 24 08:32:28 2014 -0500
>> # Node ID a7bf5d5ab5a5671c5362389d52a2e3dd438be49c
>> # Parent  0770226fad0f8961a755cb516c9e0604c7a4209a
>> workingctx: customize _matchstatus for parentworking case
>
> There is some strange magic going on here. Please elaborate.

Yeah, sorry about that. This is only to get rid of having all kinds of
special case if-blocks, specifically:


if not parentworking:
    def bad(f, msg):
        # 'f' may be a directory pattern from 'match.files()',
        # so 'f not in ctx1' is not enough
        if f not in ctx1 and f not in ctx1.dirs():
            self.ui.warn('%s: %s\n' % (self.dirstate.pathto(f), msg))
    match.bad = bad

>>
>> diff --git a/mercurial/context.py b/mercurial/context.py
>> --- a/mercurial/context.py
>> +++ b/mercurial/context.py
>> @@ -1318,10 +1318,26 @@ class workingctx(committablectx):
>>               s = super(workingctx, self)._generatestatus(other, s, match,
>>                                                           listignored, listclean,
>>                                                           listunknown)
>>           return s
>>
>> +    def _matchstatus(self, other, s, match, listignored, listclean,
>> +                     listunknown):
>> +        """return different match.bad if other isn't parent"""
>
> As said before, the black magic in strong in this area. You want a full 
> featured docstring.
>
> Reminder: doc-string works as changeset description
>
>    short oneliner
>
>    longer details
>    one multiple line

I think I got that line from Sid!

>> +        match = super(workingctx, self)._matchstatus(other, s, match,
>> +                                                     listignored, listclean,
>> +                                                     listunknown)
>> +        if other != self._repo['.']:
>> +            def bad(f, msg):
>> +                # 'f' may be a directory pattern from 'match.files()',
>> +                # so 'f not in ctx1' is not enough
>> +                if f not in other and f not in other.dirs():
>> +                    self._repo.ui.warn('%s: %s\n' %
>> +                                       (self._repo.dirstate.pathto(f), msg))
>
> Drops `self._repo.dirstate.pathto(f)` in a variable so that you do not 
> needs the two line for displaying the messages

Ok, sure.
Pierre-Yves David - May 16, 2014, 4:22 a.m.
On 05/15/2014 06:13 PM, Sean Farley wrote:
>
> Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:
>
>> On 05/15/2014 02:16 PM, Sean Farley wrote:
>>> # HG changeset patch
>>> # User Sean Farley <sean.michael.farley@gmail.com>
>>> # Date 1398346348 18000
>>> #      Thu Apr 24 08:32:28 2014 -0500
>>> # Node ID a7bf5d5ab5a5671c5362389d52a2e3dd438be49c
>>> # Parent  0770226fad0f8961a755cb516c9e0604c7a4209a
>>> workingctx: customize _matchstatus for parentworking case
>>
>> There is some strange magic going on here. Please elaborate.
>
> Yeah, sorry about that. This is only to get rid of having all kinds of
> special case if-blocks, specifically:
>
>
> if not parentworking:
>      def bad(f, msg):
>          # 'f' may be a directory pattern from 'match.files()',
>          # so 'f not in ctx1' is not enough
>          if f not in ctx1 and f not in ctx1.dirs():
>              self.ui.warn('%s: %s\n' % (self.dirstate.pathto(f), msg))
>      match.bad = bad

please explicite that the special bad function is meant to handle 
special case with directory pattern (or something more accurate if needed)

>>> diff --git a/mercurial/context.py b/mercurial/context.py
>>> --- a/mercurial/context.py
>>> +++ b/mercurial/context.py
>>> @@ -1318,10 +1318,26 @@ class workingctx(committablectx):
>>>                s = super(workingctx, self)._generatestatus(other, s, match,
>>>                                                            listignored, listclean,
>>>                                                            listunknown)
>>>            return s
>>>
>>> +    def _matchstatus(self, other, s, match, listignored, listclean,
>>> +                     listunknown):
>>> +        """return different match.bad if other isn't parent"""
>>
>> As said before, the black magic in strong in this area. You want a full
>> featured docstring.
>>
>> Reminder: doc-string works as changeset description
>>
>>     short oneliner
>>
>>     longer details
>>     one multiple line
>
> I think I got that line from Sid!

I do not really care about who wrote it. I mostly care about it being fixed.

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -1318,10 +1318,26 @@  class workingctx(committablectx):
             s = super(workingctx, self)._generatestatus(other, s, match,
                                                         listignored, listclean,
                                                         listunknown)
         return s
 
+    def _matchstatus(self, other, s, match, listignored, listclean,
+                     listunknown):
+        """return different match.bad if other isn't parent"""
+        match = super(workingctx, self)._matchstatus(other, s, match,
+                                                     listignored, listclean,
+                                                     listunknown)
+        if other != self._repo['.']:
+            def bad(f, msg):
+                # 'f' may be a directory pattern from 'match.files()',
+                # so 'f not in ctx1' is not enough
+                if f not in other and f not in other.dirs():
+                    self._repo.ui.warn('%s: %s\n' %
+                                       (self._repo.dirstate.pathto(f), msg))
+            match.bad = bad
+        return match
+
     def status(self, ignored=False, clean=False, unknown=False, match=None):
         """Explicit status query
         Unless this method is used to query the working copy status, the
         _status property will implicitly read the status using its default
         arguments."""