- Notifications
You must be signed in to change notification settings - Fork 93
Handle mode condition in inputrc #687
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
Conversation
3096fcd to 2c45901 Compare lib/reline/config.rb Outdated
| condition = true | ||
| @editing_mode_section = $1.downcase == 'emacs' ? 'emacs' : 'vi' |
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.
condition should be true only when editing_mode_label matches arg.
"\e\C-j": vi-editing-mode "\C-e": emacs-editing-mode set editing-mode vi set keymap emacs $if mode=emacs # skipped because editing-mode(==vi) is not emacs "x": "A" $else # executed. set emacs-keymap "x" to "B" (insert "B") "x": "B" $endif # type ESC C-e to switch to emacs mode, type x, then B is inserted "\e\C-j": vi-editing-mode "\C-e": emacs-editing-mode set editing-mode emacs set keymap vi $if mode=vi # skipped because editing-mode(==emacs) is not vi "x": "A" $else # executed. set vi-keymap "x" to "iB" (switch to insert mode and insert "B") "x": "iB" $endif # type ESC C-j to switch to vi-insert, type ESC to switch to vi-command, type x, then B is inserted And looks like key is registered to keymap (== @keymap_label), so I think we don't need to introduce @editing_mode_section.
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.
Indeed, you're right. I've fixed it!
2c45901 to 2164313 Compare test/reline/test_config.rb Outdated
| | ||
| assert_equal({[5] => :history_search_backward}, @config.instance_variable_get(:@additional_key_bindings)[:vi_insert]) | ||
| assert_equal({}, @config.instance_variable_get(:@additional_key_bindings)[:vi_command]) | ||
| assert_equal({}, @config.instance_variable_get(:@additional_key_bindings)[:emacs]) |
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.
I think this test case is wrong.
With this inputrc file saved to ./inputrc
$if mode=vi "x": "X" $else "y": "Y" $endif Only "y"→"Y", the $else part is reflected.
$ echo x | INPUTRC=./inputrc ruby -rreadline -e "p Readline::VERSION; Readline.readline'>'" "8.2" >x $ echo y | INPUTRC=./inputrc ruby -rreadline -e "p Readline::VERSION; Readline.readline'>'" "8.2" >Y 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.
Thanks for letting me know! I've fixed it and added tests.
59e22dd to 2d6c5a6 Compare lib/reline/config.rb Outdated
| mode = $1.downcase | ||
| if @editing_mode_label == mode.to_sym | ||
| condition = true | ||
| @keymap_label = mode == 'emacs' ? :emacs : :vi_insert |
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.
I think if mode=xxx does not modify keymap_label.
I tried this inputrc
set editing-mode vi set keymap emacs $if mode=vi "x": "FOOBAR" $endifWith this command. Typing x in vi mode just inserts x. After switching to emacs mode with \e\C-e, typing x inserts FOOBAR.
$ ruby -e 'puts "vi:x, \e\C-e\C-eemacs:x"' | ruby -rreadline -e "puts Readline::VERSION;Readline.readline('>')" 8.2 >vi:x, emacs:FOOBAR I think Readline reads inputrc like this
# Sets mode_label and keymap_label to vi set editing-mode vi # Change keymap_label to emacs. mode_label is still vi. set keymap emacs # condition=true because current mode_label is vi $if mode=vi # sets keybinding to current keymap_label=emacs "x": "FOOBAR" $endifThere 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.
Thanks for letting me know. I fixed. If you see anything else wrong, please let me know 🙏
2d6c5a6 to f9a8a6b Compare f9a8a6b to d280958 Compare
tompng left a comment
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.
LGTM 👍
Close: #624
I've implemented
.inputrcto properly handle themode, which can be set to eitheremacsorvi. Forvi, set tovi_insertmode.Upon checking Readline's behavior, it appears that
elseis entered when themodecheck fails.For instance, if
$if mode=viis written,elsewill not be executed.ref: https://tiswww.case.edu/php/chet/readline/readline.html#Conditional-Init-Constructs