Show shuffle/repeat keys in help bar and persist state#2
Conversation
They were only visible in the Ctrl+K keymap overlay but not in the always-visible bottom controls line.
Saves shuffle and repeat preferences to config.toml when toggled via z/r keys, so they survive restarts across all providers.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the user experience by making shuffle and repeat controls more accessible and persistent. Users will now see these options directly in the main interface's help bar and their chosen settings for shuffle and repeat will be remembered between sessions, improving overall usability and convenience. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds discoverability for shuffle and repeat controls and persists their state. The changes look good, but there's an improvement to be made in error handling. Currently, if saving the configuration fails, the error is ignored. I've suggested handling these errors and displaying a status message to the user, which is consistent with how other errors are handled in the application.
ui/keys.go
Outdated
|
|
||
| case "r": | ||
| m.playlist.CycleRepeat() | ||
| _ = config.Save("repeat", fmt.Sprintf("%q", m.playlist.Repeat().String())) |
There was a problem hiding this comment.
It's good practice to handle errors. The config.Save function can fail for various reasons (e.g., file permissions, disk full). Ignoring the error means the user's preference won't be saved, and they won't know why. The application already has a mechanism for displaying status messages to the user (m.saveMsg), which should be used here to report the error.
| _ = config.Save("repeat", fmt.Sprintf("%q", m.playlist.Repeat().String())) | |
| if err := config.Save("repeat", fmt.Sprintf("%q", m.playlist.Repeat().String())); err != nil { | |
| m.saveMsg = fmt.Sprintf("Config save failed: %s", err) | |
| m.saveMsgTTL = 60 // ~3s | |
| } |
ui/keys.go
Outdated
|
|
||
| case "z": | ||
| m.playlist.ToggleShuffle() | ||
| _ = config.Save("shuffle", fmt.Sprintf("%v", m.playlist.Shuffled())) |
There was a problem hiding this comment.
Similar to the repeat setting, the error from config.Save should be handled here. This ensures that if saving the shuffle state fails, the user is notified via the UI status message.
| _ = config.Save("shuffle", fmt.Sprintf("%v", m.playlist.Shuffled())) | |
| if err := config.Save("shuffle", fmt.Sprintf("%v", m.playlist.Shuffled())); err != nil { | |
| m.saveMsg = fmt.Sprintf("Config save failed: %s", err) | |
| m.saveMsgTTL = 60 // ~3s | |
| } |
They were at positions 23-24, below the 12-line visible window. Moved them right after volume controls so they're visible without scrolling.
Each help hint has a priority. When the combined width exceeds the terminal width, the lowest-priority hints are dropped first: 100 Spc(⏯) 95 Q(Quit) 90 <>(Trk) 80 +-(Vol) 70 ←→(Seek) 60 Ctrl+K(Keys) 50 Tab(Focus) 40 /(Search) 30 a(Queue) 20 z(Shfl) 20 r(Rpt) On a narrow terminal you still see play/pause, track nav, vol, and quit. On wide terminals everything shows.
Show a status message via saveMsg when persisting fails instead of silently ignoring the error. Addresses Gemini Code Assist review feedback.
* Switched to using a ReadCloser instead of straight file * Added navidrome client and playlist integration * Removed logging * Switched to a more generic implementation for future providers * Merged with latest changes
Changes
[z] Shfland[r] Rptto the bottom controls line so they're discoverable without opening theCtrl+Kkeymap overlayconfig.tomlon toggle, so preferences survive restartsBoth features are provider-agnostic — shuffle/repeat lives in the
playlistpackage and works identically across local files, Navidrome, and Spotify.