Patchwork [2,of,2] templater: make label() just fail if ui object isn't available

login
register
mail settings
Submitter Yuya Nishihara
Date March 9, 2016, 3:34 p.m.
Message ID <ed81b4ff8cf1af457072.1457537669@mimosa>
Download mbox | patch
Permalink /patch/13724/
State Accepted
Headers show

Comments

Yuya Nishihara - March 9, 2016, 3:34 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1457535566 -32400
#      Wed Mar 09 23:59:26 2016 +0900
# Node ID ed81b4ff8cf1af457072eaff8ae055943562b624
# Parent  62d64dedd1098442819b8f16ae7a7848e45d3a59
templater: make label() just fail if ui object isn't available

Silent failure hides bugs and makes it harder to track down the issue. It's
worse than raising exception.

In future patches, I plan to sort out template functions that require 'ui',
'ctx', 'fctx', etc. so that incompatible functions are excluded and the doc can
say in which context these functions are usable.

  @templatefunc('label', requires=('ui',))
  def label(context, mapping, args):
      ...
Pierre-Yves David - March 9, 2016, 6:18 p.m.
On 03/09/2016 03:34 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1457535566 -32400
> #      Wed Mar 09 23:59:26 2016 +0900
> # Node ID ed81b4ff8cf1af457072eaff8ae055943562b624
> # Parent  62d64dedd1098442819b8f16ae7a7848e45d3a59
> templater: make label() just fail if ui object isn't available
>
> Silent failure hides bugs and makes it harder to track down the issue. It's
> worse than raising exception.
>
> In future patches, I plan to sort out template functions that require 'ui',
> 'ctx', 'fctx', etc. so that incompatible functions are excluded and the doc can
> say in which context these functions are usable.
>
>    @templatefunc('label', requires=('ui',))
>    def label(context, mapping, args):

Should we introduce a devel warning for one version to let buggy 
template gracefully recover in the wild?
Yuya Nishihara - March 11, 2016, 1:52 p.m.
On Wed, 9 Mar 2016 18:18:02 +0000, Pierre-Yves David wrote:
> On 03/09/2016 03:34 PM, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1457535566 -32400
> > #      Wed Mar 09 23:59:26 2016 +0900
> > # Node ID ed81b4ff8cf1af457072eaff8ae055943562b624
> > # Parent  62d64dedd1098442819b8f16ae7a7848e45d3a59
> > templater: make label() just fail if ui object isn't available
> >
> > Silent failure hides bugs and makes it harder to track down the issue. It's
> > worse than raising exception.
> >
> > In future patches, I plan to sort out template functions that require 'ui',
> > 'ctx', 'fctx', etc. so that incompatible functions are excluded and the doc can
> > say in which context these functions are usable.
> >
> >    @templatefunc('label', requires=('ui',))
> >    def label(context, mapping, args):
> 
> Should we introduce a devel warning for one version to let buggy 
> template gracefully recover in the wild?

Er, we wouldn't want a warning in the middle of template output because it would
be hard to read. Also, if label() can access to ui.develwarn(), it can call
ui.label().

The best thing it can do here is to raise Abort, which I want to implement
later.
Pierre-Yves David - March 11, 2016, 2:25 p.m.
On 03/11/2016 01:52 PM, Yuya Nishihara wrote:
> On Wed, 9 Mar 2016 18:18:02 +0000, Pierre-Yves David wrote:
>> On 03/09/2016 03:34 PM, Yuya Nishihara wrote:
>>> # HG changeset patch
>>> # User Yuya Nishihara <yuya@tcha.org>
>>> # Date 1457535566 -32400
>>> #      Wed Mar 09 23:59:26 2016 +0900
>>> # Node ID ed81b4ff8cf1af457072eaff8ae055943562b624
>>> # Parent  62d64dedd1098442819b8f16ae7a7848e45d3a59
>>> templater: make label() just fail if ui object isn't available
>>>
>>> Silent failure hides bugs and makes it harder to track down the issue. It's
>>> worse than raising exception.
>>>
>>> In future patches, I plan to sort out template functions that require 'ui',
>>> 'ctx', 'fctx', etc. so that incompatible functions are excluded and the doc can
>>> say in which context these functions are usable.
>>>
>>>     @templatefunc('label', requires=('ui',))
>>>     def label(context, mapping, args):
>>
>> Should we introduce a devel warning for one version to let buggy
>> template gracefully recover in the wild?
>
> Er, we wouldn't want a warning in the middle of template output because it would
> be hard to read. Also, if label() can access to ui.develwarn(), it can call
> ui.label().
>
> The best thing it can do here is to raise Abort, which I want to implement
> later.

Okay, pushed to the clowncopter.

Patch

diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -547,12 +547,8 @@  def label(context, mapping, args):
         # i18n: "label" is a keyword
         raise error.ParseError(_("label expects two arguments"))
 
+    ui = mapping['ui']
     thing = evalstring(context, mapping, args[1])
-
-    ui = mapping.get('ui', '')
-    if isinstance(ui, str):
-        return thing
-
     # preserve unknown symbol as literal so effects like 'red', 'bold',
     # etc. don't need to be quoted
     label = evalstringliteral(context, mapping, args[0])