-
-
Notifications
You must be signed in to change notification settings - Fork 637
fix(bash): fix and improve the keybinding to up
#1515
New issue
Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “No 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? No Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Now, the inlined script is so long that it might be better to consider outputting just the switches for C-r and up and moving the actual binding code to let base = include_str!("../shell/atuin.bash");
let mut bind_ctrl_r = false;
let mut bind_up_arrow = false;
if std::env::var("ATUIN_NOBIND").is_err() {
bind_ctrl_r = !self.disable_ctrl_r;
bind_up_arrow = !self.disable_up_arrow;
}
println!("__atuin_bind_ctrl_r={bind_ctrl_r}");
println!("__atuin_bind_up_arrow={bind_up_arrow}");
println!("{base}"); and do branching in ...
if [[ $__atuin_bind_ctrl_r == true ]]; then
bind -m emacs -x '"\C-r": __atuin_history'
...
fi
if [[ $__atuin_bind_up_arrow == true ]]; then
...
fi
... What do you think? (Edit: Commit da9df8a is an example implementation. I can drop the commit if you don't like it.) |
I'm happy with this, thank you! Once the conflicts are resolved I'll get it merged |
Thanks! I have rebased it! |
ble.sh has a multiline mode where pressing [up] can be used to move to the previous line. The command history is loaded only when the [up] key is pressed when the cursor is in the first line. However, with Atuin's integration, the [up] key always causes Atuin's history search and cannot be used to move to the previous line when the cursor is not in the first line. There is also another situation that the [up] key does not load the command history. In this patch, with ble.sh, we perform Atuin's history search only in the situation where the command history is loaded in the original binding of ble.sh.
With the current implementation, the keybindings to [C-r] and [up] are only set up in the currently activated keymaps in Bash. As a result, if the user changes the keymap after `eval "$(atuin init bash)"`, Atuin's keybindings do not take effect. In this patch, we bind Atuin's keybindings in all the keymaps (emacs, vi-insert, and vi-command) by explicitly specifying them (as done for the Zsh integration). The keybinding to "k" in the vi-command keymap is also added to make it consistent with the Zsh integration.
In bash <= 4.2, "bind -x" with a key sequence with more than two bytes does not work properly. Inputting the key sequence will produce an error message saying "bash: bash_execute_unix_command: cannot find keymap for command", and the shell command is not executed. To work around this, we can first translate the key sequences to another key sequence with two bytes and run the shell command through the two-byte key sequence. In this patch, we use \C-x\C-p as the two-byte key sequence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Really appreciate all the improvements you've been doing in this area
Hmm, let mut bind_ctrl_r = false;
let mut bind_up_arrow = false;
if std::env::var("ATUIN_NOBIND").is_err() {
bind_ctrl_r = !self.disable_ctrl_r;
bind_up_arrow = !self.disable_up_arrow;
} as let mut bind_ctrl_r = false;
let bind_up_arrow = if std::env::var("ATUIN_NOBIND").is_err() {
bind_ctrl_r = !self.disable_ctrl_r;
!self.disable_up_arrow
} else {
false
}; but this breaks the symmetry of OK, this time I can write it in this way: let (bind_ctrl_r, bind_up_arrow) = if std::env::var("ATUIN_NOBIND").is_err() {
(!self.disable_ctrl_r, !self.disable_up_arrow)
} else {
(false, false)
}; though this wouldn't be the general solution in similar cases. |
Probably it reads better when the condition is inversed. I've amended it. |
Co-authored-by: Ellie Huxtable <ellie@elliehuxtable.com>
Thank you for all your help! |
This pull request has been mentioned on Atuin Community. There might be relevant details there: https://forum.atuin.sh/t/up-arrow-error-commands-not-saved/339/3 |
This PR includes three different fixes related to the keybinding to up in Bash. The descriptions are found in commit messages and in code comments.