From f4b832eba5ccccc884aa1201e326f25fbb66efef Mon Sep 17 00:00:00 2001 From: Dimas Setia Pambudi Date: Tue, 11 Aug 2026 23:25:44 +0700 Subject: [PATCH] fix(monitor): fix display mirroring recovery and UI state (#6457) * fix(monitor): prevent mirror toggle deletion during recovery and fix UI state * Assert the external monitor helper counts mirrors as active The helper now asks `hyprctl monitors all -j`, so the test that pinned it to plain `monitors` failed. A mirrored external is absent from plain `monitors`, which reads as a disconnect and hands the mirror toggle to recovery. Co-Authored-By: Claude Opus 5 (1M context) * Keep the mirror state line when nothing is mirrored Piping the first mirroring monitor into the branch fed jq's test() a null whenever no output mirrored, and jq aborts there rather than falling back to "". The panel reads this output by line, so the missing line shifted the focused monitor, the scale, and the display list up one, and left mirroring reading as on whenever the external display had focus. Select first and branch inside the pipeline, so the branch only ever sees a monitor and the empty case falls to "" as the lines around it do. Co-Authored-By: Claude Opus 5 (1M context) * Cover the monitor panel state the shell reads by line Nothing exercised omarchy-monitor-state, so both the mirror direction it reported and the jq that reported it went unguarded. The panel reads the output by line index, where a helper dying mid-script costs a line and shifts every field below it into the wrong property without failing. Assert the line count alongside the fields, over extended, mirrored both directions, and clamshelled displays. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: David Heinemeier Hansson Co-authored-by: Claude Opus 5 (1M context) --- bin/omarchy-hyprland-monitor-external-active | 2 +- bin/omarchy-monitor-state | 2 +- test/shell.d/monitor-recovery-test.sh | 6 +- test/shell.d/monitor-state-test.sh | 122 +++++++++++++++++++ 4 files changed, 128 insertions(+), 4 deletions(-) create mode 100755 test/shell.d/monitor-state-test.sh diff --git a/bin/omarchy-hyprland-monitor-external-active b/bin/omarchy-hyprland-monitor-external-active index 1760e981..7fde41a5 100755 --- a/bin/omarchy-hyprland-monitor-external-active +++ b/bin/omarchy-hyprland-monitor-external-active @@ -3,4 +3,4 @@ # omarchy:summary=Returns true when Hyprland has an active external monitor # omarchy:hidden=true -hyprctl monitors -j | jq -e '.[] | select(.name | test("^(eDP|LVDS|DSI)-") | not) | select(.disabled == false)' >/dev/null 2>&1 +hyprctl monitors all -j | jq -e '.[] | select(.name | test("^(eDP|LVDS|DSI)-") | not) | select(.disabled == false)' >/dev/null 2>&1 diff --git a/bin/omarchy-monitor-state b/bin/omarchy-monitor-state index f4dcd05c..fe0e72f4 100755 --- a/bin/omarchy-monitor-state +++ b/bin/omarchy-monitor-state @@ -13,7 +13,7 @@ printf '%s\n' "$monitors_json" | jq -r ' ([.[] | select(.name | internal)][0].name // ""), ([.[] | select((.name | internal) | not)][0].name // ""), ([.[] | select((.name | internal) and .disabled != true)][0].name // ""), - ([.[] | select((.name | internal) and .mirrorOf != "none")][0].mirrorOf // "") + ([.[] | select(.mirrorOf != "none") | if (.name | internal) then .mirrorOf else .name end][0] // "") ' printf '%s\n' "$focused_monitor" diff --git a/test/shell.d/monitor-recovery-test.sh b/test/shell.d/monitor-recovery-test.sh index 52a0863a..3401abc1 100755 --- a/test/shell.d/monitor-recovery-test.sh +++ b/test/shell.d/monitor-recovery-test.sh @@ -63,10 +63,12 @@ grep -F 'omarchy-hw-laptop-closed && omarchy-hw-external-monitors' "$hw_clamshel grep -F '/proc/acpi/button/lid/*/state' "$hw_laptop_closed" >/dev/null pass "clamshell helper detects closed-lid external monitor state" -grep -F 'hyprctl monitors -j' "$monitor_external_active" >/dev/null +# A mirrored external is absent from plain `monitors`, so asking without `all` +# reads as a disconnect and hands the mirror toggle straight to recovery. +grep -F 'hyprctl monitors all -j' "$monitor_external_active" >/dev/null grep -F 'select(.name | test("^(eDP|LVDS|DSI)-") | not)' "$monitor_external_active" >/dev/null grep -F 'select(.disabled == false)' "$monitor_external_active" >/dev/null -pass "active external monitor helper checks Hyprland outputs" +pass "active external monitor helper sees mirrors and ignores monitors disabled on purpose" grep -F 'omarchy-hyprland-monitor-internal recover >/dev/null 2>&1 || true' "$clamshell" >/dev/null grep -F 'omarchy-hyprland-monitor-internal-mirror recover >/dev/null 2>&1 || true' "$clamshell" >/dev/null diff --git a/test/shell.d/monitor-state-test.sh b/test/shell.d/monitor-state-test.sh new file mode 100755 index 00000000..efc1a52e --- /dev/null +++ b/test/shell.d/monitor-state-test.sh @@ -0,0 +1,122 @@ +#!/bin/bash + +source "$(dirname "$0")/base-test.sh" + +test_bin=$(mktemp -d) +monitors_file=$(mktemp) + +cleanup() { + rm -rf "$test_bin" + rm -f "$monitors_file" +} +trap cleanup EXIT + +cat >"$test_bin/hyprctl" <<'EOF' +#!/bin/bash +[[ $* == "monitors all -j" ]] || exit 1 +cat "$FAKE_MONITORS" +EOF + +cat >"$test_bin/omarchy-brightness-display" <<'EOF' +#!/bin/bash +echo 42 +EOF + +cat >"$test_bin/omarchy-hyprland-monitor-focused" <<'EOF' +#!/bin/bash +echo FOCUSED +EOF + +cat >"$test_bin/omarchy-hyprland-monitor-scaling" <<'EOF' +#!/bin/bash +echo 1.5 +EOF + +chmod +x "$test_bin"/* + +# The panel reads this output by line index, so every case has to answer with +# the same number of lines. A helper that dies mid-script drops its line and +# silently shifts every field below it into the wrong property. +state_lines=() +monitor_state() { + printf '%s\n' "$1" >"$monitors_file" + + mapfile -t state_lines < <( + FAKE_MONITORS="$monitors_file" PATH="$test_bin:$PATH" \ + bash "$ROOT/bin/omarchy-monitor-state" 2>/dev/null + ) +} + +assert_line() { + local index="$1" expected="$2" description="$3" + + [[ ${state_lines[index]-} == "$expected" ]] || + fail "$description" "line $index expected: $expected"$'\n'"line $index actual: ${state_lines[index]-}" +} + +assert_line_count() { + local description="$1" + + (( ${#state_lines[@]} == 8 )) || + fail "$description" "expected 8 lines, got ${#state_lines[@]}" +} + +extended='[ + { "name": "eDP-1", "mirrorOf": "none", "disabled": false, "focused": false, "width": 1920, "height": 1080 }, + { "name": "DP-1", "mirrorOf": "none", "disabled": false, "focused": true, "width": 2560, "height": 1440 } +]' + +# Omarchy mirrors by pointing the external at the internal, so `mirrorOf` lands +# on the external and the internal keeps saying "none". +mirrored='[ + { "name": "eDP-1", "mirrorOf": "none", "disabled": false, "focused": true, "width": 1920, "height": 1080 }, + { "name": "DP-1", "mirrorOf": "eDP-1", "disabled": false, "focused": false, "width": 1920, "height": 1080 } +]' + +# A monitors.lua of the user's own can mirror the other way instead. +reverse_mirrored='[ + { "name": "eDP-1", "mirrorOf": "DP-1", "disabled": false, "focused": false, "width": 2560, "height": 1440 }, + { "name": "DP-1", "mirrorOf": "none", "disabled": false, "focused": true, "width": 2560, "height": 1440 } +]' + +clamshell='[ + { "name": "eDP-1", "mirrorOf": "none", "disabled": true, "focused": false, "width": 0, "height": 0 }, + { "name": "DP-1", "mirrorOf": "none", "disabled": false, "focused": true, "width": 2560, "height": 1440 } +]' + +monitor_state "$extended" +assert_line_count "monitor state answers every line while extended" +assert_line 0 42 "monitor state reports brightness" +assert_line 1 eDP-1 "monitor state names the internal monitor" +assert_line 2 DP-1 "monitor state names the external monitor" +assert_line 3 eDP-1 "monitor state reports the internal monitor enabled" +assert_line 4 "" "monitor state reports no mirror while extended" +assert_line 5 FOCUSED "monitor state reports the focused monitor" +assert_line 6 1.5 "monitor state reports the scale" +pass "monitor state keeps its lines aligned when nothing is mirrored" + +monitor_state "$mirrored" +assert_line_count "monitor state answers every line while mirroring" +assert_line 4 DP-1 "monitor state names the mirroring external monitor" +assert_line 5 FOCUSED "monitor state still reports the focused monitor while mirroring" +pass "monitor state reports the external monitor when it mirrors the internal" + +monitor_state "$reverse_mirrored" +assert_line_count "monitor state answers every line while mirroring in reverse" +assert_line 4 DP-1 "monitor state names the external monitor either way round" +pass "monitor state reports the external monitor when the internal mirrors it" + +monitor_state "$clamshell" +assert_line_count "monitor state answers every line while clamshelled" +assert_line 1 eDP-1 "monitor state still names a disabled internal monitor" +assert_line 3 "" "monitor state reports the internal monitor disabled" +assert_line 4 "" "monitor state reports no mirror while clamshelled" +pass "monitor state separates a disabled internal monitor from a missing one" + +monitor_state "$extended" +[[ ${state_lines[7]-} == '[{"name":"eDP-1","enabled":true,"focused":false,"width":1920,"height":1080},{"name":"DP-1","enabled":true,"focused":true,"width":2560,"height":1440}]' ]] || + fail "monitor state lists every display for the panel" "actual: ${state_lines[7]-}" +monitor_state "$clamshell" +[[ ${state_lines[7]-} == '[{"name":"eDP-1","enabled":false,"focused":false,"width":0,"height":0},{"name":"DP-1","enabled":true,"focused":true,"width":2560,"height":1440}]' ]] || + fail "monitor state lists every display for the panel" "actual: ${state_lines[7]-}" +pass "monitor state lists every display with its enabled and focused state"