fix: use exact match with pkill to terminate Spotify #3543
Conversation
📝 WalkthroughWalkthroughUse exact-name matching for process detection/termination in Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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")
|
I'm not going to review it until you look into coderabbit's suggestions |
|
@rxri can you check it out rn |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/cmd/restart.go`:
- Around line 26-31: The macOS branch in restart.go uses lowercase "spotify", so
update the two exec.Command calls in the case "darwin" block (the
exec.Command("pgrep", "-x", "spotify") and exec.Command("pkill", "-x",
"spotify").Run()) to use the correct process name "Spotify" (capital S) so
pgrep/pkill will match and terminate the running app.
🧹 Nitpick comments (1)
src/cmd/restart.go (1)
20-31: Consider consolidating duplicate platform logic.The
linuxanddarwincases now have nearly identical logic (the only difference should be the process name casing). Once the darwin case is fixed to use the correct capitalization, consider whether this duplication is intentional or could be consolidated.♻️ Optional consolidation (after fixing the darwin case)
case "linux": isRunning := exec.Command("pgrep", "-x", "spotify") _, err := isRunning.Output() if err == nil { exec.Command("pkill", "-x", "spotify").Run() } case "darwin": - isRunning := exec.Command("pgrep", "-x", "spotify") - _, err := isRunning.Output() + isRunning := exec.Command("pgrep", "-x", "Spotify") + _, err := isRunning.Output() if err == nil { - exec.Command("pkill", "-x", "spotify").Run() + exec.Command("pkill", "-x", "Spotify").Run() }Alternatively, if both platforms should use the same logic, a helper function could reduce duplication—but keeping them separate is also reasonable for platform-specific flexibility.
src/cmd/restart.go
Outdated
| case "darwin": | ||
| isRunning := exec.Command("sh", "-c", "ps aux | grep 'Spotify' | grep -v grep") | ||
| _, err := isRunning.CombinedOutput() | ||
| isRunning := exec.Command("pgrep", "-x", "Spotify") |
There was a problem hiding this comment.
pgrep isn't natively available on macOS. You have to use change the syntax of previous cmd
There was a problem hiding this comment.
woops then the old way was probably good, am updating it rn
2 participants
Use exact match as it now kills apps like spotify_player
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.