Code review comment for lp:~spud/spud/syntaxhighlighting

Revision history for this message
Cian Wilson (cwilson) wrote :

Thanks for the reviews Patrick & Stephan.

diamond handles this change cleanly with a warning:

Warning: added xml attributes:
string_value/language

For this reason I don't think a specific script is necessary as tools/update_options.py handles it automatically.

Stephan, you're right. 'language="python"' is used in test_options.rnc and removing it has stopped all syntax highlighting on test xmls. Luckily I can add it back in without much conflict (as one is an attribute of the current node and the other is an attribute of the child of the node, thanks to string_value - see updated diff). Obviously however this is still a schema specific hack. I guess the right answer is to change the test_options.rnc from:
      element variables {
         ## A test variable
         element variable {
            attribute name { xsd:string },
            attribute language { "python" },
            xsd:string,
            comment
         }*
to:
      element variables {
         ## A test variable
         element variable {
            attribute name { xsd:string },
            python_code
         }*
with a similar change for the test element. This change however would definitely require a script as naively opening it in diamond deletes all the variables and tests! Thoughts...?

In terms of release, I wasn't really suggesting that this go in. In fact, I thought the final deadline for changes had passed. Still, it's a good point about how diamond will cope pre and post release in terms of back compatibility. It's further complicated by the dependency on where people are getting their diamond from. Not sure what to suggest here other than that people may have to put up with warnings about adding or deleting attributes depending on whether their diamond is pre or post release compatible.

« Back to merge proposal