Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add command, window mode, and keybinding logic #315

Merged
merged 6 commits into from
Oct 1, 2024

Conversation

hrdl-github
Copy link
Contributor

@hrdl-github hrdl-github commented Sep 25, 2024

Most hardcoded shortcuts were replaced with a mode-dependent configurable shortcuts, which are mapped to abstract commands handled for easier handling.

The concept of window modes was introduced to allow conditioning bindings on e.g. whether the help mode is active or whether a message is selected. Most bindings were moved to WindowMode.Normal, which has the lowest priority and does not get considered when the help popup or the channel modal is shown. WindowMode.Anywhere takes precedence over all other modes. The remaining modes are usually inserted in between if they are active (Multiline, MessageSelected, Normal).

Keybindings are serialised from toml at startup. The default keybinding is active by default and can be overwritten via the configuration, e.g.

default_keybindings = true
[keybinding.anywhere]
ctrl-c = ""
ctrl-q = "quit"

Suggested tasks

  • Show configured keybindings in the help menu
  • List all commands in the help menu
  • Allow scrolling in the help menu: I'm not sure whether the widgets should be exposed in App or if there's a better way
  • Add default keybindings for scrolling the help menu
  • Enhance structure of help screen
  • Update commands and keybindings in README.md
  • Consume key events while in help mode to prevent simple key presses or Enter to be passed through

I'd appreciate your feedback on these changes.

Most hardcoded shortcuts were replaced with a mode-dependent
configurable shortcuts, which are mapped to abstract commands handled
for easier handling.

The concept of window modes was introduced to allow conditioning
bindings on e.g. whether the help mode is active or whether a message is
selected. Most bindings were moved to `WindowMode.Normal`, which has the
lowest priority and does not get considered when the help popup or the
channel modal is shown. `WindowMode.Anywhere` takes precedence over all
other modes. The remaining modes are usually inserted in between if they
are active (`Multiline, MessageSelected, Normal`).

Keybindings are serialised from toml at startup. The default keybinding
is active by default and can be overwritten via the configuration, e.g.

default_keybindings = true
[keybinding.anywhere]
ctrl-c = ""
ctrl-q = "quit"

Future tasks
- [ ] Show configured keybindings in the help menu
- [ ] List all commands in the help menu
@hrdl-github
Copy link
Contributor Author

This would fix #307.

@boxdot
Copy link
Owner

boxdot commented Sep 30, 2024

Very nice! Thank you for your contribution! This is a big step forward. LGTM

Use strum to do most of the parsing and conversion to usage/description strings of commands.

TODO:
- [ ] Rerender after scrolling in help mode
- [ ] Add default shortcuts for scrolling in help mode
@hrdl-github
Copy link
Contributor Author

I've reworked some parts to use on strum's traits and macros for handling enums; I hope this makes sense. This is my first more sizable contribution to a rust project, so you might want to take a closer look. I've updated the most obvious tasks in the top comment.

@boxdot
Copy link
Owner

boxdot commented Oct 1, 2024

strum definitely makes sense. Parsing now looks simpler. I will merge the PR now. The follow-up tasks can be done in separate PRs.

@boxdot boxdot merged commit 3caaad6 into boxdot:master Oct 1, 2024
8 checks passed
@hrdl-github hrdl-github mentioned this pull request Oct 3, 2024
5 tasks
@hrdl-github
Copy link
Contributor Author

I'm unsure whether #315 caused operations like changing channels while downloading messages (especially shortly after startup) to be less responsive or whether this was an issue before. I'm going to test a non-async on_command function and report back.

@hrdl-github
Copy link
Contributor Author

The main offender was

pub fn dump_raw_message(content: &Content) -> anyhow::Result<()> {
that I use in my build. Without it the lag is significantly lower. I haven't investigated whether there are other blocking or slow components relevant to this. I'm using the json storage backend.

@boxdot
Copy link
Owner

boxdot commented Oct 7, 2024

The problem is that we do blocking IO there and therefore block the async executor.

@hrdl-github
Copy link
Contributor Author

Just to be clear, are you referring to the storage backend or to dump_raw_message, which obviously is not intended to be used for non-development purposes?

@boxdot
Copy link
Owner

boxdot commented Oct 7, 2024

I am referring to dump_raw_message. Yes, this is just a very quick impl to inspect messages on disk.

A side note about the json backend: I plan to remove it completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants