-
-
Notifications
You must be signed in to change notification settings - Fork 229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generate valid XML files #842
Comments
I always assumed the document itself was the (unnamed) root. Makes more sense to me, but apparently that is not the case. Thanks! I'm actually considering to switch to an entirely different format in future releases. If not, I'll make sure to reorganize and validate the files. |
Note the last bullet point in "key syntax rules"https://en.wikipedia.org/wiki/Well-formed_document
Also When I tried to open anime.xml with Notepad++ (with the XML tools plugin), I got the following popup:
One advantage of using XML is that it has very strict rules when it comes to parsing. This makes it easy to detect errors. I see you're using pugixml to parse XML files. I see pugixml accepts an input like this:
without returning an error. This means pugixml is a non-conforming parser, because
(from the same Wikipedia page; emphasis mine). So pugixml is not an XML parser. It parses something which is almost, but not quite, entirely unlike XML. I filed zeux/pugixml#337 Luckily, changing the format of
|
I'd like to go for this one. The one topic I'm not very sure about is compatibility. <?xml version="1.0"?>
<meta>
<version>1.3.1</version>
</meta>
<library>
... to <?xml version="1.0"?>
<library version="1.3.1">
... Should it maybe be called 'taiga-version' or something? (We have an XML version attribute right above it) And about compat: There's some places like auto version = ReadXmlVersion(document);
HandleCompatibility(version); If I change the code to use attributes instead of meta elements, the version that is currently inside the XML files will be ignored (version = 0.0.0). Should the old Read procedure be left in the code for compatibility? Or does this not matter, since there is nothing (except for the version itself) that changed in the XML? |
Ok, after reading through the compat code it seems pretty clear. Now, there is only settings.xml left. The file currently looks like this: <?xml version="1.0"?>
<settings>
<meta version="1.3.1" />
<account>
... This is different to the other meta elements, and does not violate the 'one root element' rule in its current state. It would be cleaner to change this to <?xml version="1.0"?>
<settings version="1.3.1">
<account>
... BUT there is still compat code for settings.xml. Keeping compatibility for both 'styles' of settings.xml shouldn't be hard, but this change isn't really necessary (as it is already valid XML, even if it is not uniform to the other files). I'd argue for keeping it as-is |
The history.xml and anime.xml files generated by Taiga have multiple roots ("meta" plus another file-specific tag), which does not conform to the XML standard and makes these files incompatible with any parser that conforms to the XML standard, like python's xml module.
Please consider reorganizing these xml files to conform with the XML standard. Thank you.
The text was updated successfully, but these errors were encountered: