ximion / appstream

Tools and libraries to work with AppStream metadata
http://www.freedesktop.org/wiki/Distributions/AppStream/
GNU Lesser General Public License v2.1
210 stars 115 forks source link

Add Plasma Mobile to desktop-style-ids.txt #622

Closed 1Maxnet1 closed 5 months ago

1Maxnet1 commented 6 months ago

As discussed in #611 this PR adds Plasma Mobile to the list of desktop style ids.

Let me know if anything is missing

tintou commented 6 months ago

Wouldn't a "chassis" property make more sense? (as defined in https://www.freedesktop.org/software/systemd/man/latest/org.freedesktop.hostname1.html#Semantics ) This would be treated as a hint for the screenshot

1Maxnet1 commented 6 months ago

Wouldn't a "chassis" property make more sense? (as defined in https://www.freedesktop.org/software/systemd/man/latest/org.freedesktop.hostname1.html#Semantics )

I agree introducing a chassis type, makes sense, but would not eliminate the need for plasma-mobile to be added. If you have chassis type desktop/mobile and plasma/plasma-mobile, you could still have Plasma on a desktop/laptop, Plasma on a tablet or convertible (which could prefer the mobile chassis for screenshots, but runs the Desktop Version of plasma), a plasma-mobile installation on a smartphone and plasma-mobile on a docked smartphone (which again would potentially prefer the desktop screenshots as long as you are in a "desktop/docked" mode. To identify all those four cases, one would need both attributes. I admit however that this is currently an edge case. Nevertheless plasma-mobile and plasma are two different desktop environments so ignoring the mentioned edge case it might make sense to add it anywhere.

tintou commented 6 months ago

Ah so if it's a completely different environment it makes sense, I'm surprised to not see them in https://specifications.freedesktop.org/menu-spec/latest/apb.html though for instance

razzeee commented 6 months ago

I guess we should add phosh and gnome-mobile too then

1Maxnet1 commented 6 months ago

I guess we should add phosh and gnome-mobile too then

I agree for phosh, however gnome-mobile is as far as I know "only" a WIP branch of GNOME which eventually aims to be completely upstreamed. (See the postmarketOS Wiki for details https://wiki.postmarketos.org/wiki/GNOME) so I would not add it to the list. Phosh and Plasma Mobile make sense to me, because they plan to stay separate environments from GNOME/KDE Plasma, while sharing certain components. Would you add Phosh to this MR or should I create another for it?

razzeee commented 6 months ago

Still, apps targeting gnome-mobile might have different screenshots (probably portait mode) and it would be useful to know that via metadata. That's why I thought it might be smart to add gnome-mobile - that said, you're not wrong.

I think adding phosh here would be fine, but @ximion 's call

1Maxnet1 commented 6 months ago

Still, apps targeting gnome-mobile might have different screenshots (probably portait mode) and it would be useful to know that via metadata. That's why I thought it might be smart to add gnome-mobile - that said, you're not wrong.

Maybe a fitting alternative here would be to add gnome:mobile instead as a style. See also the note here: https://github.com/ximion/appstream/issues/611#issuecomment-2020115882

razzeee commented 6 months ago

Still, apps targeting gnome-mobile might have different screenshots (probably portait mode) and it would be useful to know that via metadata. That's why I thought it might be smart to add gnome-mobile - that said, you're not wrong.

Maybe a fitting alternative here would be to add gnome:mobile instead as a style. See also the note here: #611 (comment)

I think that would be bad as it probably breaks the possibility to have gnome-mobile:dark then

ximion commented 6 months ago

Also I agree with the others we should at least add phosh and probably also gnome mobile too, to that list :)

I do no longer just add stuff because it might be used in future, but only when it is specifically requested by the respective project owners / maintainers / contributors. There were some instances in the past where we preemptively added stuff to AppStream that the respective projects had different ideas about, that's why I am fairly conservative (we can always add stuff, but once IDs are in, removing them will be very hard or even impossible).

ximion commented 6 months ago

Also, that C header is auto-generated, it doesn't need to be changed specifically as it will be regenerated at the next opportunity (but changing it also doesn't hurt in this case, so with respect to that, this patch is fine).

razzeee commented 6 months ago

Sorry for the noise, please don't add phosh and gnome mobile then

Edit: To be clear, upon thinking this over - I don't think gnome mobile should be it's own platform. We probably don't want any screenshots, that actually include the shell or phosh shell.

1Maxnet1 commented 5 months ago

Thanks for all the comments. I updated the PR with @CarlSchwan's suggestions and as mentioned by @ximion I won't add Phosh or GNOME Mobile for now.

ximion commented 5 months ago

Nice! Let's merge it then! :-)