feat: add macOS click-through commands for notifications#42
feat: add macOS click-through commands for notifications#42mylee04 merged 4 commits intomylee04:mainfrom
Conversation
mylee04
left a comment
There was a problem hiding this comment.
Thanks for putting this together. The direction makes sense, but I don’t think this is ready to merge yet because the two main IDE / embedded-terminal cases still break in practice.
- Custom click-through mappings are ignored when
TERM_PROGRAMis empty.
In get_terminal_bundle_id(), the new config file is only consulted behind [[ -n "$term_prog" ]]. If TERM_PROGRAM is unset or empty, notifier skips the user mapping entirely and falls back to the built-in default (Terminal.app). That means the new feature still does not help one of the main classes of terminals this PR is trying to support.
I reproduced this locally by writing a mapping like:
jb_jediterm=com.jetbrains.PhpStorm
and then invoking the notifier with TERM_PROGRAM=''. It still emitted:
-activate com.apple.Terminal
instead of the configured PhpStorm bundle ID.
click-through addcan save a guessed key instead of the realTERM_PROGRAM.
In the add flow, the current shell TERM_PROGRAM is only used when the query is already a full .app path. If the user follows the issue-thread flow and runs something like:
cn click-through add PhpStorm
from inside an IDE terminal, the code falls back to click_through_guess_term_program(...). So pressing Enter can save a guessed key based on the app name instead of the actual runtime key that notifier will later look up.
I reproduced this with TERM_PROGRAM=JetBrains-JediTerm, where running:
cn click-through add FakeCodex
saved:
fakecodex=com.example.fakecodex
instead of using the live JetBrains-JediTerm value.
Because of those two cases, I don’t think we can say this fully resolves the open PhpStorm / embedded-terminal gap from #19 yet. I’d like to see the lookup and add flow tightened so the stored key and runtime lookup are aligned for real IDE terminal environments before merging.
|
Thanks for the review. I tightened the TERM_PROGRAM handling and also cleaned up the click-through implementation so the resolution path is now explicit. Behavior changes:
I verified this locally with a real PyCharm install:
I do not have PhpStorm installed locally, so I used PyCharm for the real manual check. Since JetBrains IDEs use the same embedded terminal key ( I also expanded test coverage in two layers:
I ran |
mylee04
left a comment
There was a problem hiding this comment.
Thanks for the follow-up here. I rechecked the two cases from my earlier review, and this update addresses them: the notifier now honors configured mappings even when TERM_PROGRAM is empty, and click-through add now prefers the live runtime terminal key instead of saving a guessed app name. The added regression coverage for both cases also helps.
Summary
Add
click-throughcommands on macOS so users can control which app is activated when they click a Code-Notify system notification.What Changed
click-throughcommand group for macOScn click-throughcn click-through add [name]cn click-through removecn click-through resetcn click-through help~/.code-notify/click-through.confclick-throughhelp only on macOSWhy
On macOS, clicking a notification should bring the user back to the app they actually work in.
Default terminal detection works for common cases, but it breaks down once users switch terminals, use editors as terminals, or want a different app to be activated than the built-in fallback.
This adds an explicit configuration path instead of hard-coding one behavior for everyone.
Platform Scope
This PR intentionally limits the feature to macOS because notification click activation is implemented through
terminal-notifierbundle activation.I did not try to generalize this to Linux or Windows here. That likely needs guidance from the maintainer, both for platform-appropriate behavior and for keeping the interface aligned with the rest of the project.
If there is a better cross-platform abstraction or a more idiomatic way to fit this into Code-Notify, I’d be happy to adjust the implementation.
Behavior
click-throughcommandcn click-through addauto-detects the current app and the mapping already exists, it exits early and points the user tocn click-through removeValidation
./scripts/run_tests.sh