Make changing a default application actually take effect
Three bugs, all silent, all in the same feature. The write always worked. What failed was the refresh after it. That refresh is called from the mutation's own onExited handler and was guarded on `busy`, a binding over both processes -- and a binding hands back its cached value until the change notification feeding it has been delivered, which inside that handler has not happened yet. So `busy` read true, refresh returned immediately, and the page kept showing the old application with no error anywhere. Guards now read the Process objects directly, where the value is current, and a refresh is no longer blocked by the mutation that asked for it. The service also kept its own list of which roles it would accept. It stayed at seven when the helper and the page grew documents, text and archives, so choosing a PDF viewer set an error and changed nothing. It is derived from the snapshot now. And category matching never worked. DesktopEntries returns a QML list, for which Array.isArray is false, so the code stringified it into "Network,WebBrowser" and split on ";" alone -- one token matching no category. Browsers still appeared because their generic name contains "web browser" and the terms fallback carried the role by itself. Archives matched nothing at all, so that row could only ever offer the application it already had. The harness that should have caught the first bug passed while it was live: it set each role to the value it already had and asserted no error appeared, and the bug produces no error. It now changes a role to a genuinely different application, requires the service to observe the new value, and changes it back -- with the contract restoring the original from the outside however the run ends. Claude-Session: https://claude.ai/code/session_01BRvzt4H8XXLPVH5MyYdk9L
This commit is contained in:
@@ -18,9 +18,28 @@ Singleton {
|
||||
property var luaAutostartEntries: []
|
||||
property string lastError: ""
|
||||
|
||||
// For the UI, which wants one answer to "is anything happening".
|
||||
//
|
||||
// Guards inside this file do NOT use it. `busy` is a binding, and a binding
|
||||
// hands back its cached value until the change notification that feeds it
|
||||
// has been delivered. Inside a process's own onExited handler that has not
|
||||
// happened yet, so `busy` still reads true there -- which silently turned
|
||||
// the refresh after every successful write into a no-op. The write landed
|
||||
// and the settings page never noticed, which looks exactly like a settings
|
||||
// page that cannot change anything. Guards below read the Process objects
|
||||
// directly, where the value is current.
|
||||
readonly property bool busy: snapshotProcess.running || mutationProcess.running
|
||||
readonly property string helper: Quickshell.shellDir + "/scripts/panama-default-apps"
|
||||
readonly property var supportedRoles: ["browser", "mail", "files", "terminal", "music", "images", "video"]
|
||||
// Derived from the snapshot, never restated. This was a hardcoded list of
|
||||
// seven, and when the helper and the page grew documents, text and archives
|
||||
// it stayed at seven -- so choosing a PDF viewer set lastError and did
|
||||
// nothing, which reads exactly like a settings page that does not work.
|
||||
//
|
||||
// The snapshot already reports one handler per role the helper supports, so
|
||||
// that IS the list. An empty one means no snapshot has landed yet; the
|
||||
// helper validates the role itself and reports a failure, so there is
|
||||
// nothing for this guard to add before then.
|
||||
readonly property var supportedRoles: Object.keys(root.handlers ?? ({}))
|
||||
|
||||
Process {
|
||||
id: snapshotProcess
|
||||
@@ -59,7 +78,9 @@ Singleton {
|
||||
}
|
||||
|
||||
function refresh(): void {
|
||||
if (root.busy)
|
||||
// Only a snapshot already in flight is a reason not to start another.
|
||||
// A mutation finishing is the single best reason TO refresh.
|
||||
if (snapshotProcess.running)
|
||||
return;
|
||||
root.lastError = "";
|
||||
snapshotProcess.exec([root.helper, "snapshot"]);
|
||||
@@ -76,9 +97,11 @@ Singleton {
|
||||
}
|
||||
|
||||
function setDefault(role: string, desktopId: string): void {
|
||||
if (root.busy)
|
||||
if (mutationProcess.running)
|
||||
return;
|
||||
if (!root.supportedRoles.includes(role) || !root.knownDesktopId(desktopId)) {
|
||||
const roleIsKnown = root.supportedRoles.length === 0
|
||||
|| root.supportedRoles.includes(role);
|
||||
if (!roleIsKnown || !root.knownDesktopId(desktopId)) {
|
||||
root.lastError = "Choose an application from the available list."
|
||||
return;
|
||||
}
|
||||
@@ -87,7 +110,7 @@ Singleton {
|
||||
}
|
||||
|
||||
function setAutostart(desktopId: string, enabled: bool): void {
|
||||
if (root.busy)
|
||||
if (mutationProcess.running)
|
||||
return;
|
||||
const known = root.autostartEntries.some(entry => entry.id === desktopId);
|
||||
if (!known) {
|
||||
@@ -99,7 +122,7 @@ Singleton {
|
||||
}
|
||||
|
||||
function addAutostart(desktopId: string): void {
|
||||
if (root.busy)
|
||||
if (mutationProcess.running)
|
||||
return;
|
||||
if (!root.knownDesktopId(desktopId)) {
|
||||
root.lastError = "Choose an installed application.";
|
||||
|
||||
Reference in New Issue
Block a user