Skip to content

Conversation

@ima1zumi
Copy link
Member

Close: #624

I've implemented .inputrc to properly handle the mode, which can be set to either emacs or vi. For vi, set to vi_insert mode.
Upon checking Readline's behavior, it appears that else is entered when the mode check fails.
For instance, if $if mode=vi is written, else will not be executed.

ref: https://tiswww.case.edu/php/chet/readline/readline.html#Conditional-Init-Constructs

@ima1zumi ima1zumi force-pushed the treat-mode-condition branch from 3096fcd to 2c45901 Compare April 24, 2024 16:26
@ima1zumi ima1zumi added the enhancement New feature or request label Apr 24, 2024
Comment on lines 242 to 243
condition = true
@editing_mode_section = $1.downcase == 'emacs' ? 'emacs' : 'vi'
Copy link
Member

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.

Copy link
Member Author

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!

@ima1zumi ima1zumi force-pushed the treat-mode-condition branch from 2c45901 to 2164313 Compare April 25, 2024 16:23

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])
Copy link
Member

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 
Copy link
Member Author

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.

@ima1zumi ima1zumi force-pushed the treat-mode-condition branch 2 times, most recently from 59e22dd to 2d6c5a6 Compare April 27, 2024 16:52
mode = $1.downcase
if @editing_mode_label == mode.to_sym
condition = true
@keymap_label = mode == 'emacs' ? :emacs : :vi_insert
Copy link
Member

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" $endif

With 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" $endif
Copy link
Member Author

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 🙏

@ima1zumi ima1zumi force-pushed the treat-mode-condition branch from 2d6c5a6 to f9a8a6b Compare April 28, 2024 08:47
@ima1zumi ima1zumi marked this pull request as draft April 28, 2024 17:43
@ima1zumi ima1zumi force-pushed the treat-mode-condition branch from f9a8a6b to d280958 Compare April 29, 2024 11:54
@ima1zumi ima1zumi marked this pull request as ready for review April 29, 2024 11:57
Copy link
Member

@tompng tompng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@tompng tompng merged commit bed5fb3 into ruby:master Apr 29, 2024
@ima1zumi ima1zumi deleted the treat-mode-condition branch April 29, 2024 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

2 participants