T35065 handle app renaming #752
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To get X-Flatpak-RenamedFrom from the .desktop file, we would like to
call g_key_file_get_string_list(), but GDesktopAppInfo does not expose
its internal GKeyFile, nor provides a get_string_list() wrapper.
GKeyFile does not expose its string-list parser, so we open-code our
own.
An interesting quirk of the GKeyFile format is that lists are (supposed
to be) ;-terminated, not just ;-delimited. This is so you can express an
empty trailing element: "a;;" represents the list ["a", ""].
g_strsplit("a;", ";", -1) returns ["a", ""], so we take care to skip any
empty elements in the returned list.
It is possible to escape a semicolon in a list: "a\;b;" represents
["a;",
"b"]. We cannot support this here, because g_key_file_get_string(), used
internally by g_desktop_app_info_get_string(), rejects the "\;" sequence
and returns NULL.
In practice, .desktop filenames do not contain semicolons (or indeed any
characters more exotic than ASCII alphanumerics, hyphens, underscores
and dots) so this code will be fine. (The alternative would be to
re-parse every .desktop file using GKeyFile directly, fish this field
out, and throw it away again.)
https://phabricator.endlessm.com/T22596
AllView monitors ShellAppSystem::installed-changed, which indicates that 1 or more .desktop files have been added, removed, or changed, and queues a call to _redisplay(). Since no 'force' parameter is passed to BaseAppView._redisplay() in this case, it calls BaseAppView.iconsNeedRedraw() to decide whether anything has actually changed. That method checks for any (re)moved items, and if there are none, compares the old and new versions of each item. However, it previously only compared their name and their icon (name). This meant that changes to (for example) the Exec= line of a .desktop file would not be picked up until something else (such as a logout and login) causes the grid to be refreshed. In the case of a backwards-incompatible change to the application's command-line arguments, this would break launching the updated app from the icon grid. 2019-10-03: adapt checks to new upstream code https://phabricator.endlessm.com/T23877
In the upstream version of this file there is only one instance of this code. We have two, in app_is_stale() and app_info_changed(). The order of comparisons is different between the two, but ignoring that, the only difference is that the latter called g_app_info_equal(). For GDesktopAppInfo, g_app_info_equal() just compares the two desktop file IDs; in the common case, these are derived from the on-disk path, so comparing g_desktop_app_info_get_filename(), as app_is_stale() did, is enough. But, g_desktop_app_info_get_filename() can return NULL if the object was constructed with g_desktop_app_info_new_from_keyfile(); in that case, g_app_info_equal() falls back to pointer equality. With this patch, we call g_app_info_equal() on both code paths, and compare the results of g_desktop_app_info_get_filename() with g_strcmp0() since they can be NULL. https://phabricator.endlessm.com/T23877
Previously, we handled renamed apps on the icon grid, but not on the taskbar. There is some clear overlap with the RENAMED_DESKTOP_IDS in this file, which is inherited from upstream; https://gitlab.gnome.org/GNOME/gnome-shell/issues/593 filed to discuss upstreaming our approach. https://phabricator.endlessm.com/T22596
dylanmccall
force-pushed
the
T35065-handle-app-renaming
branch
from
d014159 to
7286317
Compare
Member
|
I proposed https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/593 upstream 5 years ago. Florian has an alternative approach using libappstream which has also stalled. Personally I don't think invoking another library and reading massive XML files from the compositor seems like a good idea! |
manuq
approved these changes
Jan 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At the moment, this pull request is bringing back the following patches, removing references to
X-Endless-Alias:563b2f1 shell-app-system: Add support for X-Endless-Alias key in desktop files(removed, squashed with the next patch)It is a draft while I look around for an opportunity to better align with upstream.
https://phabricator.endlessm.com/T35065