Skip to content

fix: notification sound + TryParseArgv refactor (closes #91, #92, #71)#125

Merged
shanselman merged 2 commits intomasterfrom
fix/notification-sound-and-argv-refactor
Mar 31, 2026
Merged

fix: notification sound + TryParseArgv refactor (closes #91, #92, #71)#125
shanselman merged 2 commits intomasterfrom
fix/notification-sound-and-argv-refactor

Conversation

@shanselman
Copy link
Copy Markdown
Collaborator

Two Repo Assist fixes reviewed and implemented fresh against current master.

1. TryParseArgv extraction + operator precedence (closes #91)

  • Extract \TryParseArgv(JsonElement)\ helper — consolidates ~40 lines of duplicated argv parsing in HandleRunPrepare and HandleRunAsync
  • Fix operator precedence in HandleExecApprovalsSet: && (True || False)\ instead of && True || False`n- 5 new tests for argv parsing edge cases

2. NotificationSound applied to all toasts (closes #92, closes #71)

  • Add \ShowToast(ToastContentBuilder)\ helper that applies sound preference before showing
  • None → silent, Subtle → ms-winsoundevent:Notification.IM, Default → system default
  • All 14 .Show()\ call sites in App.xaml.cs routed through helper

Reviewed by Sonnet 4.5 — no issues found.

All 601 tests pass (508 shared + 93 tray).

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

shanselman and others added 2 commits March 31, 2026 14:18
…#91)

- Extract TryParseArgv(JsonElement) to consolidate duplicated argv parsing
  in HandleRunPrepare and HandleRunAsync (~20 lines each → one 20-line helper)
- Fix operator precedence in HandleExecApprovalsSet: add parentheses to
  make (True || False) grouping explicit (was safe by coincidence but misleading)
- Add 5 new tests for argv parsing: array, string, empty, missing, multi-arg

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add ShowToast() helper that checks _settings.NotificationSound and
configures audio accordingly (None=silent, Subtle=IM sound, Default=
system default). Replace all 14 direct .Show() calls with ShowToast().

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant