This closes the follow-ups of #16115, tracked in #16120.
The icon handling of the app launcher commands was written four times: once
in omarchy-tui-install, once in omarchy-webapp-install, and once in each
of the two removal scripts. The copies had already started to differ, and
the review of #16115 named what the differences cost. This gives the rule one
owner and closes the gaps that were left.
What changed
One owner for the icon rule. The name an icon is known by, the
reference-to-name rule, the local install, the guarded download, the theme
lookup and the removal live in install/helpers/icon.sh, which the four
commands source. The copies that already differed are now the caller's
choice: the web app installer discards the downloader's error output because
it falls back to another source quietly, and the TUI installer keeps it
because a failed fetch has to say why it failed.
An icon is removed in the format it was installed with. The installers
copy a local icon under the extension that file already has, so the formats
they can produce are not bounded by the formats the theme ships. The removal
matched the icon by name in png, svg and xpm, so an icon installed in
any other format was left behind. It is now matched by its sanitized name in
every format, and only the files that are images go with it, so a file that
carries the name but is not an icon stays.
A download in flight leaves nothing in the indexed directory. The answer
is downloaded to a temporary file before the icon is written. That file used
to be created inside the icon directory the desktop indexes, so a download
that died left a file there for the theme to pick up. The temporary file is
now written outside that directory, and it is removed when the install is
interrupted. The caller's own signal handlers are put back afterwards, so a
command that already handles a signal does not lose it because it downloaded
an icon.
The icon theme cache is refreshed. The base refreshed it after every
download, and the shared helper refreshes it too, so an icon installed from a
URL is registered with the cache the desktop resolves it through.
One download deadline, named. The TUI path had no limit before #16115 and
the web app path had ten seconds. The shared helper holds one deadline of ten
seconds, and the limit the search reaches is named next to the rule as well.
The theme lookup reaches every real entry. The bound is the icon theme's
own layout: the theme, the size, the context and the name, which is four
levels below the icons directory the search starts at. On a live system
68,952 entries sit at that level and none sits deeper, so the bound covers
everything that is really there.
The suite covers what it did not. The web app installer's download
contract is now exercised: an icon URL that answers with an image installs
it, one that answers with an error body fails the install and leaves the
installed icon alone. The lookup is also exercised against the system data
directories, the branch the environment takes when it names no list of its
own, and every case now asserts the exit status it captures.
How it was verified
The suite in test/shell.d/tui-icon-test.sh runs the real commands
against a fake home. The new cases fail against the code before this
change: an icon installed as .jpg survived its removal, and a download in
flight left a file in the icon directory.
The full suite was run on this change and on the commit before it, one
after the other, and the set of failures is the same. The failures that
were already there are environmental, and the pull request lists them.
The change was reviewed with the native review contract, and the receipt
covers the commit that is pushed.
This is stacked on #16115: the rule only exists there, so this branch is
based on it and the pull request targets that branch.
Stacked on #16115: the rule this change gives one owner is introduced there,
so this branch is based on it and the pull request targets quattro until
#16115 lands. Once it does, this pull request shows only its own two commits.
The seven commits it carries now are #16115's.
greptile-apps[bot]16h ago
Confidence Score: 5/5
[Medium impact] The PR appears safe to merge, with two non-blocking cleanup and test concerns.
The PR gives the four launcher commands one shared owner for icon naming, downloads, lookup, installation, and removal.
TUI and web-app launchers share rules for installing icons.
Removing a launcher removes its matching image icons in any format.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[TUI install and remove] --> C[Shared icon helper]
B[Web app install and remove] --> C
C --> D[Icon names and theme lookup]
C --> E[Checked downloads and cache refresh]
C --> F[Local install and image removal]
Stacked on #16115: the rule this change gives one owner is introduced there, so this branch is based on it and the pull request targets
quattrountil #16115 lands. Once it does, this pull request shows only its own two commits.The seven commits it carries now are #16115's.