fix: use exact match with pkill to terminate Spotify #3543
Conversation
WalkthroughUpdated process termination logic in src/cmd/restart.go: on Linux and Darwin, pkill now uses exact-match (-x) for the Spotify process name; Windows logic unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant RestartCmd as Restart Command
participant SpotifyKill as SpotifyKill()
participant OS as OS Check
participant Proc as pkill
User->>RestartCmd: invoke restart
RestartCmd->>SpotifyKill: terminate Spotify
SpotifyKill->>OS: detect platform
alt Linux/Darwin
OS-->>SpotifyKill: unix-like
SpotifyKill->>Proc: pkill -x "spotify" / "Spotify"
note over Proc,SpotifyKill: Exact-name match required
else Windows
OS-->>SpotifyKill: windows
SpotifyKill->>Proc: taskkill /IM Spotify.exe /F (unchanged)
end
SpotifyKill-->>RestartCmd: result
RestartCmd-->>User: continue restart flow
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cmd/restart.go (1)
20-25: Use exact match with pgrep for consistency.Line 21 uses
pgrep spotifywithout the-xflag, which will match process names likespotify_player. For consistency with the exact-match termination on line 24, and to avoid false positives in the detection logic, add the-xflag topgrepas well.Apply this diff:
- isRunning := exec.Command("pgrep", "spotify") + isRunning := exec.Command("pgrep", "-x", "spotify")
| case "darwin": | ||
| isRunning := exec.Command("sh", "-c", "ps aux | grep 'Spotify' | grep -v grep") | ||
| _, err := isRunning.CombinedOutput() | ||
| if err == nil { | ||
| exec.Command("pkill", "Spotify").Run() | ||
| exec.Command("pkill", "-x", "Spotify").Run() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use pgrep with exact match for consistency.
Line 27 uses ps aux | grep 'Spotify' which will match any process containing "Spotify" in its command line, creating an inconsistency with the exact-match termination on line 30. For consistency with the Linux implementation and to avoid false positives, replace the grep-based detection with pgrep -x Spotify.
Apply this diff:
- isRunning := exec.Command("sh", "-c", "ps aux | grep 'Spotify' | grep -v grep")
- _, err := isRunning.CombinedOutput()
+ isRunning := exec.Command("pgrep", "-x", "Spotify")
+ _, err := isRunning.Output()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case "darwin": | |
| isRunning := exec.Command("sh", "-c", "ps aux | grep 'Spotify' | grep -v grep") | |
| _, err := isRunning.CombinedOutput() | |
| if err == nil { | |
| exec.Command("pkill", "Spotify").Run() | |
| exec.Command("pkill", "-x", "Spotify").Run() | |
| } | |
| case "darwin": | |
| - isRunning := exec.Command("sh", "-c", "ps aux | grep 'Spotify' | grep -v grep") | |
| isRunning := exec.Command("pgrep", "-x", "Spotify") | |
| _, err := isRunning.Output() | |
| if err == nil { | |
| exec.Command("pkill", "-x", "Spotify").Run() | |
| } |
🤖 Prompt for AI Agents
In src/cmd/restart.go around lines 26 to 31, replace the ps+grep detection with
an exact-match pgrep call: run exec.Command("pgrep", "-x",
"Spotify").CombinedOutput() and use its error/result to decide whether to call
exec.Command("pkill", "-x", "Spotify").Run(); this ensures exact-process
matching consistent with the pkill usage and avoids false positives from
substring matches.
Use exact match as it now kills apps like spotify_player
Summary by CodeRabbit