[PATCH] color: labeled text should be passed to ui.write() as ui.labeled

Matt Mackall mpm at selenic.com
Tue Jun 8 15:44:54 CDT 2010


On Thu, 2010-06-03 at 23:53 -0500, Steve Borho wrote:
> # HG changeset patch
> # User Steve Borho <steve at borho.org>
> # Date 1275625098 18000
> # Node ID ce4224afddd4e2c3c3a138d0fdec001fe0dbd5ef
> # Parent  e581f3acc3385da5d5ddf547882f274d9f7e5b6d
> color: labeled text should be passed to ui.write() as ui.labeled
> 
> Some implementations of ui.label() (HTML versions in particular) must escape
> the provided text and then markup the text with their tags.  When this marked
> up text is then passed to ui.write(), we must label the text as 'ui.labeled'
> so the implementation knows not to escape it a second time (exposing the initial
> markup).
> 
> This required the addition of a 'ui.plain' label for text that is purposefully
> not marked up.
> 
> I was a little pedantic here, passing even ' ' strings to ui.label() when it
> would be included with other labeled text in a ui.write() call.   But it seemed
> appropriate to lean to the side of caution.

Missed this on the list, but I see this made it into crew. I can't say I
like it..

> diff --git a/hgext/churn.py b/hgext/churn.py
> --- a/hgext/churn.py
> +++ b/hgext/churn.py
> @@ -150,8 +150,10 @@
>      if opts.get('diffstat'):
>          width -= 15
>          def format(name, (added, removed)):
> -            return "%s %15s %s%s\n" % (pad(name, maxname),
> -                                       '+%d/-%d' % (added, removed),
> +            return "%s %15s %s%s\n" % (ui.label(pad(name, maxname),
> +                                                'ui.plain'),
> +                                       ui.label('+%d/-%d' % (added, removed),
> +                                                'ui.plain'),
>                                         ui.label('+' * charnum(added),
>                                                  'diffstat.inserted'),
>                                         ui.label('-' * charnum(removed),

There are a lot of changes here that don't make any sense unless you
know that there's some weird implementation detail -that you can't
actually find anywhere in the source- that's getting tripped over.

This is not great. It'd be better to inflict some ugly hack on the HTML
labeler than on the whole of core. For instance, enclosing labeled
that's gone through the wash once already in "<div class=labelled>" and
then stripping that out later in a wrapper on ui.write().

The guiding principle here is: extensions don't get to inflict pain on
the core. And this is definitely pain.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list