-
Notifications
You must be signed in to change notification settings - Fork 103
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
Allowing passing unknown attributes #431
Comments
Got any idea for an API? |
I see 2 ways:
Normally I'd think the second idea is nicer because we reuse the existing API, but since it looks like description is the only contents which would be concerned by this, the first idea is probably better. |
The idea is to add some "demo" attribute to a list item inside the <release> tag, since we already decided that (for now at least) we'd keep a strict "intro + list" logics, as we did until now. This demo attribute uses an internal format to specify successive widgets to blink (like a demo path towards a feature). For now, what it allows is: * raise the toolbox, select a tool and blink the tool button. * raise a dockable, blink any widgets in there. Now it is still limited and needs to evolve. In particular: * What happens if the blinked tool button was explicitly removed from Preferences? Should we re-add it for the demo? And once done, should we remove it again? Then should we select back the tool previously selected? * What happens if the dockable widget is not visible? Should we allow changing the settings to be able to demo correctly the new/changed settings? Should it be temporary? If temporary, it can be annoying as you'd still have to look attentively the demo to find back the path to the new settings. If not temporary, some people may dislike we touch their settings. * What if docks are hidden? Should we unhide them, then hide them back after demo time? Also regarding the implementation: originally I wanted to just grab the demo attribute directly from the AppStream metadata file, but I realized that appstream-glib cleans out unknown attribute from the XML. I could then simply parse the file with a generic XML parser, but I found simpler to pre-parse it into a header built within GIMP. I still use appstream-glib at runtime as it takes care of localization for us (though in the same time, we also have the localization in the main po files, so maybe we could just embed the release note strings as well). See appstream-glib report: hughsie/appstream-glib#431
Thinking about this some more; isn't putting unknown tags in to the description metadata going to make it fail the appstream validator? |
It absolutely will, but I think unknown attributes will just be ignored and just not rendered (and I think that's true for libappstream and libappstream-glib). |
In my case, it's not tags, only attribute, and it doesn't fail in I haven't tested other validators. Are there any? (I know there is an appstream validator on flathub too, I'm not sure what tools it uses and have not tested this on Flathub, though I should) This being said, adding unknown tags would be interesting too, because it would allow to add translatable text using the existing XML-aware gettext infrastructure (stuff like After all a great advantage of XML is its extensibility (it's even in the name!), i.e. that you can add more types of custom nodes, attributes of whatelse for custom needs. A specific purpose parser should not break on unknown tags or properties (or ideally it should have several modes, so that you can choose if you want to break on, ignore/discard, or pass along the unknown tags/properties). Most XML formats work this way (I used to do a lot of things with XMPP and one of the main point was its extensibility to communicate organized data with sub-specifications). Right now, I am pre-processing the AppStream file with a generic XML parser to generate a header with the info (since I could not use appstream-glib). This works very well of course and possibly I'll stay like this now that I did this work. But long term, I think that it would be a good idea to allow passing the unknown data along (as an option). As I said, I believe a good XML-format parser should allow such usage. For the record, this is the kind I think which GIMP does: https://twitter.com/zemarmot/status/1500781782857392130 And it does it from this single "demo" attribute in our AppStream file (inside the
|
Yes, the reference implementation, The libappstream policy for unknown stuff is to flag it as bad when validating (after all it may be a misspelled tag, the person is trying to use a feature that isn't supported by the implementation yet), strip such data when exporting (we only export pristine AppStream data, ideally) as software centers must be able to understand the generated data, but keep extra attributes when loading data. |
Btw, did you consider using Itstool for translation? Using plain xgettext with AppStream's ITS rules may also work for many usecases (I even added a helper for this to newer Meson versions to make translating AppStream data easier: https://mesonbuild.com/i18n-module.html#i18nitstool_join ). |
I confirm that this tool does not complain on unknown attributes, so it's cool on this side (regarding my current implementation). But it does error out on unknown tags.
Yeah I am not pushing this as a feature proposal. It's obviously specific to our program and how it works. ;-) I just think it's a nice idea to be able to add additional data (as long as it doesn't break other parsers; it could even be in separate XML namespaces if needed be). The reason why I thought to reuse the AppStream is that I realized we already have a list of release items, and it's even already translated by our translator teams. So it just made sense to tie this to the AppStream data. And while doing this, avoiding out-of-sync data requires to be in the same file.
Sure looks interesting. Now I wouldn't work on this though I would accept a patch for this if it's considered the way forward (is the old |
as_release_get_description()
and other similar API will clean out the returned XML from any unknown attribute.While it is understandable as a default, I would like to propose we get a way to get a description with unknown attribute.
Context: in GIMP, we are starting to use the AppStream data to show the release notes at first launch when a new version is installed/updated. Since the data was already here, localized and all, it seemed a good way to use it. I want to go further by adding some internal data which allow us to link a release point to specific GUI elements (people could then click a release point and the GUI element would blink). And basically how I want to do it is by adding some attribute to the release points. Unfortunately I realized these were removed by appstream-glib.
Note that we can for instance just store this data in yet another file, maybe with an order relation. But it's less foolproof. If we add new release items in the middle or reorder the items, we'd always have to make sure we don't break the relations. The real good and clean solution would be that appstream-glib allowed us to get uncleaned release items.
The text was updated successfully, but these errors were encountered: