Add support for AppStream 1.0 #2099
Conversation
src/Core/Package.vala
Outdated
| @@ -677,7 +681,11 @@ public class AppCenterCore.Package : Object { | |||
| } | |||
|
|
|||
| try { | |||
| #if HAS_APPSTREAM_1_0 | |||
| description = AppStream.markup_convert (description, AppStream.MarkupKind.XML); | |||
There was a problem hiding this comment.
I'm assuming we only convert XML 🤔
There was a problem hiding this comment.
The 2nd parameter is the format to convert to.
markup_convert_simple in AppStream < 1.0 converts to the TEXT markup kind, so XML here would be a difference in behaviour with the Appstream 1.0 code and should probably be changed to TEXT.
That said, it feels like we'd want to keep the description as markup so we can display it more richly in the interface, but I think checking and testing that is outside the scope of supporting AppStream 1.0
I've opened #2103 to investigate whether converting to TEXT is still correct.
| description = AppStream.markup_convert (description, AppStream.MarkupKind.XML); | |
| description = AppStream.markup_convert (description, AppStream.MarkupKind.TEXT); |
src/Widgets/ReleaseRow.vala
Outdated
| @@ -116,7 +116,11 @@ public class AppCenter.Widgets.ReleaseRow : Gtk.Box { | |||
| private string format_release_description (string? description ) { | |||
| if (description != null) { | |||
| try { | |||
| #if HAS_APPSTREAM_1_0 | |||
| var markup = AppStream.markup_convert (description, AppStream.MarkupKind.XML); | |||
There was a problem hiding this comment.
I'm assuming we only convert XML 🤔
There was a problem hiding this comment.
| var markup = AppStream.markup_convert (description, AppStream.MarkupKind.XML); | |
| var markup = AppStream.markup_convert (description, AppStream.MarkupKind.TEXT); |
As above
src/Views/AppInfoView.vala
Outdated
| var release_version = new AppStream.Relation (); | ||
| release_version.set_version (release.get_version ()); | ||
| if (package.installed && release_version.version_compare (package.get_version ())) { |
There was a problem hiding this comment.
I'm not sure if that's the correct way to handle this...
There was a problem hiding this comment.
I've left an alternative suggestion in a review comment for checking/testing
src/Views/AppInfoView.vala
Outdated
| var active_locale = "en_US"; | ||
| var languages = package_component.get_languages (); | ||
| if (languages.length () > 0) { | ||
| active_locale = languages.nth_data (0); | ||
| } |
There was a problem hiding this comment.
I think the active_locale here is essentially the user's language, not related to the languages of the package/component, so we can't always assume that the first language on the component is what we want to compare against.
However, I'm not sure how correct the existing code is, the en_US check feels weird to me, so does splitting the locale from the string.
But, these changes should be closer to the original AppStream < 1.0 code.
| var active_locale = "en_US"; | |
| var languages = package_component.get_languages (); | |
| if (languages.length () > 0) { | |
| active_locale = languages.nth_data (0); | |
| } | |
| var active_locale = "en_US"; | |
| if (package_component.get_context () != null) | |
| active_locale = package_component.get_context ().get_locale () ?? "C"; | |
| } |
There was a problem hiding this comment.
Ah yeah so what this is intended to do is get the user's current locale and then immediately after this we check if it's US English and if it is we don't show localization information because US English will always be 100% and report 0%
src/Views/AppInfoView.vala
Outdated
| var release_version = new AppStream.Relation (); | ||
| release_version.set_version (release.get_version ()); | ||
| if (package.installed && release_version.version_compare (package.get_version ())) { |
There was a problem hiding this comment.
Haven't tested if this compiles or works, but this feels closer to the original:
| var release_version = new AppStream.Relation (); | |
| release_version.set_version (release.get_version ()); | |
| if (package.installed && release_version.version_compare (package.get_version ())) { | |
| if (package.installed && AppStream.vercmp_simple (release.get_version (), package.get_version ()) <= 0) { |
appstream-1.0.vapi