[PR #629] [MERGED] Allow changing of volume in different steps #1759

New issue

Closed

opened 2026-03-14 15:31:32 +03:00 by kerem · 0 comments

kerem commented

2026-03-14 15:31:32 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/aome510/spotify-player/pull/629
Author: @DOD-101
Created: 12/8/2024
Status: ✅ Merged
Merged: 12/8/2024
Merged by: @aome510

Base: master ← Head: add-616

📝 Commits (4)

abc43b3 feat(command)!: replace Volume -Up / -Down with VolumeChange
1760669 docs(command): update docs for new VolumeChange command
c74ec81 refactor(clippy): change format macro to pass clippy tests
3dd30f4 fix(code-review): apply changes from code-review

📊 Changes

5 files changed (+18 additions, -19 deletions)

View changed files

📝 README.md (+1 -2)
📝 docs/config.md (+3 -0)
📝 spotify_player/src/command.rs (+10 -5)
📝 spotify_player/src/config/keymap.rs (+2 -2)
📝 spotify_player/src/event/mod.rs (+2 -10)

📄 Description

Resolves #616

This change implements what was requested in #616, but would require users to re-write their keymap.toml.

The new format for keymap.tomlwould be:

[[keymaps]]
key_sequence = "+"
command = { type = "VolumeChange", args = { offset = 1 } }

[[keymaps]]
key_sequence = "-"
command = { type = "VolumeChange", args = { offset = -1 } }

[[keymaps]]
key_sequence = "n"
command = { type = "NextTrack" }

I'm looking for input on whether this is a good idea to implement this way. Or if an alternative would be better.

There is also the option of leaving the configuration syntax as is and setting the offset like this:

[[keymaps]]
key_sequence = "+"
command = "VolumeChange 1"

And this approach has some advantages and disadvantages.

For one, the config format for the other commands wouldn't change, but as far as I can tell I would have to write some custom deserialization logic, since TOML doesn't support this syntax out of the box. And any future commands, which have arguments, would need to do the same.

Personally, I don't have a preference either way.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/aome510/spotify-player/pull/629 **Author:** [@DOD-101](https://github.com/DOD-101) **Created:** 12/8/2024 **Status:** ✅ Merged **Merged:** 12/8/2024 **Merged by:** [@aome510](https://github.com/aome510) **Base:** `master` ← **Head:** `add-616` --- ### 📝 Commits (4) - [`abc43b3`](https://github.com/aome510/spotify-player/commit/abc43b3237fda08d2d25ccb50ab7a4f44c00d1f3) feat(command)!: replace Volume -Up / -Down with VolumeChange - [`1760669`](https://github.com/aome510/spotify-player/commit/1760669423d3e4606492ee803875e6638a69f54c) docs(command): update docs for new VolumeChange command - [`c74ec81`](https://github.com/aome510/spotify-player/commit/c74ec81a040c056c3a363a8691eaed3950f1d6be) refactor(clippy): change format macro to pass clippy tests - [`3dd30f4`](https://github.com/aome510/spotify-player/commit/3dd30f44c5436884be203b3be723f053f94be542) fix(code-review): apply changes from code-review ### 📊 Changes **5 files changed** (+18 additions, -19 deletions) <details> <summary>View changed files</summary> 📝 `README.md` (+1 -2) 📝 `docs/config.md` (+3 -0) 📝 `spotify_player/src/command.rs` (+10 -5) 📝 `spotify_player/src/config/keymap.rs` (+2 -2) 📝 `spotify_player/src/event/mod.rs` (+2 -10) </details> ### 📄 Description Resolves #616 This change implements what was requested in #616, but would require users to re-write their `keymap.toml`. The new format for `keymap.toml`would be: ```toml [[keymaps]] key_sequence = "+" command = { type = "VolumeChange", args = { offset = 1 } } [[keymaps]] key_sequence = "-" command = { type = "VolumeChange", args = { offset = -1 } } [[keymaps]] key_sequence = "n" command = { type = "NextTrack" } ``` I'm looking for input on whether this is a good idea to implement this way. Or if an alternative would be better. There is also the option of leaving the configuration syntax as is and setting the offset like this: ```toml [[keymaps]] key_sequence = "+" command = "VolumeChange 1" ``` And this approach has some advantages and disadvantages. For one, the config format for the other commands wouldn't change, but as far as I can tell I would have to write some custom deserialization logic, since TOML doesn't support this syntax out of the box. And any future commands, which have arguments, would need to do the same. Personally, I don't have a preference either way. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>