Patchwork [7,of,7] templater: abstract truth testing to fix {if(list_of_empty_strings)}

login
register
mail settings
Submitter Yuya Nishihara
Date June 12, 2018, 2:49 p.m.
Message ID <4eb2c19f741f50389c3d.1528814949@mimosa>
Download mbox | patch
Permalink /patch/32083/
State Accepted
Headers show

Comments

Yuya Nishihara - June 12, 2018, 2:49 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1528518887 -32400
#      Sat Jun 09 13:34:47 2018 +0900
# Node ID 4eb2c19f741f50389c3de234654510deede597e2
# Parent  0bf2bc3ec4f89f4a847b68d00011968732aacd7a
templater: abstract truth testing to fix {if(list_of_empty_strings)}

Non-empty list should always be True even if it's stringified to ''. Spotted
by Martin von Zweigbergk.
Augie Fackler - June 12, 2018, 9:24 p.m.
On Tue, Jun 12, 2018 at 11:49:09PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1528518887 -32400
> #      Sat Jun 09 13:34:47 2018 +0900
> # Node ID 4eb2c19f741f50389c3de234654510deede597e2
> # Parent  0bf2bc3ec4f89f4a847b68d00011968732aacd7a
> templater: abstract truth testing to fix {if(list_of_empty_strings)}

queued, thanks
Augie Fackler - June 13, 2018, 9:21 p.m.
> On Jun 12, 2018, at 17:24, Augie Fackler <raf@durin42.com> wrote:
> 
> On Tue, Jun 12, 2018 at 11:49:09PM +0900, Yuya Nishihara wrote:
>> # HG changeset patch
>> # User Yuya Nishihara <yuya@tcha.org>
>> # Date 1528518887 -32400
>> #      Sat Jun 09 13:34:47 2018 +0900
>> # Node ID 4eb2c19f741f50389c3de234654510deede597e2
>> # Parent  0bf2bc3ec4f89f4a847b68d00011968732aacd7a
>> templater: abstract truth testing to fix {if(list_of_empty_strings)}
> 
> queued, thanks

I'm confused: this appears to have regressed test-obsmarker-template.t on Python 3. I'll try and take a look, but if anyone sees anything obvious please let me know.

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - June 14, 2018, 11:15 a.m.
On Wed, 13 Jun 2018 17:21:00 -0400, Augie Fackler wrote:
> 
> 
> > On Jun 12, 2018, at 17:24, Augie Fackler <raf@durin42.com> wrote:
> > 
> > On Tue, Jun 12, 2018 at 11:49:09PM +0900, Yuya Nishihara wrote:
> >> # HG changeset patch
> >> # User Yuya Nishihara <yuya@tcha.org>
> >> # Date 1528518887 -32400
> >> #      Sat Jun 09 13:34:47 2018 +0900
> >> # Node ID 4eb2c19f741f50389c3de234654510deede597e2
> >> # Parent  0bf2bc3ec4f89f4a847b68d00011968732aacd7a
> >> templater: abstract truth testing to fix {if(list_of_empty_strings)}
> > 
> > queued, thanks
> 
> I'm confused: this appears to have regressed test-obsmarker-template.t on Python 3. I'll try and take a look, but if anyone sees anything obvious please let me know.

It's bool(map(...)). I'll send a fix shortly.
Augie Fackler - June 14, 2018, 3:16 p.m.
> On Jun 14, 2018, at 07:15, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> On Wed, 13 Jun 2018 17:21:00 -0400, Augie Fackler wrote:
>> 
>> 
>>> On Jun 12, 2018, at 17:24, Augie Fackler <raf@durin42.com> wrote:
>>> 
>>> On Tue, Jun 12, 2018 at 11:49:09PM +0900, Yuya Nishihara wrote:
>>>> # HG changeset patch
>>>> # User Yuya Nishihara <yuya@tcha.org>
>>>> # Date 1528518887 -32400
>>>> #      Sat Jun 09 13:34:47 2018 +0900
>>>> # Node ID 4eb2c19f741f50389c3de234654510deede597e2
>>>> # Parent  0bf2bc3ec4f89f4a847b68d00011968732aacd7a
>>>> templater: abstract truth testing to fix {if(list_of_empty_strings)}
>>> 
>>> queued, thanks
>> 
>> I'm confused: this appears to have regressed test-obsmarker-template.t on Python 3. I'll try and take a look, but if anyone sees anything obvious please let me know.
> 
> It's bool(map(...)). I'll send a fix shortly.

Sigh, it's going to take getting used to, map() being a generator. Thanks for the fix!

Patch

diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
--- a/mercurial/hgweb/webutil.py
+++ b/mercurial/hgweb/webutil.py
@@ -743,6 +743,9 @@  class sessionvars(templateutil.wrapped):
     def show(self, context, mapping):
         return self.join(context, '')
 
+    def tobool(self, context, mapping):
+        return bool(self._vars)
+
     def tovalue(self, context, mapping):
         return self._vars
 
diff --git a/mercurial/templateutil.py b/mercurial/templateutil.py
--- a/mercurial/templateutil.py
+++ b/mercurial/templateutil.py
@@ -85,6 +85,10 @@  class wrapped(object):
         """
 
     @abc.abstractmethod
+    def tobool(self, context, mapping):
+        """Return a boolean representation of the inner value"""
+
+    @abc.abstractmethod
     def tovalue(self, context, mapping):
         """Move the inner value object out or create a value representation
 
@@ -136,6 +140,9 @@  class wrappedbytes(wrapped):
     def show(self, context, mapping):
         return self._value
 
+    def tobool(self, context, mapping):
+        return bool(self._value)
+
     def tovalue(self, context, mapping):
         return self._value
 
@@ -169,6 +176,14 @@  class wrappedvalue(wrapped):
             return b''
         return pycompat.bytestr(self._value)
 
+    def tobool(self, context, mapping):
+        if self._value is None:
+            return False
+        if isinstance(self._value, bool):
+            return self._value
+        # otherwise evaluate as string, which means 0 is True
+        return bool(pycompat.bytestr(self._value))
+
     def tovalue(self, context, mapping):
         return self._value
 
@@ -201,6 +216,9 @@  class date(mappable, wrapped):
     def tomap(self, context):
         return {'unixtime': self._unixtime, 'tzoffset': self._tzoffset}
 
+    def tobool(self, context, mapping):
+        return True
+
     def tovalue(self, context, mapping):
         return (self._unixtime, self._tzoffset)
 
@@ -272,6 +290,9 @@  class hybrid(wrapped):
             return gen()
         return gen
 
+    def tobool(self, context, mapping):
+        return bool(self._values)
+
     def tovalue(self, context, mapping):
         # TODO: make it non-recursive for trivial lists/dicts
         xs = self._values
@@ -327,6 +348,9 @@  class hybriditem(mappable, wrapped):
             return gen()
         return gen
 
+    def tobool(self, context, mapping):
+        return bool(self.tovalue(context, mapping))
+
     def tovalue(self, context, mapping):
         return _unthunk(context, mapping, self._value)
 
@@ -396,6 +420,9 @@  class mappinggenerator(_mappingsequence)
     def itermaps(self, context):
         return self._make(context, *self._args)
 
+    def tobool(self, context, mapping):
+        return _nonempty(self.itermaps(context))
+
 class mappinglist(_mappingsequence):
     """Wrapper for list of template mappings"""
 
@@ -406,6 +433,9 @@  class mappinglist(_mappingsequence):
     def itermaps(self, context):
         return iter(self._mappings)
 
+    def tobool(self, context, mapping):
+        return bool(self._mappings)
+
 class mappedgenerator(wrapped):
     """Wrapper for generator of strings which acts as a list
 
@@ -449,6 +479,9 @@  class mappedgenerator(wrapped):
     def show(self, context, mapping):
         return self.join(context, mapping, '')
 
+    def tobool(self, context, mapping):
+        return _nonempty(self._gen(context))
+
     def tovalue(self, context, mapping):
         return [stringify(context, mapping, x) for x in self._gen(context)]
 
@@ -607,6 +640,13 @@  def findsymbolicname(arg):
         else:
             return None
 
+def _nonempty(xiter):
+    try:
+        next(xiter)
+        return True
+    except StopIteration:
+        return False
+
 def _unthunk(context, mapping, thing):
     """Evaluate a lazy byte string into value"""
     if not isinstance(thing, types.GeneratorType):
@@ -655,13 +695,7 @@  def evalboolean(context, mapping, arg):
             thing = stringutil.parsebool(data)
     else:
         thing = func(context, mapping, data)
-    if isinstance(thing, wrapped):
-        thing = thing.tovalue(context, mapping)
-    if isinstance(thing, bool):
-        return thing
-    # other objects are evaluated as strings, which means 0 is True, but
-    # empty dict/list should be False as they are expected to be ''
-    return bool(stringify(context, mapping, thing))
+    return makewrapped(context, mapping, thing).tobool(context, mapping)
 
 def evaldate(context, mapping, arg, err=None):
     """Evaluate given argument as a date tuple or a date string; returns
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
@@ -4151,6 +4151,10 @@  Test boolean expression/literal passed t
   empty string is False
   $ hg log -r 0 -T '{if(revset(r"0 - 0"), "", "empty list is False")}\n'
   empty list is False
+  $ hg log -r 0 -T '{if(revset(r"0"), "non-empty list is True")}\n'
+  non-empty list is True
+  $ hg log -r 0 -T '{if(revset(r"0") % "", "list of empty strings is True")}\n'
+  list of empty strings is True
   $ hg log -r 0 -T '{if(true, "true is True")}\n'
   true is True
   $ hg log -r 0 -T '{if(false, "", "false is False")}\n'