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

Bind to both emacs and vi-insert keymaps in Bash #434

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

akinomyoga
Copy link
Contributor

Thank you for reviewing all the PRs that I submitted so far!

This is the final PR. Unlike the previous ones, this is a suggestion for a change.

Problem: Bash has several keymaps depending on the current editing mode (i.e., the emacs editing mode or the vi editing mode). The default keymap for the vi editing mode is separate from the one for the emacs editing mode. Currently (in the master branch), the Bash keybindings are only set up in the current keymap. If the user changes the editing mode after eval "$(mcfly init bash)", the keybindings become unavailable because the default keymap is switched.

For example, with the following bashrc, the keybindings are inactive:

# bashrc
eval "$(mcfly init bash)"
set -o vi

Solution: This PR registers the keybindings to both keymaps by explicitly specifying a keymap to register the keybindings.

This is needed when the user changes the editing mode by specifying
`set -o vi` after `source mcfly.bash'.
@cantino cantino merged commit 0d01326 into cantino:master Aug 12, 2024
17 of 19 checks passed
@cantino
Copy link
Owner

cantino commented Aug 12, 2024

Thanks!

@akinomyoga akinomyoga deleted the bash-keymap branch August 12, 2024 02:34
@akinomyoga
Copy link
Contributor Author

Thank you for taking the time to review and process so many pull requests from me! I appreciate your dedication to maintaining the project! Thank you.

@akinomyoga
Copy link
Contributor Author

Ah, I'm sorry for the extra fi.

@cantino
Copy link
Owner

cantino commented Aug 12, 2024

No problem at all, and thank you for all the time you put into these!

@cantino
Copy link
Owner

cantino commented Aug 12, 2024

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