Patchwork [02,of,12] localrepo: add a helper method for creating a label

login
register
mail settings
Submitter Sean Farley
Date Aug. 18, 2014, 9:17 p.m.
Message ID <997cc95ccdd7d74898e2.1408396678@1.0.0.127.in-addr.arpa>
Download mbox | patch
Permalink /patch/5460/
State Changes Requested
Headers show

Comments

Sean Farley - Aug. 18, 2014, 9:17 p.m.
# HG changeset patch
# User Sean Farley <sean.michael.farley@gmail.com>
# Date 1396217998 18000
#      Sun Mar 30 17:19:58 2014 -0500
# Node ID 997cc95ccdd7d74898e2aa52d4cadb1ad24ad0ef
# Parent  ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
localrepo: add a helper method for creating a label
Augie Fackler - Aug. 19, 2014, 2:01 p.m.
On Mon, Aug 18, 2014 at 04:17:58PM -0500, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1396217998 18000
> #      Sun Mar 30 17:19:58 2014 -0500
> # Node ID 997cc95ccdd7d74898e2aa52d4cadb1ad24ad0ef
> # Parent  ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
> localrepo: add a helper method for creating a label
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -700,10 +700,23 @@ class localrepository(object):
>          for bookmark, n in self._bookmarks.iteritems():
>              if n == node:
>                  marks.append(bookmark)
>          return sorted(marks)
>
> +    def _createlabelnamespace(self, namespace):

Probably worth a comment that collections.defaultdict could obsolete
this once we drop 2.4.

(I don't think it's worth building a generalized defaultdict polyfill
for this one case, but I do want to believe that EOLing 2.4 is
coming.)

> +        '''Function to create a dictionary entry for namespace. Exists to avoid code
> +        duplication.
> +
> +        Return True if we created a new namespace entry in self._labels,
> +        otherwise return False.
> +        '''
> +        if namespace not in self._labels:
> +            # ensure a blank dictionary exists
> +            self._labels[namespace] = {}
> +            return True
> +        return False
> +
>      def branchmap(self):
>          '''returns a dictionary {branch: [branchheads]} with branchheads
>          ordered by increasing revision number'''
>          branchmap.updatecache(self)
>          return self._branchcaches[self.filtername]
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Sean Farley - Aug. 19, 2014, 2:54 p.m.
Augie Fackler writes:

> On Mon, Aug 18, 2014 at 04:17:58PM -0500, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley@gmail.com>
>> # Date 1396217998 18000
>> #      Sun Mar 30 17:19:58 2014 -0500
>> # Node ID 997cc95ccdd7d74898e2aa52d4cadb1ad24ad0ef
>> # Parent  ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
>> localrepo: add a helper method for creating a label
>>
>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> --- a/mercurial/localrepo.py
>> +++ b/mercurial/localrepo.py
>> @@ -700,10 +700,23 @@ class localrepository(object):
>>          for bookmark, n in self._bookmarks.iteritems():
>>              if n == node:
>>                  marks.append(bookmark)
>>          return sorted(marks)
>>
>> +    def _createlabelnamespace(self, namespace):
>
> Probably worth a comment that collections.defaultdict could obsolete
> this once we drop 2.4.
>
> (I don't think it's worth building a generalized defaultdict polyfill
> for this one case, but I do want to believe that EOLing 2.4 is
> coming.)

Sure, but in an earlier iteration I used this function to create a
templatekw for each namespace. I dropped it for this series since I
imagine there will be some major bikeshedding.
Pierre-Yves David - Aug. 21, 2014, 7:48 a.m.
On 08/18/2014 02:17 PM, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1396217998 18000
> #      Sun Mar 30 17:19:58 2014 -0500
> # Node ID 997cc95ccdd7d74898e2aa52d4cadb1ad24ad0ef
> # Parent  ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
> localrepo: add a helper method for creating a label
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -700,10 +700,23 @@ class localrepository(object):
>           for bookmark, n in self._bookmarks.iteritems():
>               if n == node:
>                   marks.append(bookmark)
>           return sorted(marks)
>
> +    def _createlabelnamespace(self, namespace):
> +        '''Function to create a dictionary entry for namespace. Exists to avoid code
> +        duplication.
> +
> +        Return True if we created a new namespace entry in self._labels,
> +        otherwise return False.
> +        '''
> +        if namespace not in self._labels:
> +            # ensure a blank dictionary exists
> +            self._labels[namespace] = {}
> +            return True
> +        return False
> +

I do not quite get why you need an explicit function for that. And I did 
not found place where the return value was used (but I might have missed it.

Are you aware of dict.setdefault('key', {}) ?
Sean Farley - Aug. 21, 2014, 6:05 p.m.
Pierre-Yves David writes:

> On 08/18/2014 02:17 PM, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley@gmail.com>
>> # Date 1396217998 18000
>> #      Sun Mar 30 17:19:58 2014 -0500
>> # Node ID 997cc95ccdd7d74898e2aa52d4cadb1ad24ad0ef
>> # Parent  ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
>> localrepo: add a helper method for creating a label
>>
>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> --- a/mercurial/localrepo.py
>> +++ b/mercurial/localrepo.py
>> @@ -700,10 +700,23 @@ class localrepository(object):
>>           for bookmark, n in self._bookmarks.iteritems():
>>               if n == node:
>>                   marks.append(bookmark)
>>           return sorted(marks)
>>
>> +    def _createlabelnamespace(self, namespace):
>> +        '''Function to create a dictionary entry for namespace. Exists to avoid code
>> +        duplication.
>> +
>> +        Return True if we created a new namespace entry in self._labels,
>> +        otherwise return False.
>> +        '''
>> +        if namespace not in self._labels:
>> +            # ensure a blank dictionary exists
>> +            self._labels[namespace] = {}
>> +            return True
>> +        return False
>> +
>
> I do not quite get why you need an explicit function for that. And I did 
> not found place where the return value was used (but I might have missed it.

I don't use the return value yet.

> Are you aware of dict.setdefault('key', {}) ?

I guess I could use that for now, and if we decide to, say, create a
template keyword later, we could refactor it easily enough.

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -700,10 +700,23 @@  class localrepository(object):
         for bookmark, n in self._bookmarks.iteritems():
             if n == node:
                 marks.append(bookmark)
         return sorted(marks)
 
+    def _createlabelnamespace(self, namespace):
+        '''Function to create a dictionary entry for namespace. Exists to avoid code
+        duplication.
+
+        Return True if we created a new namespace entry in self._labels,
+        otherwise return False.
+        '''
+        if namespace not in self._labels:
+            # ensure a blank dictionary exists
+            self._labels[namespace] = {}
+            return True
+        return False
+
     def branchmap(self):
         '''returns a dictionary {branch: [branchheads]} with branchheads
         ordered by increasing revision number'''
         branchmap.updatecache(self)
         return self._branchcaches[self.filtername]