Add vim-like page scrolling

[BlakeJC94]

First pass at implementing page scrolling for list views and help views with <C-d>/<C-u> to scroll down/up by half a page and <C-f>/<C-b> to scroll down/up by a full page.

Addresses the first part of issue #959.

I've confirmed this build works with this in my .config/ncspot/config.toml:

[keybindings]
"Ctrl+f" = "move pagedown 1.0"
"Ctrl+b" = "move pageup 1.0"
"Ctrl+d" = "move pagedown 0.5"
"Ctrl+u" = "move pageup 0.5"

I've been using NCSpot for a while, and this is a feature I personally wanted as well. I was inspired to learn enough Rust so that I could make this contribution! Very open to feedback, let me know if adding this feature to one of the keybinds tables in the README.md would be appreciated.

[ThomasFrans]

Ooh, I would love this feature as well! There is one thing that I'm a bit worried about when adding this, and it's something I also mentioned in my feature request. In Vim, you don't really delete things by accident by using d, and if you do, you can undo that. Here, if your ctrl key isn't working well (like mine is on my laptop atm), pressing ctrl+d several times to find a bunch of tracks gone is slightly worrying...

[BlakeJC94]

Dang, the rest of the feature request makes more sense now!

A suggestion: what if the default key to delete were changed from d to D? It reduces the risk overall for accidental changes to a playlist and addresses this specific concern (and it's still a valid vim key for that action)

EDIT: I just saw that D is already mapped to "remove currently playing item from library", perhaps mapping d/D to x/X could help avoid these mishaps?

[ThomasFrans]

I know I suggested changing the key in the feature request as well but in hindsight, I'm not a big fan of that. I think programs should keep working the same for long time users of the software. Of course if hrkfdn thinks it's best to change the keymap, this issue of d and ctrl+d could be easily solved, but personally I'm not a big fan of changing the behavior, just like some other people that commented/reacted on the feature request. There are other ways to solve this problem, like a configuration option, or providing the extra vim commands as optional by forcing users that want to use them to map them themselves.

[ThomasFrans]

Also quickly wanted to say you forgot to format I think 😉 The workflow would catch this anyway but thought I'd quickly let you know. In Rust, you can use cargo fmt to format all the code automatically, and cargo clippy to test for common errors. These are both tested by the CI. (I couldn't wait to see this PR in action so I cloned it, that's how I noticed 😝 )

[BlakeJC94]

I know I suggested changing the key in the feature request as well but in hindsight, I'm not a big fan of that. I think programs should keep working the same for long time users of the software. Of course if hrkfdn thinks it's best to change the keymap, this issue of d and ctrl+d could be easily solved, but personally I'm not a big fan of changing the behavior, just like some other people that commented/reacted on the feature request. There are other ways to solve this problem, like a configuration option, or providing the extra vim commands as optional by forcing users that want to use them to map them themselves.

True, yeah that sounds like a diplomatic solution. If @hrkfdn would prefer to change the map in a future major release, the groundwork and some good suggestions are here. When I get a bit of time, I'll remove the default keymaps this adds and propose an addition to the readme to make this feature a bit more discoverable 👍

Also quickly wanted to say you forgot to format I think 😉

Haha yep! I had forgotten to enable autoformatting on write for rs files. I didn't know about cargo clippy though, thanks for the tip!

[BlakeJC94]

Alrighty, new default keybinds removed and added move variants to the parser. I'll think about the readme change for a bit more and update this soon

[BlakeJC94]

When updating the readme, I thought of a slightly better interface for these commands, I think I'll try implement a MoveAmount::Float variant instead that accepts a float, modelled on MoveAmount::Integer. This seems a bit cleaner than using "halfpage" and "page" literals in the command interface. I'll try it out and see what it's like to use

[hrkfdn]

Hey there, sorry for the delay. Thanks for the PR and the valuable discussion. FYI: I agree w/ both of you: while the existing binding isn't perfect we probably shouldn't break it.

Merging the PR in a second :)