Patchwork [RFC] templater: provide ring operations on integers

login
register
mail settings
Submitter Simon Farnsworth
Date Oct. 8, 2016, 4:24 p.m.
Message ID <e89699ba5c9f0bf883bf.1475943886@devvm022.lla2.facebook.com>
Download mbox | patch
Permalink /patch/16950/
State Superseded
Headers show

Comments

Simon Farnsworth - Oct. 8, 2016, 4:24 p.m.
# HG changeset patch
# User Simon Farnsworth <simonfar@fb.com>
# Date 1475943860 25200
#      Sat Oct 08 09:24:20 2016 -0700
# Node ID e89699ba5c9f0bf883bfae7c485e50219b90b2f9
# Parent  91a3c58ecf938ed675f5364b88f0d663f12b0047
templater: provide ring operations on integers

The termwidth template keyword is of limited use without some way to ensure
that margins are respected. Provide the three core ring operations -
addition, subtraction and multiplication. We can extend to division and
modulus later if necessary.
Simon Farnsworth - Oct. 8, 2016, 4:26 p.m.
This is RFC because I have no idea what I'm doing in the parser. If 
someone at the sprint has 5 minutes available to educate me, I can 
update this to a better version.

On 08/10/2016 18:24, Simon Farnsworth wrote:
> # HG changeset patch
> # User Simon Farnsworth <simonfar@fb.com>
> # Date 1475943860 25200
> #      Sat Oct 08 09:24:20 2016 -0700
> # Node ID e89699ba5c9f0bf883bfae7c485e50219b90b2f9
> # Parent  91a3c58ecf938ed675f5364b88f0d663f12b0047
> templater: provide ring operations on integers
>
> The termwidth template keyword is of limited use without some way to ensure
> that margins are respected. Provide the three core ring operations -
> addition, subtraction and multiplication. We can extend to division and
> modulus later if necessary.
>
> diff --git a/mercurial/templater.py b/mercurial/templater.py
> --- a/mercurial/templater.py
> +++ b/mercurial/templater.py
> @@ -33,6 +33,9 @@
>      "|": (5, None, None, ("|", 5), None),
>      "%": (6, None, None, ("%", 6), None),
>      ")": (0, None, None, None, None),
> +    "+": (10, None, None, ("+", 10), None),
> +    "-": (10, None, ("negate", 10), ("-", 10), None),
> +    "*": (12, None, None, ("*", 12), None),
>      "integer": (0, "integer", None, None, None),
>      "symbol": (0, "symbol", None, None, None),
>      "string": (0, "string", None, None, None),
> @@ -48,7 +51,7 @@
>          c = program[pos]
>          if c.isspace(): # skip inter-token whitespace
>              pass
> -        elif c in "(,)%|": # handle simple operators
> +        elif c in "(,)%|+-*": # handle simple operators
>              yield (c, None, pos)
>          elif c in '"\'': # handle quoted templates
>              s = pos + 1
> @@ -70,7 +73,7 @@
>                  pos += 1
>              else:
>                  raise error.ParseError(_("unterminated string"), s)
> -        elif c.isdigit() or c == '-':
> +        elif c.isdigit():
>              s = pos
>              if c == '-': # simply take negate operator as part of integer
>                  pos += 1
> @@ -420,6 +423,29 @@
>              # If so, return the expanded value.
>              yield i
>
> +def buildnegate(exp, context):
> +    arg = compileexp(exp[1], context, exprmethods)
> +    return (runnegate, arg)
> +
> +def runnegate(context, mapping, data):
> +    data = evalinteger(context, mapping, data,
> +                        _('negation needs an integer argument'))
> +    return -data
> +
> +
> +def buildarithmetic(exp, context, func):
> +    left = compileexp(exp[1], context, exprmethods)
> +    right = compileexp(exp[2], context, exprmethods)
> +    return (runarithmetic, (func, left, right))
> +
> +def runarithmetic(context, mapping, data):
> +    func, left, right = data
> +    left = evalinteger(context, mapping, left,
> +                        _('arithmetic only defined on integers'))
> +    right = evalinteger(context, mapping, right,
> +                        _('arithmetic only defined on integers'))
> +    return func(left, right)
> +
>  def buildfunc(exp, context):
>      n = getsymbol(exp[1])
>      args = [compileexp(x, context, exprmethods) for x in getlist(exp[2])]
> @@ -906,6 +932,7 @@
>  # methods to interpret function arguments or inner expressions (e.g. {_(x)})
>  exprmethods = {
>      "integer": lambda e, c: (runinteger, e[1]),
> +    "negate": lambda e, c: (runinteger, e[1]),
>      "string": lambda e, c: (runstring, e[1]),
>      "symbol": lambda e, c: (runsymbol, e[1]),
>      "template": buildtemplate,
> @@ -914,6 +941,10 @@
>      "|": buildfilter,
>      "%": buildmap,
>      "func": buildfunc,
> +    "+": lambda e, c: buildarithmetic(e, c, lambda a, b: a + b),
> +    "-": lambda e, c: buildarithmetic(e, c, lambda a, b: a - b),
> +    "negate": buildnegate,
> +    "*": lambda e, c: buildarithmetic(e, c, lambda a, b: a * b),
>      }
>
>  # methods to interpret top-level template (e.g. {x}, {x|_}, {x % "y"})
> 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
> @@ -5,6 +5,8 @@
>    $ echo line 1 > b
>    $ echo line 2 >> b
>    $ hg commit -l b -d '1000000 0' -u 'User Name <user@hostname>'
> +  $ hg log -T '{date(date, "%s") + 1} {date(date, "%s") - 2 * 1}\n'
> +  1000001 999998
>
>    $ hg add b
>    $ echo other 1 > c
> @@ -2890,14 +2892,15 @@
>    $ hg debugtemplate -v '{(-4)}\n'
>    (template
>      (group
> -      ('integer', '-4'))
> +      (negate
> +        ('integer', '4')))
>      ('string', '\n'))
>    -4
>    $ hg debugtemplate '{(-)}\n'
> -  hg: parse error at 2: integer literal without digits
> +  hg: parse error at 3: not a prefix: )
>    [255]
>    $ hg debugtemplate '{(-a)}\n'
> -  hg: parse error at 2: integer literal without digits
> +  hg: parse error: negation needs an integer argument
>    [255]
>
>  top-level integer literal is interpreted as symbol (i.e. variable name):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DQIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=mEgSWILcY4c4W3zjApBQLA&m=XFhoSiP_P_OfcF1nPonzg26UVzoFHwYzgiVf6eYEeTQ&s=MJGhJupzANn-msr3d3iC-NygLglXMLwtCUJOgegoM0I&e=
>
Yuya Nishihara - Oct. 9, 2016, 10:52 a.m.
On Sat, 8 Oct 2016 18:26:30 +0200, Simon Farnsworth wrote:
> This is RFC because I have no idea what I'm doing in the parser. If
> someone at the sprint has 5 minutes available to educate me, I can
> update this to a better version.
> 
> On 08/10/2016 18:24, Simon Farnsworth wrote:
> > # HG changeset patch
> > # User Simon Farnsworth <simonfar@fb.com>
> > # Date 1475943860 25200
> > #      Sat Oct 08 09:24:20 2016 -0700
> > # Node ID e89699ba5c9f0bf883bfae7c485e50219b90b2f9
> > # Parent  91a3c58ecf938ed675f5364b88f0d663f12b0047
> > templater: provide ring operations on integers

This generally looks good to me.

A few nits follow.

> > --- a/mercurial/templater.py
> > +++ b/mercurial/templater.py
> > @@ -33,6 +33,9 @@
> >      "|": (5, None, None, ("|", 5), None),
> >      "%": (6, None, None, ("%", 6), None),
> >      ")": (0, None, None, None, None),
> > +    "+": (10, None, None, ("+", 10), None),
> > +    "-": (10, None, ("negate", 10), ("-", 10), None),
> > +    "*": (12, None, None, ("*", 12), None),

Perhaps these operators should have lower binding values than template
operations so 'x|y - z' is parsed as (x|y) - z.

  $ hg debugtemplate -r0 -v '{revset(".")|count - 1}\n'

but "negate" will still need higher binding limit.

  $ hg debugtemplate -r0 -v '{-3|stringify}\n'

https://selenic.com/repo/hg/rev/e797fdf91df4

> > -        elif c.isdigit() or c == '-':
> > +        elif c.isdigit():
> >              s = pos

We can remove the next 4 or 5 lines which were necessary to handle '-'.

> >              if c == '-': # simply take negate operator as part of integer
> >                  pos += 1
timeless - Oct. 21, 2016, 1:21 p.m.
Simon Farnsworth wrote:
> @@ -33,6 +33,9 @@
> +    "+": (10, None, None, ("+", 10), None),
> +    "-": (10, None, ("negate", 10), ("-", 10), None),
> +    "*": (12, None, None, ("*", 12), None),

Is it reasonable for me to ask for some user facing documentation of this?

Possibly just an example. Although, we might want to wait until we're
sure that this is the way we want to go before publishing it. I'd
definitely like to see what the docs would look like. -- And yes, in
theory math is "simple", but as soon as you switch from {Integers} to
{Rings} you are going to confuse someone. If I do 20*20 on a 137
character wide screen, what happens?

Patch

diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -33,6 +33,9 @@ 
     "|": (5, None, None, ("|", 5), None),
     "%": (6, None, None, ("%", 6), None),
     ")": (0, None, None, None, None),
+    "+": (10, None, None, ("+", 10), None),
+    "-": (10, None, ("negate", 10), ("-", 10), None),
+    "*": (12, None, None, ("*", 12), None),
     "integer": (0, "integer", None, None, None),
     "symbol": (0, "symbol", None, None, None),
     "string": (0, "string", None, None, None),
@@ -48,7 +51,7 @@ 
         c = program[pos]
         if c.isspace(): # skip inter-token whitespace
             pass
-        elif c in "(,)%|": # handle simple operators
+        elif c in "(,)%|+-*": # handle simple operators
             yield (c, None, pos)
         elif c in '"\'': # handle quoted templates
             s = pos + 1
@@ -70,7 +73,7 @@ 
                 pos += 1
             else:
                 raise error.ParseError(_("unterminated string"), s)
-        elif c.isdigit() or c == '-':
+        elif c.isdigit():
             s = pos
             if c == '-': # simply take negate operator as part of integer
                 pos += 1
@@ -420,6 +423,29 @@ 
             # If so, return the expanded value.
             yield i
 
+def buildnegate(exp, context):
+    arg = compileexp(exp[1], context, exprmethods)
+    return (runnegate, arg)
+
+def runnegate(context, mapping, data):
+    data = evalinteger(context, mapping, data,
+                        _('negation needs an integer argument'))
+    return -data
+
+
+def buildarithmetic(exp, context, func):
+    left = compileexp(exp[1], context, exprmethods)
+    right = compileexp(exp[2], context, exprmethods)
+    return (runarithmetic, (func, left, right))
+
+def runarithmetic(context, mapping, data):
+    func, left, right = data
+    left = evalinteger(context, mapping, left,
+                        _('arithmetic only defined on integers'))
+    right = evalinteger(context, mapping, right,
+                        _('arithmetic only defined on integers'))
+    return func(left, right)
+
 def buildfunc(exp, context):
     n = getsymbol(exp[1])
     args = [compileexp(x, context, exprmethods) for x in getlist(exp[2])]
@@ -906,6 +932,7 @@ 
 # methods to interpret function arguments or inner expressions (e.g. {_(x)})
 exprmethods = {
     "integer": lambda e, c: (runinteger, e[1]),
+    "negate": lambda e, c: (runinteger, e[1]),
     "string": lambda e, c: (runstring, e[1]),
     "symbol": lambda e, c: (runsymbol, e[1]),
     "template": buildtemplate,
@@ -914,6 +941,10 @@ 
     "|": buildfilter,
     "%": buildmap,
     "func": buildfunc,
+    "+": lambda e, c: buildarithmetic(e, c, lambda a, b: a + b),
+    "-": lambda e, c: buildarithmetic(e, c, lambda a, b: a - b),
+    "negate": buildnegate,
+    "*": lambda e, c: buildarithmetic(e, c, lambda a, b: a * b),
     }
 
 # methods to interpret top-level template (e.g. {x}, {x|_}, {x % "y"})
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
@@ -5,6 +5,8 @@ 
   $ echo line 1 > b
   $ echo line 2 >> b
   $ hg commit -l b -d '1000000 0' -u 'User Name <user@hostname>'
+  $ hg log -T '{date(date, "%s") + 1} {date(date, "%s") - 2 * 1}\n'
+  1000001 999998
 
   $ hg add b
   $ echo other 1 > c
@@ -2890,14 +2892,15 @@ 
   $ hg debugtemplate -v '{(-4)}\n'
   (template
     (group
-      ('integer', '-4'))
+      (negate
+        ('integer', '4')))
     ('string', '\n'))
   -4
   $ hg debugtemplate '{(-)}\n'
-  hg: parse error at 2: integer literal without digits
+  hg: parse error at 3: not a prefix: )
   [255]
   $ hg debugtemplate '{(-a)}\n'
-  hg: parse error at 2: integer literal without digits
+  hg: parse error: negation needs an integer argument
   [255]
 
 top-level integer literal is interpreted as symbol (i.e. variable name):