Skip to content
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

spec: Allow <icon> tags in generic components #138

Merged
merged 2 commits into from Oct 21, 2017

Conversation

mvollmer
Copy link
Contributor

And not just in the collection metadata. This way, components of type
"service" and any other other component can specify icons upstream in
a standardized way.

@mvollmer
Copy link
Contributor Author

See hughsie/appstream-glib#192 for the corresponding change to appstream-builder.

@ximion
Copy link
Owner

ximion commented Sep 25, 2017

The reference implementation and appstream-generator support this already for a really long time. The spec indeed should mention that (I actually thought appstream-glib started allowing the <icon/> tag for arbitrary components first...)

@mvollmer
Copy link
Contributor Author

The spec indeed should mention that .

What about the paragraph about the location restrictions? Should they be in the spec at all? They are mostly a consequence of how appstream-builder works. (But I think appstream-builder has very good reasons to work the way it works.)

I think the situation is similar to the one with icons from a *.desktop file. The Desktop Entry spec allows absolute pathnames pointing everywhere, but appstream-builder will ignore any that are not in the usual locations. This is documented as part of the appstream-builder README (https://github.com/hughsie/appstream-glib#guidelines-for-applications). I think we can do the same here.

<para>
<literal>local</literal> icons are loaded from a file in the filesystem.
They should specify a full file path in either <code>/usr/share/pixmaps</code>
or <code>/usr/share/icons</code>. (These restrictions on the icon location are in
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those restrictions are appstream-builder specific, I guess - the local type explicitly makes no restrictions for where icons are located, it could be anywhere on the filesystem. The appstream-generator tool deals with them whereever they are.
For the paths you mention, stock is the better solution anyway.
In general, using local is discouraged, stock is almost always the better option.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed that sentence.

</para>
<para>
<literal>remote</literal> icons loaded from a remote URL. Currently, at least HTTP urls must be supported.
This icon type may have <literal>width</literal> and <literal>height</literal> properties.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make that "may" a "should".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed. I have also made the corresponding change in xmldata.xml

properties.
</para>
<para>
<literal>remote</literal> icons loaded from a remote URL. Currently, at least HTTP urls must be supported.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can make "at least HTTP urls must be supported" and "only HTTP urls are supported", because that is much more precise and leaves no room for people reading the spec to speculate which other weird protocols we might support.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed. I have also made the corresponding change in xmldata.xml

And not just in the collection metadata.  This way, components of type
"service" and any other other component can specify icons upstream in
a standardized way.
<para>
The <code>&lt;icon/&gt;</code> tag describes the component icon. It is mostly used for GUI applications (component-type <literal>desktop-application</literal>).
It can be of the type <literal>stock</literal>, <literal>local</literal>,
or <literal>url</literal>.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be remote, but I can quickly fix that after merging the patch.

ximion
ximion approved these changes Oct 21, 2017
@ximion
Copy link
Owner

ximion commented Oct 21, 2017

Looks good now, except for one small type that I'll fix after merging the changes.
Thanks for your work!

@ximion ximion merged commit 10df64d into ximion:master Oct 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants