Patchwork [6,of,6] templatekw: forward _hybrid.get to raw values so that get(extras, key) works

login
register
mail settings
Submitter Yuya Nishihara
Date March 8, 2015, 11:56 a.m.
Message ID <3627f2ee9b407e144d7a.1425815768@mimosa>
Download mbox | patch
Permalink /patch/7937/
State Accepted
Headers show

Comments

Yuya Nishihara - March 8, 2015, 11:56 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1424269072 -32400
#      Wed Feb 18 23:17:52 2015 +0900
# Node ID 3627f2ee9b407e144d7aad72465ace65714a22a9
# Parent  600b24dca4f75c9692e599febe5a85e435fb79c1
templatekw: forward _hybrid.get to raw values so that get(extras, key) works

ef78450c8df6 implies that the primary goal is to allow "{get(extras, key)}",
but it didn't work.

I'm not sure if _hybrid should forward all unknown attributes to values, so
only "get" is forwarded for now.
Ryan McElroy - March 9, 2015, 7:15 a.m.
On 3/8/2015 4:56 AM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1424269072 -32400
> #      Wed Feb 18 23:17:52 2015 +0900
> # Node ID 3627f2ee9b407e144d7aad72465ace65714a22a9
> # Parent  600b24dca4f75c9692e599febe5a85e435fb79c1
> templatekw: forward _hybrid.get to raw values so that get(extras, key) works
>
> ef78450c8df6 implies that the primary goal is to allow "{get(extras, key)}",
> but it didn't work.
>
> I'm not sure if _hybrid should forward all unknown attributes to values, so
> only "get" is forwarded for now.
>
> diff --git a/mercurial/templatekw.py b/mercurial/templatekw.py
> --- a/mercurial/templatekw.py
> +++ b/mercurial/templatekw.py
> @@ -14,6 +14,7 @@ import hbisect
>   #  "{files % '{file}\n'}" (hgweb-style with inlining and function support)
>   # and to access raw values:
>   #  "{ifcontains(file, files, ...)}", "{ifcontains(key, extras, ...)}"
> +#  "{get(extras, key)}"
>   
>   class _hybrid(object):
>       def __init__(self, gen, values, makemap, joinfmt=None):
> @@ -34,6 +35,10 @@ class _hybrid(object):
>           return x in self.values
>       def __len__(self):
>           return len(self.values)
> +    def __getattr__(self, name):
> +        if name != 'get':
> +            raise AttributeError(name)
Where is this tested? The error below seems to be coming from somewhere 
else?
> +        return getattr(self.values, name)
>   
>   def showlist(name, values, plural=None, element=None, **args):
>       if not element:
> diff --git a/tests/test-command-template.t b/tests/test-command-template.t
> --- a/tests/test-command-template.t
> +++ b/tests/test-command-template.t
> @@ -2286,6 +2286,14 @@ Test branches inside if statement:
>     $ hg log -r 0 --template '{if(branches, "yes", "no")}\n'
>     no
>   
> +Test get function:
> +
> +  $ hg log -r 0 --template '{get(extras, "branch")}\n'
> +  default
> +  $ hg log -r 0 --template '{get(files, "should_fail")}\n'
> +  hg: parse error: get() expects a dict as first argument
> +  [255]
> +
>   Test shortest(node) function:
>   
>     $ echo b > b
I have some qualms about the methods, but the end seems like there are 
some useful results.
Yuya Nishihara - March 9, 2015, 1:29 p.m.
On Mon, 9 Mar 2015 00:15:40 -0700, Ryan McElroy wrote:
> On 3/8/2015 4:56 AM, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1424269072 -32400
> > #      Wed Feb 18 23:17:52 2015 +0900
> > # Node ID 3627f2ee9b407e144d7aad72465ace65714a22a9
> > # Parent  600b24dca4f75c9692e599febe5a85e435fb79c1
> > templatekw: forward _hybrid.get to raw values so that get(extras, key) works
> >
> > ef78450c8df6 implies that the primary goal is to allow "{get(extras, key)}",
> > but it didn't work.
> >
> > I'm not sure if _hybrid should forward all unknown attributes to values, so
> > only "get" is forwarded for now.
> >
> > diff --git a/mercurial/templatekw.py b/mercurial/templatekw.py
> > --- a/mercurial/templatekw.py
> > +++ b/mercurial/templatekw.py
> > @@ -14,6 +14,7 @@ import hbisect
> >   #  "{files % '{file}\n'}" (hgweb-style with inlining and function support)
> >   # and to access raw values:
> >   #  "{ifcontains(file, files, ...)}", "{ifcontains(key, extras, ...)}"
> > +#  "{get(extras, key)}"
> >   
> >   class _hybrid(object):
> >       def __init__(self, gen, values, makemap, joinfmt=None):
> > @@ -34,6 +35,10 @@ class _hybrid(object):
> >           return x in self.values
> >       def __len__(self):
> >           return len(self.values)
> > +    def __getattr__(self, name):
> > +        if name != 'get':
> > +            raise AttributeError(name)
> Where is this tested? The error below seems to be coming from somewhere 
> else?

No test because there is no code that calls _hybrid.update(), keys(), pop(),
etc.

> > +  $ hg log -r 0 --template '{get(files, "should_fail")}\n'
> > +  hg: parse error: get() expects a dict as first argument
> > +  [255]

I wrote this to show that we can't implement _hybrid.get() instead of
__getattr__.

-    def __getattr__(self, name):
-        if name != 'get':
-            raise AttributeError(name)
-        return getattr(self.values, name)
+    def get(self, key):
+        return self.values.get(key)

Regards,
Pierre-Yves David - March 10, 2015, 12:31 a.m.
On 03/08/2015 04:56 AM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1424269072 -32400
> #      Wed Feb 18 23:17:52 2015 +0900
> # Node ID 3627f2ee9b407e144d7aad72465ace65714a22a9
> # Parent  600b24dca4f75c9692e599febe5a85e435fb79c1
> templatekw: forward _hybrid.get to raw values so that get(extras, key) works

I've raised a few eyes brown but this series actually look good to me.

I've pushed it to the clowncopter. Thanks.

Patch

diff --git a/mercurial/templatekw.py b/mercurial/templatekw.py
--- a/mercurial/templatekw.py
+++ b/mercurial/templatekw.py
@@ -14,6 +14,7 @@  import hbisect
 #  "{files % '{file}\n'}" (hgweb-style with inlining and function support)
 # and to access raw values:
 #  "{ifcontains(file, files, ...)}", "{ifcontains(key, extras, ...)}"
+#  "{get(extras, key)}"
 
 class _hybrid(object):
     def __init__(self, gen, values, makemap, joinfmt=None):
@@ -34,6 +35,10 @@  class _hybrid(object):
         return x in self.values
     def __len__(self):
         return len(self.values)
+    def __getattr__(self, name):
+        if name != 'get':
+            raise AttributeError(name)
+        return getattr(self.values, name)
 
 def showlist(name, values, plural=None, element=None, **args):
     if not element:
diff --git a/tests/test-command-template.t b/tests/test-command-template.t
--- a/tests/test-command-template.t
+++ b/tests/test-command-template.t
@@ -2286,6 +2286,14 @@  Test branches inside if statement:
   $ hg log -r 0 --template '{if(branches, "yes", "no")}\n'
   no
 
+Test get function:
+
+  $ hg log -r 0 --template '{get(extras, "branch")}\n'
+  default
+  $ hg log -r 0 --template '{get(files, "should_fail")}\n'
+  hg: parse error: get() expects a dict as first argument
+  [255]
+
 Test shortest(node) function:
 
   $ echo b > b