genshi.builder._kwargs_to_attrs generates duplicate attributes

[jmillikin@…]

Duplicate attributes are not allowed in XML, but without careful checks genshi.builder.tag can generate them. This is because _kwargs_to_attrs does not filter duplicates:

>>> from genshi.builder import _kwargs_to_attrs
>>> kwargs = {'class': 'a', 'class_': 'b'}
>>> _kwargs_to_attrs (kwargs)
[(QName(u'class'), u'b'), (QName(u'class'), u'a')]

Since kwargs are unordered anyway, an easy solution is to pass the list of attributes through a dictionary:

>>> def _kwargs_to_attrs(kwargs):
...     def to_qname(k):
...             return QName(k.rstrip('_').replace('_', '-'))
...     normalized = ((to_qname(k), unicode(v))
...                   for k, v in kwargs.items() if v is not None)
...     return list(dict (normalized).items())
>>> _kwargs_to_attrs (kwargs)
[(QName(u'class'), u'a')]

[cmlenz]

Fixed in [854]. Thanks!

[cboos]

Just a quick question: doesn't relying on the items() order mean that we'll have different orderings on different platforms? If so, it could make comparing the rendered output impossible (in unit tests like in ​trac/mimeview/tests for example).

[jmillikin@…]

That is true, but already in my experience already happens with 0.4.4. Not sure how the existing trac tests work around it, but in my unit tests, I define a function sorted_stream. It sorts attributes and renders to a string:

from genshi.core import START, Attrs, Stream

def sort_attr_filter (stream):
	for kind, data, pos in stream:
		if kind is START:
			tag, attributes = data
			yield (kind, (tag, Attrs (sorted (attributes))), pos)
		else:
			yield (kind, data, pos)
			
def sorted_stream (stream_or_tag):
	stream = Stream (stream_or_tag)
	sorted_stream = stream.filter (sort_attr_filter)
	return unicode (sorted_stream)
	

[cboos]

Ah, right, I didn't look closely enough, there was already a [... for k, v in kwargs.items()] in the previous code. So I guess we're just lucky so far ;-)

[cmlenz]

That sure broke a hell of a lot of Trac tests :P

I revised the change in [859], so that the ordering is the same as it was before [854]. Note that the ordering does rely on the way dict.items() in CPython orders results by the hash keys (IIUC), so it's still not a great idea to rely on it. The only way to make this fully (and portably) deterministic would be to sort the attributes, but that breaks too many tests for now. Maybe we can do that in a future release.