feat: wl-togglesink to mute/unmute active-window.#1598
Open
IvyProtocol wants to merge 14 commits intoHyDE-Project:devfrom
Open
feat: wl-togglesink to mute/unmute active-window.#1598IvyProtocol wants to merge 14 commits intoHyDE-Project:devfrom
IvyProtocol wants to merge 14 commits intoHyDE-Project:devfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Hyprland keybinding (Shift+H) and a new Bash script that locates the focused Hyprland window, matches its PulseAudio sink inputs, toggles their mute state, and emits a desktop notification. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Hyprland as "Hyprland\n(hyprctl)"
participant Script as "wl-togglesink.sh"
participant PulseAudio as "PulseAudio\n(pactl)"
participant Notify as "Notification\nDaemon"
User->>Hyprland: Press Shift+H
Hyprland->>Script: Launch script
Script->>Script: Check dependencies
Script->>Hyprland: Query focused window (PID, class, title)
Hyprland-->>Script: Return window metadata
Script->>Script: Resolve descendant PIDs
Script->>PulseAudio: List sink inputs (pactl -f json)
PulseAudio-->>Script: Return sink inputs
Script->>Script: Match sink inputs to focused window (PID/name/title, pgrep fallback)
Script->>PulseAudio: Toggle mute/unmute on matched sink inputs
PulseAudio-->>Script: Confirm changes
Script->>Notify: Send notification with resulting state
Notify-->>User: Display mute status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
Configs/.local/lib/hyde/wl-togglesink.sh (2)
95-100: Notification shows default sink name, not the sink the app is actually using.
pamixer --get-default-sinkreturns the system default sink, which may differ from the sink the matched application is actually outputting to. This could confuse the user (e.g., app outputs to a USB headset but the notification shows "Built-in Audio Analog Stereo"). Consider extracting the sink name from the already-availablesink_jsoninstead.Also, typo on line 95: "paxmier" → "pamixer".
Extract sink name from sink_json
You already have
sink_jsonand the matchedsink_ids. You could pull the sink name from the sink-input's properties (e.g.,properties["media.name"]) or resolve the actual sink from the sink-input'ssinkfield, avoiding the pamixer dependency entirely for this notification.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Configs/.local/lib/hyde/wl-togglesink.sh` around lines 95 - 100, The notification is showing the system default sink via pamixer instead of the actual sink the app uses; replace the pamixer call with extracting the real sink name from the existing sink_json/sink_ids (either use the sink-input's "sink" field to resolve the sink id -> name or read properties["media.name"] from the matched sink-input) and pass that string into notify-send in place of the pamixer output; also fix the typo "paxmier" to "pamixer" if you keep any mention of it.
33-48:containscan produce false-positive sink matches.The jq
containsfilter is a substring match, so a window class like"fire"would match anapplication.nameof"firefox", and vice-versa. This is noted as a known issue in the PR description, but worth keeping in mind: a stricter equality or anchored-regex check (test("^" + ... + "$")) would reduce accidental cross-matches.Additionally, if
__classor__titleis"null"(string, from jq-ron a missing field),contains("null")could spuriously match unrelated properties.Guard against "null" class/title leaking into matches
Add early validation after line 26 to treat
"null"class/title the same as empty:IFS=$'\t' read -r __pid __class __title <<< "${PID}" +[[ "${__class}" == "null" ]] && __class="" +[[ "${__title}" == "null" ]] && __title=""And in the jq filter, skip
containschecks when the variable is empty:select( (.properties["application.process.id"] // "") == $pid or - (lc(.properties["application.name"]) | contains(lc($class))) + ($class != "" and (lc(.properties["application.name"]) | contains(lc($class))))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Configs/.local/lib/hyde/wl-togglesink.sh` around lines 33 - 48, The jq contains() checks are producing false positives and also match the literal string "null" when __class/__title come from jq -r; update the script to normalize/guard inputs and tighten matching: after you set __class and __title (the vars used in mapfile sink_ids), treat the string "null" as empty (e.g., if [ "${__class}" = "null" ] || [ -z "${__class}" ] then __class=""; same for __title) and then change the jq predicate to skip substring checks when the variable is empty and use anchored regex or exact equality instead of contains (e.g., replace (lc(.properties["application.name"]) | contains(lc($class))) with ($class != "" and (lc(.properties["application.name"]) | test("^" + lc($class) + "$"))) and similarly for application.id, application.process.binary and the normalized media.name check) so you avoid inadvertent substring matches and "null" leaks while still matching intended sinks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Configs/.config/hypr/keybindings.conf`:
- Line 130: The keybinding uses "hyde-shell wl-togglesink.sh" which will fail
because hyde-shell's run_command() auto-appends ".sh" (so it would look for
wl-togglesink.sh.sh); update the binding to call the script name without the
extension (use hyde-shell wl-togglesink) to match other invocations and let
run_command() resolve the .sh suffix; locate the binding line referencing
wl-togglesink in keybindings.conf and remove the ".sh" suffix.
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 23-24: The command substitutions for active_json and PID (using
hyprctl and jq) capture stdout so their inline echo error messages never reach
the user; update these calls to either redirect the diagnostic echoes to stderr
(echo ... >&2) inside the failure blocks or, better, move the "|| { echo ...;
exit 1; }" logic outside the $(...) so the subshell does not capture the
message—apply this change for the active_json assignment (hyprctl -j
activewindow) and the PID assignment (jq -r '"\(.pid)\t\(.class)\t\(.title)"')
and the other similar occurrence noted in the comment.
- Around line 89-93: If the swayIcon path is missing, avoid passing that broken
path to notify-send by unsetting or replacing swayIcon with a known fallback
(e.g., a generic icon name) before the notify-send calls so the -i argument is
always valid; in the sink loop, make pactl set-sink-input-mute calls resilient
to individual failures by catching errors (e.g., append || true or accumulate
failures in a variable) so one failing pactl does not abort the loop under set
-e and then report a single consolidated error/exit code after the loop;
reference swayIcon, the notify-send invocations, the sink_ids array and the
pactl set-sink-input-mute invocation to locate edits.
- Around line 26-29: The PID extraction currently only checks -z for __pid and
doesn't handle hyprctl returning -1 or jq producing the literal "null", and the
separate unset IFS is unnecessary; update the validation after the read (the
IFS=$'\t' read -r __pid __class __title <<< "${PID}" and subsequent check) to
treat "__pid" as invalid if it's empty, "-1", or "null" (string), e.g.
explicitly test [[ -z "${__pid}" || "${__pid}" = "-1" || "${__pid}" = "null" ]]
and exit with the same error, and remove the redundant "unset IFS" since the IFS
was only set for the single read.
---
Nitpick comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 95-100: The notification is showing the system default sink via
pamixer instead of the actual sink the app uses; replace the pamixer call with
extracting the real sink name from the existing sink_json/sink_ids (either use
the sink-input's "sink" field to resolve the sink id -> name or read
properties["media.name"] from the matched sink-input) and pass that string into
notify-send in place of the pamixer output; also fix the typo "paxmier" to
"pamixer" if you keep any mention of it.
- Around line 33-48: The jq contains() checks are producing false positives and
also match the literal string "null" when __class/__title come from jq -r;
update the script to normalize/guard inputs and tighten matching: after you set
__class and __title (the vars used in mapfile sink_ids), treat the string "null"
as empty (e.g., if [ "${__class}" = "null" ] || [ -z "${__class}" ] then
__class=""; same for __title) and then change the jq predicate to skip substring
checks when the variable is empty and use anchored regex or exact equality
instead of contains (e.g., replace (lc(.properties["application.name"]) |
contains(lc($class))) with ($class != "" and
(lc(.properties["application.name"]) | test("^" + lc($class) + "$"))) and
similarly for application.id, application.process.binary and the normalized
media.name check) so you avoid inadvertent substring matches and "null" leaks
while still matching intended sinks.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Configs/.local/lib/hyde/wl-togglesink.sh (1)
96-96: Typo in comment: "paxmier" → "pamixer"-#// Append paxmier to get a nice result. Pamixer is complete optional here. +#// Append pamixer to get a nice result. pamixer is completely optional here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Configs/.local/lib/hyde/wl-togglesink.sh` at line 96, Fix the typo in the inline comment: change "paxmier" to "pamixer" in the comment line that reads "#// Append paxmier to get a nice result. Pamixer is complete optional here." so the comment correctly references the pamixer tool (and optionally adjust capitalization to "pamixer" consistently).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 32-47: The jq filter in mapfile sink_ids uses contains(lc($class))
and contains(...lc($title)) which will match everything when $class or $title
are empty; update the select predicate to guard those branches by first checking
the arg is non-empty (e.g. $class != "" and <contains check>) and similarly for
$title (e.g. $title != "" and <normalized contains check>), leaving the other
equality and process checks unchanged; locate the jq expression inside sink_ids
assignment and modify the contains conditions to combine a non-empty check with
the existing contains() calls for __class and __title.
- Around line 17-19: The diagnostic for missing dependencies currently prints to
stdout via the echo in wl-togglesink.sh; change the echo that references the
missing array ("${missing[*]}") so it writes to stderr (redirect to >&2) so the
message is observable when stdout is captured, keeping the existing exit 1
behavior unchanged.
- Around line 90-94: The loop currently uses `|| (( errors++ ))` which triggers
set -e on the first failure and the final check `((errors)) && { ... }`
similarly returns non-zero on success; change the increment to a safe
pre-increment (e.g. `(( ++errors ))`) or use `errors=$((errors+1))` and replace
the final `((errors)) && { ... }` with an explicit `if (( errors )); then ...
fi` test so set -e won't be tripped. Also accumulate failed sink ids (e.g. push
failing `$id` into a `failed_ids` array) and use that array when composing the
error/notify message instead of the loop variable `${id}`, keeping references to
`sink_ids`, `want_mute`, `state_msg`, `dunstDir`, `errors`, and `id` to locate
the relevant code.
---
Nitpick comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Line 96: Fix the typo in the inline comment: change "paxmier" to "pamixer" in
the comment line that reads "#// Append paxmier to get a nice result. Pamixer is
complete optional here." so the comment correctly references the pamixer tool
(and optionally adjust capitalization to "pamixer" consistently).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 90-104: The state notification block (the notify-send calls using
state_msg and __class and optional pamixer output) must only run when all pactl
operations succeeded; change the script to check the errors counter (variable
errors updated in the loop that calls pactl set-sink-input-mute for each id in
sink_ids) and only execute the pamixer-conditional notify-send branch when ((
errors == 0 )); leave the existing error-reporting notify-send for nonzero
errors intact so the user still gets the "Failed to set mute" message.
- Around line 58-67: The echo messages for the "No sink input" branches
currently write to stdout and one contains a typo ("active_window"); modify the
two echo calls in the block that checks sink_ids and HYPRLAND_INSTANCE_SIGNATURE
so they write to stderr (e.g., redirect to >&2 or use >&2-aware logging) and
correct the message to "active window" (ensure the variable ${__class} remains
included); keep the notify-send/exit logic unchanged and update only the echo
destination and text in this conditional.
---
Duplicate comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 17-19: The script currently prints the missing-dependencies
message to stdout; change the echo that outputs "Missing required dependencies:
\"${missing[*]}\"" so it writes to stderr instead (for example, redirect the
echo/printf to >&2) while keeping the existing condition that checks the missing
array and the exit 1; update the line that references the missing array to send
its output to stderr.
- Around line 32-47: The jq selection silently matches when $class or $title are
empty because contains("") is always true; update the filter used in mapfile -t
sink_ids so each contains(...) check is guarded by a non-empty check of the
corresponding variable (e.g., only evaluate (lc(.properties["application.name"])
| contains(lc($class))) if $class != "" and only evaluate
(normalize(lc(.properties["media.name"])) | contains(normalize(lc($title)))) if
$title != ""), keeping the other exact/equality checks unchanged so the jq
expression in the mapfile command only uses contains when the input variable is
non-empty.
Contributor
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 99-104: The success notification block currently always runs even
when pactl operations failed; change the logic around the pamixer notification
(the if command -v pamixer ... else notify-send ...) to only execute when errors
== 0 (or the same success condition used when sending the "Failed to set mute"
notification is false), and update the inline comment above it to fix typos
("pamixer" and "completely"); ensure you reference the same notify-send
invocation(s) and the errors variable so the success message is suppressed
whenever errors > 0.
- Around line 62-66: The two echo statements that print "No sink input for
focused window: ${__class}" and "No sink input for focused active_window
${__class}" should write to stderr instead of stdout and the second message must
fix the typo by changing "active_window" to "active window"; update both echo
invocations to use shell stderr redirection and adjust the second echoed string
accordingly so diagnostics surface correctly and the message reads "No sink
input for focused active window ${__class}".
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Author
|
@coderabbitai resume |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
Configs/.local/lib/hyde/wl-togglesink.sh (1)
33-47: BFS traversal is copy-pasted in the fallback block (lines 71–84) — extract a helper functionThe descendant-PID collection logic at lines 33–47 and lines 71–84 is identical apart from the seed array and output variable names. Extracting it into a function eliminates the duplication and makes future changes safe.
♻️ Proposed refactor
+collect_descendants() { + # Usage: collect_descendants pid1 [pid2 ...]; outputs one PID per line + local -A _seen=() + local _queue=("$@") _pid + while (( ${`#_queue`[@]} )); do + _pid="${_queue[0]}" + _queue=("${_queue[@]:1}") + [[ -n "${_seen[$_pid]:-}" ]] && continue + _seen["$_pid"]=1 + echo "$_pid" + mapfile -t _children < <(pgrep -P "$_pid" || true) + for _child in "${_children[@]}"; do + [[ -n "${_seen[$_child]:-}" ]] || _queue+=("$_child") + done + done +} + `#//` Collect all descendant PIDs for the active window -declare -A seen_pids=() -queue=("${__pid}") -all_pids=() -while ((${`#queue`[@]})); do - pid="${queue[0]}" - queue=("${queue[@]:1}") - [[ -n "${seen_pids[$pid]:-}" ]] && continue - seen_pids["$pid"]=1 - all_pids+=("$pid") - mapfile -t children < <(pgrep -P "$pid" || true) - for child in "${children[@]}"; do - [[ -n "${seen_pids[$child]:-}" ]] || queue+=("$child") - done -done +mapfile -t all_pids < <(collect_descendants "${__pid}") idsJson="$(printf '%s\n' "${all_pids[@]}" | jq -s 'map(tonumber)')"Apply analogously in the fallback block (lines 71–84):
- declare -A seen_fallback=() - queue=("${fallback_pids[@]}") - all_fallback=() - while ((${`#queue`[@]})); do - ... - done + mapfile -t all_fallback < <(collect_descendants "${fallback_pids[@]}") fallbackJson="$(printf '%s\n' "${all_fallback[@]}" | jq -s 'map(tonumber)')"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Configs/.local/lib/hyde/wl-togglesink.sh` around lines 33 - 47, Extract the duplicated BFS descendant-PID traversal into a reusable Bash function (e.g., collect_descendant_pids) that accepts a seed array and an output array via namerefs (declare -n) and reuses the inner logic (seen_pids, queue, all_pids, mapfile -t children < <(pgrep -P "$pid" || true), loop). Replace the two identical blocks (the one that seeds with ("${__pid}") and the fallback block) with calls to this function, passing the original seed array and the desired output array name so existing callers and variable names (queue, all_pids, seen_pids, children) remain scoped inside the helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 5-6: After sourcing globalcontrol.sh, ensure iconsDir is actually
defined before using it to build dunstDir: check the variable (iconsDir)
immediately after source "${scrDir}/globalcontrol.sh" and if it is empty or
unset, either set a sensible default for dunstDir or exit with a clear
diagnostic; update the assignment of dunstDir to rely on this guarded value so
that wl-togglesink.sh does not proceed with an empty/relative path (reference
symbols: scrDir, globalcontrol.sh, iconsDir, dunstDir).
- Around line 26-28: The jq invocation that builds PID from active_json uses a
malformed interpolation for .initialTitle causing jq to emit the literal string
"(.initialTitle)" into PID; update the jq format string to include the missing
backslash so the field is interpolated (change the segment to \t\(.initialTitle)
in the jq expression used to set PID) and then either use the parsed
__initialTitle value where needed or remove the __initialTitle placeholder from
the IFS read (the read into __pid __class __title __initialTitle) to eliminate
dead code.
- Around line 16-22: The script currently calls notify-send inside the
missing-dependencies block without ensuring notify-send itself is available, so
if both pactl and notify-send are missing the script will abort before printing
the diagnostic; update the conditional around the notify-send call (the block
that checks the missing array and the grep -qx "pactl" test) to also check
membership of "notify-send" in the missing array before invoking notify-send
(use the same membership approach used for pactl), e.g., only run notify-send
when "pactl" is missing AND "notify-send" is not in missing, leaving the echo
"Missing required dependencies: \"${missing[*]}\"" and exit 1 to always run
afterward to report failures.
- Around line 51-65: The primary jq invocation that fills sink_ids uses --arg
pid with idsJson (a JSON array), causing $pid to be a string and the equality
check (.properties["application.process.id"] == $pid) to always fail; change
that invocation to pass the PID array as JSON (use --argjson pids or equivalent)
and adjust the jq filter to test membership (e.g., check if
(.properties["application.process.id"] // "" ) is contained in the pids array or
compare index membership) so the BFS-derived idsJson is correctly matched in the
select; update the mapfile call that sets sink_ids and the $pid references
inside that jq script accordingly (keep function names/variables mapfile,
sink_ids, idsJson, $pid/$pids, and the select block intact).
---
Duplicate comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 128-130: The failure message uses the loop variable id (which
holds the last iteration) instead of the failure count; update the messages in
the error handling block (the echo and notify-send lines around the if
((errors)) check) to reference ${errors} and adjust wording for plurality (e.g.,
"Failed to set ${errors} sink input(s) to ${state_msg}" or similar) so the
notification accurately reports how many sink inputs failed rather than the
stale ${id}.
- Around line 98-100: Fix the typo in the error string emitted in
wl-togglesink.sh: change the message that currently reads "No sink input for
focused active_window ${__class}" to use "active window" (two words) so it
prints "No sink input for focused active window ${__class}" when the script
exits with code 1; update the echo call that references __class accordingly.
---
Nitpick comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 33-47: Extract the duplicated BFS descendant-PID traversal into a
reusable Bash function (e.g., collect_descendant_pids) that accepts a seed array
and an output array via namerefs (declare -n) and reuses the inner logic
(seen_pids, queue, all_pids, mapfile -t children < <(pgrep -P "$pid" || true),
loop). Replace the two identical blocks (the one that seeds with ("${__pid}")
and the fallback block) with calls to this function, passing the original seed
array and the desired output array name so existing callers and variable names
(queue, all_pids, seen_pids, children) remain scoped inside the helper.
Contributor
✅ Actions performedReviews resumed. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Configs/.local/lib/hyde/wl-togglesink.sh (1)
33-47: BFS traversal logic is duplicated — extract a helper functionThe descendant-PID collection pattern (lines 33–47 and 71–84) is identical, differing only in the seed array and result variable name. Extracting it into a function eliminates the duplication and makes both call sites easier to maintain.
♻️ Suggested refactor
+# Usage: collect_descendants <result_var_name> <seed_pid...> +collect_descendants() { + local -n _result=$1; shift + local -A _seen=() + local _queue=("$@") _pid + _result=() + while (( ${`#_queue`[@]} )); do + _pid="${_queue[0]}"; _queue=("${_queue[@]:1}") + [[ -n "${_seen[$_pid]:-}" ]] && continue + _seen["$_pid"]=1 + _result+=("$_pid") + mapfile -t _children < <(pgrep -P "$_pid" || true) + for _child in "${_children[@]}"; do + [[ -n "${_seen[$_child]:-}" ]] || _queue+=("$_child") + done + done +} + -declare -A seen_pids=() -queue=("${__pid}") -all_pids=() -while ((${`#queue`[@]})); do - pid="${queue[0]}" - queue=("${queue[@]:1}") - [[ -n "${seen_pids[$pid]:-}" ]] && continue - seen_pids["$pid"]=1 - all_pids+=("$pid") - mapfile -t children < <(pgrep -P "$pid" || true) - for child in "${children[@]}"; do - [[ -n "${seen_pids[$child]:-}" ]] || queue+=("$child") - done -done +collect_descendants all_pids "${__pid}" idsJson="$(printf '%s\n' "${all_pids[@]}" | jq -s 'map(tonumber)')"- declare -A seen_fallback=() - queue=("${fallback_pids[@]}") - all_fallback=() - while ((${`#queue`[@]})); do - pid="${queue[0]}" - queue=("${queue[@]:1}") - [[ -n "${seen_fallback[$pid]:-}" ]] && continue - seen_fallback["$pid"]=1 - all_fallback+=("$pid") - mapfile -t children < <(pgrep -P "$pid" || true) - for child in "${children[@]}"; do - [[ -n "${seen_fallback[$child]:-}" ]] || queue+=("$child") - done - done + collect_descendants all_fallback "${fallback_pids[@]}" fallbackJson="$(printf '%s\n' "${all_fallback[@]}" | jq -s 'map(tonumber)')"Also applies to: 71-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Configs/.local/lib/hyde/wl-togglesink.sh` around lines 33 - 47, Extract the repeated BFS descendant-PID traversal into a helper function such as collect_descendant_pids that accepts a seed PID (or seed array) and returns (or populates) an array of all descendant PIDs; move the logic that declares seen_pids, queue, mapfile < <(pgrep -P "$pid" || true), the loop that pops queue and appends children, and the all_pids accumulation into that function, and replace both inline copies (the blocks using __pid and the other seed) with calls to collect_descendant_pids to get the result into the original result variables (e.g., all_pids or another_named_array). Ensure the function preserves the same behavior (use of associative seen_pids, pgrep -P, and preserving order) and returns values in a way shell callers can read (e.g., echo lines or populate a global/nameref).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 133-134: The notify-send command is using a command substitution
of pamixer --get-default-sink which can exit non‑zero and, under set -eo
pipefail, abort the whole script; fix by running pamixer --get-default-sink
separately (e.g., capture its output into a variable like default_sink) and
protect the call with stderr redirection and a fallback (use || true or test for
empty output) before calling notify-send, then pass that sanitized variable to
notify-send instead of the direct substitution; update the block around command
-v pamixer and notify-send in wl-togglesink.sh to perform the safe capture and
fallback.
---
Duplicate comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 124-130: The failure message incorrectly reports only the last
loop id because ${id} is overwritten; modify the loop that iterates over
sink_ids to record failed sink ids (e.g., push $id into a failed_ids array when
pactl set-sink-input-mute "$id" "$want_mute" fails) and then, in the if
((errors)) block, reference the joined failed_ids instead of ${id} in both the
echo and notify-send calls (ensure proper quoting of the joined list and keep
variables state_msg, want_mute, errors, sink_ids unchanged).
- Line 99: Update the error message string in wl-togglesink.sh so it reads "No
sink input for focused active window ${__class}" instead of "active_window"
(replace the underscore with a space) to avoid looking like a variable; locate
the echo call that prints "No sink input for focused active_window ${__class}"
and change the literal text only, leaving the ${__class} variable interpolation
untouched.
- Around line 16-22: The script calls notify-send unconditionally when pactl is
missing which can itself be absent and cause the script to abort under set -e;
update the block that checks the missing array so you only invoke notify-send if
it is not missing (i.e., check that "notify-send" is not in the missing[@] list
or use command -v notify-send before calling it). Modify the code around the if
(( ${`#missing`[@]} )); then ... notify-send -a "t1" ... line to first verify
presence of notify-send (or that it is not in missing[@]) and only then call
notify-send; keep the existing echo "Missing required dependencies:
\"${missing[*]}\"" >&2 and exit 1 behavior intact.
- Around line 5-6: The script unconditionally uses iconsDir after sourcing
globalcontrol.sh, so if sourcing fails or iconsDir is unset dunstDir becomes
"/Wallbash-Icon"; modify the startup to verify the source succeeded and that
iconsDir is defined: after attempting to source globalcontrol.sh
(scrDir/globalcontrol.sh) check the return status and/or test [ -n
"${iconsDir:-}" ], and if either check fails either set a sensible fallback for
iconsDir or exit with an explicit error message; then compute
dunstDir="${iconsDir}/Wallbash-Icon" only after those checks to avoid silent
broken paths.
- Around line 26-28: The jq format string currently includes an unescaped
'(.initialTitle)' which never interpolates and makes __initialTitle dead; update
the PID extraction to only output pid, class and title (remove the .initialTitle
field from the jq format string used in PID) and adjust the IFS read to only
populate __pid __class __title (drop __initialTitle), or if you actually need
the initial title later, escape it correctly as '\t\(.initialTitle)' so it
interpolates into PID and keep the corresponding __initialTitle in the read.
- Around line 51-66: The jq call is passing idsJson as a string via --arg so the
equality (.properties["application.process.id"] // "") == $pid always fails;
change the jq invocation to use --argjson pid "${idsJson}" and replace the
equality check with a membership test using index, e.g. replace
(.properties["application.process.id"] // "") == $pid with ($pid |
index((.properties["application.process.id"] // "") | tonumber) != null) so the
numeric PID array (idsJson) is tested for membership; update the jq expression
in the mapfile sink_ids assignment accordingly (referencing idsJson, sink_json,
$pid and the (.properties["application.process.id"] ...) check).
---
Nitpick comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 33-47: Extract the repeated BFS descendant-PID traversal into a
helper function such as collect_descendant_pids that accepts a seed PID (or seed
array) and returns (or populates) an array of all descendant PIDs; move the
logic that declares seen_pids, queue, mapfile < <(pgrep -P "$pid" || true), the
loop that pops queue and appends children, and the all_pids accumulation into
that function, and replace both inline copies (the blocks using __pid and the
other seed) with calls to collect_descendant_pids to get the result into the
original result variables (e.g., all_pids or another_named_array). Ensure the
function preserves the same behavior (use of associative seen_pids, pgrep -P,
and preserving order) and returns values in a way shell callers can read (e.g.,
echo lines or populate a global/nameref).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 69-70: The current logic runs pgrep -f "${__class}" unguarded
which will match all processes when __class is empty; modify the block that
fills fallback_pids so it first checks if __class is non-empty (and optionally
non-whitespace) and only then runs pgrep -f "${__class}"; if __class is empty,
set fallback_pids to an empty array or skip the fallback path to avoid
collecting every PID. Update the conditional that currently uses sink_ids and
fallback_pids so behavior remains correct when fallback_pids is empty.
- Around line 5-7: The guard in wl-togglesink.sh is checking the wrong variable
name: replace the conditional that tests iconDir with one that tests the actual
exported variable iconsDir (set by globalcontrol.sh) so the fallback to
XDG_DATA_HOME/icons only runs when iconsDir is empty; update the check around
the current line that uses [[ -n "${iconDir}" ]] and ensure subsequent use of
iconsDir (e.g., dunstDir="${iconsDir}/Wallbash-Icon") remains consistent with
the corrected variable name.
---
Duplicate comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 99-101: Update the echo message in the else branch so the word
"active_window" is corrected to "active window"; specifically change the echo
invocation that prints "No sink input for focused active_window ${__class}" (and
still redirects to stderr with >&2) to "No sink input for focused active window
${__class}" and keep the subsequent exit 1 unchanged.
- Around line 129-131: The error message uses the loop variable ${id} (which
holds the last iterated sink) instead of the failure count ${errors}; update the
two messages that mention "${id}" and "${state_msg}" (the echo and notify-send
lines) to report the number of failed sinks using ${errors} (e.g., "Failed to
set ${errors} sink(s) to ${state_msg}") so they reflect the actual failure count
rather than the last ${id}.
- Around line 134-135: The notify-send command uses command substitution with
pamixer (pamixer --get-default-sink | awk ...) which can fail under set -eo
pipefail and abort the script; fix by capturing the pamixer output into a
variable first with a safe fallback (e.g., run pamixer --get-default-sink
2>/dev/null | awk ... or capture exit status and use a default like "unknown" or
empty if it fails) and then pass that variable to notify-send (references:
pamixer, notify-send, __initialTitle, state_msg, swayIcon); this ensures a
non-zero pamixer result does not cause the entire script to exit.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 125-143: Remove the duplicated inner "if ((errors))" block so the
outer if/else pairing is balanced (close the outer if with a single fi), and
update the error messages in the remaining error branch to report the number of
failures via the errors variable instead of using ${id} (which only contains the
last loop value). Specifically, edit the post-loop conditional that checks the
errors counter (the if ((errors)) block after the pactl loop and its
corresponding notify-send/echo calls) to remove the duplicate block, ensure
there is exactly one matching fi, and change the echo/notify-send text to
mention "${errors}" failed sink(s) or similar to accurately reflect failures
from the sink_ids loop.
- Around line 52-67: The jq expression in the mapfile filter uses "tonumber? as
$p" which yields empty on parse failure and short-circuits the surrounding
or-chain; change that to "(tonumber? // null) as $p" so failed parses become
null (making "$p != null" evaluate false) and allow the subsequent class/title
branches to run; apply the same replacement in the fallback filter later in the
file (the other jq select that checks .properties["application.process.id"]) so
both occurrences behave consistently.
---
Duplicate comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Line 100: The echoed message contains a typo "active_window"; update the
string in the echo call (the line that prints: echo "No sink input for focused
active_window ${__class}" >&2) to read "active window" instead of
"active_window" so it becomes "No sink input for focused active window
${__class}" to fix the wording.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Configs/.local/lib/hyde/wl-togglesink.sh (1)
134-134: Comment on line 134 has two typos: "paxmier" → "pamixer", "complete optional" → "completely optional".✏️ Proposed fix
- # // Append paxmier to get a nice result. Pamixer is complete optional here. + # // Append pamixer to get a nice result. Pamixer is completely optional here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Configs/.local/lib/hyde/wl-togglesink.sh` at line 134, Fix the typos in the comment string "Append paxmier to get a nice result. Pamixer is complete optional here." by changing "paxmier" to "pamixer" and "complete optional" to "completely optional" so the comment reads "Append pamixer to get a nice result. Pamixer is completely optional here."; locate and update that comment in wl-togglesink.sh where the existing line contains "Append paxmier" to apply the correction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 87-89: The fallback jq invocation that populates sink_ids (the
mapfile -t sink_ids < <( jq -r --argjson pids "${fallbackJson}" '.[] |
select(((.properties["application.process.id"] | tostring | (tonumber? // null))
as $p | $p != null and ($pids | index($p)))) | .index') ) is missing input and
therefore never sees sink_json; fix it by feeding sink_json into that jq call
(same way as the earlier queries and want_mute) so jq reads the sinks (e.g.,
pass <<< "${sink_json}" to the fallback jq invocation), keeping the existing
--argjson pids "${fallbackJson}" and selector logic unchanged.
---
Duplicate comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Line 100: Fix the typo in the error message inside wl-togglesink.sh: change
the echo string that currently reads "No sink input for focused active_window
${__class}" to use a space ("focused active window ${__class}") while keeping
the stderr redirect (>&2) and the variable ${__class} unchanged so the message
reads correctly to users.
- Around line 130-132: The error messages use the loop variable ${id} which ends
up being the last iterated value instead of the actual failing sink IDs; modify
the logic where pactl failures increment errors to also append the current ${id}
to a collection (e.g., failed_ids array or a comma-separated string) and then,
in the final if ((errors)) block, use that collection to report which IDs failed
(and still include the failure count via ${errors} or ${`#failed_ids`[@]}) in both
the echo and notify-send messages; update the references to ${id} in the failure
messages to use the accumulated failed_ids variable so the reported IDs are
correct.
---
Nitpick comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Line 134: Fix the typos in the comment string "Append paxmier to get a nice
result. Pamixer is complete optional here." by changing "paxmier" to "pamixer"
and "complete optional" to "completely optional" so the comment reads "Append
pamixer to get a nice result. Pamixer is completely optional here."; locate and
update that comment in wl-togglesink.sh where the existing line contains "Append
paxmier" to apply the correction.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Configs/.local/lib/hyde/wl-togglesink.sh (2)
34-48: Duplicated BFS logic — consider extracting a helper function.The descendant-PID BFS at lines 34–48 and its near-identical repeat at lines 72–85 differ only in the seed array (
"${__pid}"vs"${fallback_pids[@]}") and the tracking variables (seen_pids/all_pidsvsseen_fallback/all_fallback). Extracting it reduces maintenance surface:♻️ Example refactor
# Collect all descendant PIDs starting from the given seeds. # Usage: collect_descendants <result_array_name> seed1 [seed2 ...] collect_descendants() { local -n _result=$1; shift local -A _seen=() local _queue=("$@") _pid _result=() while (( ${`#_queue`[@]} )); do _pid="${_queue[0]}"; _queue=("${_queue[@]:1}") [[ -n "${_seen[$_pid]:-}" ]] && continue _seen["$_pid"]=1 _result+=("$_pid") local -a _children mapfile -t _children < <(pgrep -P "$_pid" || true) local _c for _c in "${_children[@]}"; do [[ -n "${_seen[$_c]:-}" ]] || _queue+=("$_c") done done } collect_descendants all_pids "${__pid}" # ... later in fallback: collect_descendants all_fallback "${fallback_pids[@]}"Also applies to: 72-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Configs/.local/lib/hyde/wl-togglesink.sh` around lines 34 - 48, Extract the duplicated BFS that gathers descendant PIDs into a reusable helper (e.g., collect_descendants) and replace both blocks that populate seen_pids/all_pids and seen_fallback/all_fallback with calls to that helper; the helper should accept a destination array name and seed PIDs, internally use a local associative array for seen nodes and a local queue to perform the BFS, populate the result array, and be invoked as collect_descendants all_pids "${__pid}" and collect_descendants all_fallback "${fallback_pids[@]}"; keep existing uses of pgrep -P and preserve behavior when no children are found.
134-134: Two typos in the comment:paxmier→pamixer,complete→completely.✏️ Proposed fix
- # // Append paxmier to get a nice result. Pamixer is complete optional here. + # // Append pamixer to get a nice result. Pamixer is completely optional here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Configs/.local/lib/hyde/wl-togglesink.sh` at line 134, Fix the two typos in the comment in wl-togglesink.sh (the comment starting with "# // Append paxmier to get a nice result. Pamixer is complete optional here."): change "paxmier" to "pamixer" and "complete" to "completely" so the comment reads correctly and mentions pamixer as optional.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Line 100: Update the error echo in wl-togglesink.sh so the message uses
"active window" (with a space) instead of "active_window" and add a colon before
the ${__class} variable to match the formatting used on line 97; locate the echo
statement that prints "No sink input for focused active_window ${__class}" and
change it to the corrected phrasing and punctuation.
- Around line 69-70: The code calls pgrep -x "${__class}" inside the mapfile
assignment even when __class may be empty; add a guard that checks __class is
non-empty before invoking pgrep so you don't run pgrep with an empty
pattern—e.g., if __class is non-empty run mapfile -t fallback_pids < <(pgrep -x
"${__class}" || true), otherwise set fallback_pids=() or skip the pgrep branch;
update the logic around the sink_ids and fallback_pids variables in
wl-togglesink.sh to use this guarded path.
- Around line 131-132: The error messages use ${id} (which after the loop is
only the last sink id) so they misreport failures; modify the loop that calls
pactl to increment a failure counter (e.g., failed_count=0 and
failed_count=$((failed_count+1)) on each pactl error) and after the loop use
that counter in the echo and notify-send messages instead of ${id}; optionally
collect failed ids into an array like failed_ids and report either the count
(${failed_count}) or join failed_ids for clearer output, updating references to
failed_count/failed_ids in the error text where "Failed to set" is emitted.
---
Nitpick comments:
In `@Configs/.local/lib/hyde/wl-togglesink.sh`:
- Around line 34-48: Extract the duplicated BFS that gathers descendant PIDs
into a reusable helper (e.g., collect_descendants) and replace both blocks that
populate seen_pids/all_pids and seen_fallback/all_fallback with calls to that
helper; the helper should accept a destination array name and seed PIDs,
internally use a local associative array for seen nodes and a local queue to
perform the BFS, populate the result array, and be invoked as
collect_descendants all_pids "${__pid}" and collect_descendants all_fallback
"${fallback_pids[@]}"; keep existing uses of pgrep -P and preserve behavior when
no children are found.
- Line 134: Fix the two typos in the comment in wl-togglesink.sh (the comment
starting with "# // Append paxmier to get a nice result. Pamixer is complete
optional here."): change "paxmier" to "pamixer" and "complete" to "completely"
so the comment reads correctly and mentions pamixer as optional.
rubiin
changed the title
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Pull Request
Description
Adding wl-togglesink.sh to mute/unmute active-window. (Credits to sl1ng for the original script, rewritten/enhanced by Vyle to be compatible with HyDE-Project)
Dependencies:
Type of change
Please put an
xin the boxes that apply:Checklist
Please put an
xin the boxes that apply:Screenshots
Additional context
Issues:
Specific Application like melonDS would not work with this script due to mismatch of pactl and hyprctl. For melonDS, the PID does not exist and hence it might work.
Any application or package containing spaces or regex naming might cause the script to not parse the PID correctly to $sink_ids, hence I have added support for those regex. Yet, I am not that confident to say that it will work always. There are some caveat to this and it might fail if it is out of reach.
Music Player Daemon does not get muted as hyprctl only recognizes kitty. Might need manual intervention as I don't know how to solve this.
GNOME applications tends to have 'org.gnome.XXXX'. Notify-send would return 'Muted org.gnome.XXXX' which I couldn't solve. (I am tired of bugs but the script is robust now)
Summary by CodeRabbit