Markup should be interoperable with other stuff

[ianb@…]

The Markup class can't be interoperable with other classes. Specifically in WebHelpers? we want to implement our own HTML literal class, basically equivalent to Markup('<tags>'). Our object has a method .__html__(), that both indicates the object is a literal, and can be used to give an HTML representation of the object.

To work with this, Markup.escape would have to test for this attribute. Right now it does an exact class check (even subclasses won't work). We can't even do any kind of conditional mix-in.

If the Markup class also provides an __html__ method, we could also support using Markup as a way of providing HTML literals in our own quoting code.

Also, Markup(0) == '', while Markup(1) == '1'.

[cmlenz]

(description formatting fixes)

[cmlenz]

Replying to ianb@colorstudy.com:

If the Markup class also provides an __html__ method, we could also support using Markup as a way of providing HTML literals in our own quoting code.

So what exactly is that method to return? In the case of Markup, simply self (instance of unicode subclass), or unicode(self) (unicode object), or…?

Also, Markup(0) == '', while Markup(1) == '1'.

I can't reproduce this. Markup(0) is '0' here.

[ianb@…]

So what exactly is that method to return? In the case of Markup, simply self (instance of unicode subclass), or unicode(self) (unicode object), or…?

It would return self, like:

class Markup:
    def __html__(self):
        return self

In escape it would look like:

def escape(thing):
    if hasattr(thing, '__html__'):
        return Markup(thing.__html__())
    ...

I can't reproduce this. Markup(0) is '0' here.

I was noticing a truth test in the code, where I would have expected text=='', but I didn't really test so I might have misread.

[cmlenz]

Patch

[cmlenz]

I've attached a patch against trunk that implements this.

[cboos]

In the patch for the _cspeedups.c,

59	        ret = PyObject_CallMethod(text, "__html__", NULL); 
60	        Py_INCREF(ret); 

the INCREF looks wrong here, as PyObject_CallMethod already returns a new reference.

[cboos]

I just came across a new use case for this: the genshi.Element itself could define this __html__ method, so that it becomes possible to pass tags in interpolated Markup strings.

Example in Trac:

 ...
    [(tag.p(_(Markup("Tickets for %(group)s:"),
              group=tag.a(query.group, ' ', v, href=href,
                          class_='query', title=title))),
 ...

See attachment:Element-__html__-r826+t211.diff​, which contains attachment:ticket202.diff​ + the fix from comment:5. That patch applies on r826 + the latest patch in #211, which provides the mapping-style interpolation.

Note that without the comment:5 fix, I got crashes while running the tests.

The example snippet above was tested in Trac and it works fine with the above patch. To be fully functional with translations, I guess the Babel string extractor has to be enhanced as well in order to find the text in _(Markup(...)) expressions.

[cboos]

Previous patch (fixed) + added Element.__html__ method. Applies on ticket:211:mapping-style-__mod__-only-and-no-Markup_new-r826.diff​

[cboos]

Amending comment:6: After a discussion with cmlenz, it seems that we could as well handle that in Trac itself, with a new translation function dedicated to this case.

Example in Trac:

Could be rewritten:

  ...
     [(tag.p(M_("Tickets for %(group)s:",
                group=tag.a(query.group, ' ', v, href=href,
                            class_='query', title=title)),
  ...

The M_ is itself an alias for mgettext(msg, **kwargs) (for "Markup" gettext) and takes msg to be valid markup. The advantage of this approach is that we could even defer serialization by producing tags if needed (tag(Markup("Tickets for ", tag.a(...), Markup(":")) in the above example).

See attachment:Element-__html__-r826+t211.diff​

So I'm not sure if it's still that much relevant now (don't forget to take into account the fix from comment:5 if you want to apply the original patch).

[cmlenz]

Patch applied in [861].