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

feat: support custom key mapping #22

Merged
merged 14 commits into from
Jun 30, 2023
Merged

feat: support custom key mapping #22

merged 14 commits into from
Jun 30, 2023

Conversation

eeeXun
Copy link
Owner

@eeeXun eeeXun commented Jun 18, 2023

No description provided.

@doums
Copy link

doums commented Jun 23, 2023

Hey, so I checked a bit the PR, first thx for it.
As my 2 cents feedback, I'd say it's not ideal as it only allows keys to combine with arbitrary Ctrl modifier. Which is a bit restrictive and I think will be a bit confusing for the user. Usually when user provide their keymap, they expect to provide full combination (key + modifier) as they wish.
But as you said you made this way because of avoiding conflict with default TUI framework bindings, maybe for some other reason?

In any case I think this should appear clearly in the README, like "Only keybinding with Ctrl modifier are supported. Please only provide the key for each custom keybind (Ctrl is always assumed)"

@eeeXun
Copy link
Owner Author

eeeXun commented Jun 23, 2023

Thanks for the feedback.

But as you said you made this way because of avoiding conflict with default TUI framework bindings, maybe for some other reason?

After looking into it. I found it's not conflict. It can be overwritten. Currently, it only provide key with modifier Ctrl and function key from F1 to F64.

I'd say it's not ideal as it only allows keys to combine with arbitrary Ctrl modifier.

Yes, you are right. This is not an idea design. But other modifier, like Alt, is a bit cumbersome to handle. I look the lf, which use the same TUI framework, do the job. I might look into how it is designed.

@eeeXun
Copy link
Owner Author

eeeXun commented Jun 23, 2023

In the last commit

For key to combine with Ctrl, the value can be "C-space", "C-\\", "C-]", "C-^", "C-_" or "C-a" to "C-z".

⚠️ Note, don't use "C-c", <C-c> is for exit program.

For key to combine with Alt, the value can be "A-space" or "A-" + the character you want.

Or you can use function key, the value can be "F1" to "F64".

I think this can cover all cases. If the document or the feature is not completed, feedback is still welcome.

@eeeXun
Copy link
Owner Author

eeeXun commented Jun 24, 2023

UPDATE:
You can set "C-c" now. And you can config the exit key.

@doums
Copy link

doums commented Jun 26, 2023

Looks good now

@eeeXun eeeXun merged commit d01ef4e into master Jun 30, 2023
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