Do not use value and namespaces to sort directives

[cedk]

The value and namespaces are dictionaries that can not be ordered.

The sorted was added in d48d93c

[FelixSchwarz]

Is there a way to trigger the problem? As far as I know all tests are passing. Is this is a problem for Tryton? (sorry, I misread you profile and thought of https://github.com/naftaliharris/tauthon/).

[cedk]

For some reason when using i18n.Translator filter, some directives like <for each=""><for each=""> ends up in the same SUB.
I do not know if it is normal or not but in this case the two same indexed directives are sorted and so the comparison is done on the other element. Indeed I think the sort should be done only on the index.

[cedk]

On a second though I do not really understand why d48d93c added such sorting. For me the parsing order should be always respected.

[FelixSchwarz]

Well, that commit is ancient history and I guess by now not even cmlenz would be able to recall the rationale. That's why I'm asking for any kind of test case which provides much more context. Personally I never touched this part of the code so I'm really out of my depth here but I hope a test case makes it easier to understand why your change is right.

[cedk]

I added a test case.

[FelixSchwarz]

Thank you very much for the test case which clearly shows that there is a problem. I think we should merge this (with all 3 commits squashed?).

@hodgestar any objections? (btw: if you merge this would you mind to use rebase merging? I prefer not to use merge commits unless there is a lot of parallel development ;-)

[cedk]

For me it is just fixing the surface. I think there is a problem with Translator which in some way change the order of directives by grouping them inside the same directives list.

[hodgestar]

The directives are sorted to ensure that they are applied in a predictable order.

In Python 2, most built-in objects were comparable, so sorting by the directive index and then everything else was probably a sensible idea. In Python 3, many objects that were comparable are no longer, so sorting by arbitrary dicts, lists, etc is now a bug that wasn't picked up when we ported Genshi to Python 3.

[hodgestar]

@cedk The fix looks correct to me. Thank you for finding, reporting and fixing this old bug leftover from the port to Python 3.

[hodgestar]

@hodgestar any objections? (btw: if you merge this would you mind to use rebase merging? I prefer not to use merge commits unless there is a lot of parallel development ;-)

This is probably a discussion for the mailing list rather than this PR. Our personal preferences seem to differ, but I have no real objection to doing other kinds of merges if it's generally appealing or particularly important to people. My preference order is probably: merge commit (single commit on the target branch & keeps commit hashes), squash commit (single commit & makes target branch tidy), rebase commits (many commits, avoids merges).

[cedk]

But I still think the real problem is the need to sort the directives. Normally they should just have the same order as when they are parsed.
As far as I can see in the repository history this sorting was cherry-picked for a i18n branch which probably introduced the Translator. So for me we should not need to sort the directives if the Translator was not grouping them in the same list of directives.

[hodgestar]

@cedk The element attributes in XML are explicitly unordered -- from https://www.w3.org/TR/2006/REC-xml11-20060816/#IDA10FS:

Note that the order of attribute specifications in a start-tag or empty-element tag is not significant.

Many directives in templates are associated with tags, which do have a parsing order, but others are not (e.g. <div py:match="..." py:attrs="...">).

[cedk]

The test case show the problem appears not with multiple attribute on the same tag but by having different tag directives merged into the same directive list.