Make the focus exception list real, or it was a page telling a lie
The mode data model shipped with an allow list and nothing that read it. The summary would say "2 apps may interrupt" while notification delivery never consulted the list and no editor could set it. That is the dead row this work has spent its time removing, introduced by the work itself. The banner gate consults the mode in force now, and the list can be edited from the applications that have actually sent a notification -- an exception for something that never notifies is not a choice worth offering. Exceptions belong to a mode. allowedApps is empty whenever no mode is active, so a Do Not Disturb switched on by hand stays absolute and nothing can leak into it. That scoping is asserted, not just written. Verifying this took three attempts, and the second was a real defect in the guard rather than in the code. The contract grep for FocusModes.allows matched the comment that explains it, so the check passed with the enforcement deleted. It matches the gate expression now. A guard a comment can satisfy is not a guard, and this is the third time prose has satisfied one here. Claude-Session: https://claude.ai/code/session_01BRvzt4H8XXLPVH5MyYdk9L
This commit is contained in:
Gabriel Brown
@@ -12,6 +12,20 @@ SettingsPage {
|
||||
// Which focus mode is open for editing. One at a time, like the app rules.
|
||||
property string expandedMode: ""
|
||||
|
||||
// Which mode's exception list is open. Separate from expandedMode so the
|
||||
// list does not unfold every time a mode is opened to change a switch.
|
||||
property string expandedAllow: ""
|
||||
|
||||
function toggleAllowed(mode: var, appId: string, allowed: bool): void {
|
||||
const current = Array.isArray(mode.allow) ? mode.allow.map(String) : [];
|
||||
const at = current.indexOf(appId);
|
||||
if (allowed && at < 0)
|
||||
current.push(appId);
|
||||
else if (!allowed && at >= 0)
|
||||
current.splice(at, 1);
|
||||
FocusModes.update(String(mode.id), { allow: current });
|
||||
}
|
||||
|
||||
// Schedule edits go through the mode's trigger list rather than replacing
|
||||
// it, so a mode that also has a game or fullscreen trigger keeps it.
|
||||
function reschedule(mode: var, changes: var): void {
|
||||
@@ -298,6 +312,77 @@ SettingsPage {
|
||||
onAccepted: value => root.reschedule(modeEntry.mode, { end: value })
|
||||
}
|
||||
|
||||
SettingRow {
|
||||
width: parent.width
|
||||
visible: modeEntry.open && modeEntry.mode.silence === true
|
||||
label: "May interrupt"
|
||||
detail: (modeEntry.mode.allow ?? []).length === 0
|
||||
? "Nothing gets through while this mode is on"
|
||||
: "Everything else is held until the mode ends"
|
||||
activatable: true
|
||||
controlWidth: 150
|
||||
onActivated: root.expandedAllow =
|
||||
root.expandedAllow === modeEntry.modeId ? "" : modeEntry.modeId
|
||||
|
||||
Row {
|
||||
anchors.right: parent.right
|
||||
anchors.verticalCenter: parent.verticalCenter
|
||||
spacing: 9
|
||||
|
||||
Text {
|
||||
anchors.verticalCenter: parent.verticalCenter
|
||||
text: (modeEntry.mode.allow ?? []).length === 0
|
||||
? "Nothing"
|
||||
: (modeEntry.mode.allow ?? []).length + " apps"
|
||||
color: Theme.fgDim
|
||||
font.family: Theme.fontFamily
|
||||
font.pixelSize: Theme.fontSize
|
||||
}
|
||||
|
||||
Text {
|
||||
anchors.verticalCenter: parent.verticalCenter
|
||||
text: root.expandedAllow === modeEntry.modeId ? "\u25B4" : "\u25BE"
|
||||
color: Theme.fgMuted
|
||||
font.family: Theme.fontFamily
|
||||
font.pixelSize: Theme.fontSizeSmall
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Drawn from the applications that have actually sent a
|
||||
// notification, which is the same list the rules above use --
|
||||
// an exception for something that never notifies is not a
|
||||
// choice worth offering.
|
||||
Repeater {
|
||||
model: (modeEntry.open && root.expandedAllow === modeEntry.modeId)
|
||||
? Notifs.applications
|
||||
: []
|
||||
|
||||
delegate: SettingRow {
|
||||
required property var modelData
|
||||
width: parent.width
|
||||
label: String(modelData.name ?? "")
|
||||
detail: String(modelData.id ?? "")
|
||||
controlWidth: 48
|
||||
|
||||
SettingsToggle {
|
||||
anchors.right: parent.right
|
||||
anchors.verticalCenter: parent.verticalCenter
|
||||
checked: (modeEntry.mode.allow ?? []).indexOf(String(modelData.id)) >= 0
|
||||
onToggled: value => root.toggleAllowed(modeEntry.mode, String(modelData.id), value)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
TextRow {
|
||||
width: parent.width
|
||||
visible: modeEntry.open && root.expandedAllow === modeEntry.modeId
|
||||
&& Notifs.applications.length === 0
|
||||
label: "No applications yet"
|
||||
detail: "They appear here once they have sent a notification."
|
||||
value: ""
|
||||
}
|
||||
|
||||
SettingRow {
|
||||
width: parent.width
|
||||
visible: modeEntry.open && modeEntry.schedule !== null
|
||||
|
||||
@@ -57,6 +57,21 @@ Singleton {
|
||||
}
|
||||
|
||||
readonly property bool active: root.activeMode !== null
|
||||
|
||||
// Applications the mode in force lets through anyway, by the same id the
|
||||
// per-application rules use. Empty whenever no mode is active, which is
|
||||
// what keeps a Do Not Disturb somebody set by hand absolute: an exception
|
||||
// belongs to a mode, so without a mode there are no exceptions.
|
||||
readonly property var allowedApps: {
|
||||
const mode = root.activeMode;
|
||||
if (!mode || mode.silence !== true)
|
||||
return [];
|
||||
return Array.isArray(mode.allow) ? mode.allow.map(String) : [];
|
||||
}
|
||||
|
||||
function allows(appId: string): bool {
|
||||
return root.allowedApps.indexOf(String(appId)) >= 0;
|
||||
}
|
||||
readonly property string activeName: String(root.activeMode?.name ?? "")
|
||||
|
||||
// Why it is on, in the words the page uses, so "something silenced my
|
||||
|
||||
@@ -234,7 +234,10 @@ Singleton {
|
||||
root.unreadCount += 1;
|
||||
}
|
||||
|
||||
if (!root.doNotDisturb) {
|
||||
// An exception belongs to the focus mode that is in force. A Do Not
|
||||
// Disturb switched on by hand has no exceptions and stays absolute,
|
||||
// because FocusModes.allows is false whenever no mode is active.
|
||||
if (!root.doNotDisturb || FocusModes.allows(root.notificationAppId(notification))) {
|
||||
root.popups = [notification].concat(root.popups);
|
||||
} else if (notification.transient) {
|
||||
// Never shown, and (being transient) never filed in history
|
||||
|
||||
@@ -135,4 +135,30 @@ grep -q 'key: "gamingSilenceNotifications"' "$schema" \
|
||||
grep -q 'FocusModes' "$gaming_page" \
|
||||
|| fail 'the Gaming page does not point at the mode that replaced its switch'
|
||||
|
||||
# ── 6. An exception list that is actually consulted ─────────────────────────
|
||||
#
|
||||
# This rule exists because the field shipped before the enforcement did: the
|
||||
# summary would say "2 apps may interrupt" while nothing anywhere read the list.
|
||||
# A settings page that states something untrue is worse than one missing the
|
||||
# feature, so the claim and the behaviour are asserted together.
|
||||
|
||||
notifs="$repo_dir/config/dot/quickshell/services/Notifs.qml"
|
||||
page="$repo_dir/config/dot/quickshell/modules/settings/NotificationsPage.qml"
|
||||
|
||||
# Matched on the gate itself, not the name. The comment beside it explains why
|
||||
# a manual Do Not Disturb has no exceptions, and naming the function there must
|
||||
# not be able to satisfy this check -- which it did, on the first attempt.
|
||||
grep -q 'doNotDisturb || FocusModes.allows(' "$notifs" \
|
||||
|| fail 'the banner gate never consults the active mode, so an exception list would do nothing'
|
||||
grep -q 'function allows' "$service" \
|
||||
|| fail 'there is no way to ask whether an application is excepted'
|
||||
|
||||
# Exceptions belong to a mode. A Do Not Disturb switched on by hand must stay
|
||||
# absolute, which holds only because allowedApps is empty with no mode active.
|
||||
grep -q 'if (!mode || mode.silence !== true)' "$service" \
|
||||
|| fail 'exceptions are not scoped to an active silencing mode, so a manual Do Not Disturb could leak'
|
||||
|
||||
grep -q 'function toggleAllowed' "$page" \
|
||||
|| fail 'the exception list cannot be edited, so the summary could claim something unreachable'
|
||||
|
||||
printf 'focus modes contract: ok\n'
|
||||
|
||||
@@ -84,8 +84,14 @@ if (!source.includes("next[knownAppId] = root.appRule(knownAppId)"))
|
||||
fail("persisted rules are not normalized to the required three-field shape");
|
||||
if (!source.includes("root.fallbackAppRules = next"))
|
||||
fail("missing-schema preference writes do not retain an in-memory fallback");
|
||||
if (!source.includes("if (!root.doNotDisturb)"))
|
||||
fail("global DND popup override was removed");
|
||||
// Do Not Disturb still gates popups; it gained exactly one exception, and the
|
||||
// assertion names that exception rather than being loosened. A bare
|
||||
// if (!doNotDisturb) would mean the allow list of the active focus mode is
|
||||
// never consulted. Anything wider would let a manual Do Not Disturb be
|
||||
// overridden. No apostrophes here: this block is inside a single-quoted shell
|
||||
// string, and one closes it.
|
||||
if (!source.includes("if (!root.doNotDisturb || FocusModes.allows("))
|
||||
fail("Do Not Disturb no longer gates popups with a focus-mode exception");
|
||||
for (const required of [
|
||||
"DesktopEntries.applications.values",
|
||||
"DesktopEntries.byId(appId)",
|
||||
|
||||
Reference in New Issue
Block a user