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

Upward navigation through completion results not working as expected #675

Closed
mjgiarlo opened this issue Apr 9, 2024 · 4 comments · Fixed by #677
Closed

Upward navigation through completion results not working as expected #675

mjgiarlo opened this issue Apr 9, 2024 · 4 comments · Fixed by #677
Labels
enhancement New feature or request

Comments

@mjgiarlo
Copy link
Contributor

mjgiarlo commented Apr 9, 2024

Description

Navigating through multi-line autocomplete results in IRB using the Tab key works as expected. Using S-Tab takes the current selection instead of navigating back up. I have S-Tab configured for menu-complete-backward in .inputrc (see below), and it works fine when I turn multi-line off and use readline (see irb_info pastes below).

When I look at https://github.com/ruby/reline/blob/master/lib/reline/line_editor.rb#L1137, the value of key.char comes through as the symbol :menu_complete_backward, which reline does not seem to know how to handle. If I re-map S-Tab from menu-complete-backward to completion-journey-up, upward navigation works in IRB but breaks upward/backward navigation in other command-line tools, e.g., file completion, likely because completion-journey-up does not seem to be defined or used beyond reline (as far as I can tell!).

Terminal Emulator, Shell, etc.

  • Terminal: GNOME Terminal 3.49.92 using VTE 0.74.0 +BIDI +GNUTLS +ICU +SYSTEMD
  • Shell: GNU bash, version 5.2.15(1)-release (x86_64-pc-linux-gnu)
  • OS: Ubuntu 23.10
  • irb_info:
    Ruby version: 3.2.2
    IRB version: irb 1.12.0 (2024-03-06)
    InputMethod: ReadlineInputMethod with ext/readline 8.2 and /home/mjg/.inputrc
    Completion: RegexpCompletor
    .irbrc paths: /home/mjg/.rvm/rubies/ruby-3.2.2/.irbrc, /home/mjg/.irbrc
    RUBY_PLATFORM: x86_64-linux
    LANG env: en_US.UTF-8
    East Asian Ambiguous Width: 1
    
  • irb_info with readline (USE_MULTILINE=false):
    Ruby version: 3.2.2
    IRB version: irb 1.12.0 (2024-03-06)
    InputMethod: RelineInputMethod with Reline 0.5.1 and /home/mjg/.inputrc
    Completion: Autocomplete, RegexpCompletor
    .irbrc paths: /home/mjg/.rvm/rubies/ruby-3.2.2/.irbrc, /home/mjg/.irbrc
    RUBY_PLATFORM: x86_64-linux
    LANG env: en_US.UTF-8
    East Asian Ambiguous Width: 1
    
  • .inputrc:
    set editing-mode emacs
    set show-all-if-ambiguous on
    set completion-ignore-case on
    set menu-complete-display-prefix on
    TAB: menu-complete
    "\e[Z": menu-complete-backward
    "\e[5~": history-search-backward
    "\e[6~": history-search-forward
    "\e[1;5C": forward-word
    "\e[1;5D": backward-word
    "\e[3;5~": kill-word
    
@mjgiarlo
Copy link
Contributor Author

I'm not sure if this is what recommended, but if I tweak my .inputrc to include:

$if Reline
  "\e[Z": completion-journey-up
$else
  "\e[Z": menu-complete-backward
$endif

That allows upward navigation to work in IRB without borking e.g. bash file autocompletion. I'm still fuzzy on where "completion-journey-up" comes from or whether configuration like I pasted above should be necessary navigate results in both directions.

@ima1zumi
Copy link
Member

The issue arises because Reline does not support the setting for menu-complete-backward. Although the setting cannot be configured, the functionality exists and seems to work with completion-journey-up. Here, the hyphen is converted to an underscore and is being called.

elsif @config.editing_mode_is?(:emacs, :vi_insert) and key.char == :completion_journey_up

Reline should support configuring menu-complete-backward.

As a workaround, using $if Reline for branching, as you wrote, seems like a good idea. Please let me know if it doesn't work.

@ima1zumi ima1zumi added the enhancement New feature or request label Apr 10, 2024
@mjgiarlo
Copy link
Contributor Author

@ima1zumi 💬

...
Reline should support configuring menu-complete-backward.

I'd be happy to work on a PR for this to scratch my own itch, if that'd be valuable. It would help me to know if there are any similar examples in the codebase already for related configuration.

As a workaround, using $if Reline for branching, as you wrote, seems like a good idea. Please let me know if it doesn't work.

It does work, yes. 🙂

@ima1zumi
Copy link
Member

ima1zumi commented Apr 10, 2024

@mjgiarlo

Thank you for your offer!

In this case, probably we need to turn this part into a method.

elsif @config.editing_mode_is?(:emacs, :vi_insert) and key.char == :completion_journey_up
if not @config.disable_completion and @config.autocompletion
process_insert(force: true)
@completion_state = CompletionState::NORMAL
completion_occurs = move_completed_list(:up)
end

The reason is because Reline converts commands read from .inputrc from hyphens to underscores and then calls the method as a Symbol. For example, for beginning-of-line (C-a), beginning_of_line is called, which is an alias for ed_move_to_beg.

private def ed_move_to_beg(key)
@byte_pointer = 0
end
alias_method :beginning_of_line, :ed_move_to_beg

Similarly, for other commands, you can find them by converting hyphens in Readline commands to underscores. If you can't find it even after converting hyphens to underscores, it's likely not implemented.

If you need to test, please refer to this file. It's a test using a testing framework called yamatanooroti.
https://github.com/ruby/reline/blob/d348df90d266e44f321810bcf272841fb9669f72/test/reline/yamatanooroti/test_rendering.rb

Here's how to run the tests:
https://github.com/ruby/reline?tab=readme-ov-file#run-tests

Feel free to ask me if you have any questions.

mjgiarlo added a commit to mjgiarlo/reline that referenced this issue Apr 13, 2024
Fixes ruby#675

This commit extracts the upward navigation condition in `LineEditor#input_key` to a new private method, and adds a new alias. This change allows Reline to support upward navigation in when a user has configured `inputrc` to map Shift-Tab to `menu-complete-backward`, a common setting in Bash (>= 4.x).

Instead of special-casing upward navigation in `LineEditor#input_key`, we now allow it to be processed by the branch that calls `process_key`. The extracted method no longer includes the editing mode check since this check is already made by `#wrap_method_call` by the time `#completion_journey_up` (or `#menu_complete_backward`) is called. Since upward navigation is happening in a method other than `#input_key` now, the `completion_occurs` variable that used to be local to `#input_key` is changed to an instance variable so that the new method can change its value. (I see many examples of mutating such instance variables in `LineEditor`, so I assumed this would be an uncontroversial change consistent with the coding practices already in place.)

Test coverage of this change has been added to the emacs and vi `KeyActor` tests.

Many thanks to @ima1zumi for their very helpful comments on ruby#675 which encouraged me to contribute this work!
matzbot pushed a commit to ruby/ruby that referenced this issue Apr 14, 2024
navigation
(ruby/reline#677)

Fixes ruby/reline#675

This commit extracts the upward navigation condition in `LineEditor#input_key` to a new private method, and adds a new alias. This change allows Reline to support upward navigation in when a user has configured `inputrc` to map Shift-Tab to `menu-complete-backward`, a common setting in Bash (>= 4.x).

Instead of special-casing upward navigation in `LineEditor#input_key`, we now allow it to be processed by the branch that calls `process_key`. The extracted method no longer includes the editing mode check since this check is already made by `#wrap_method_call` by the time `#completion_journey_up` (or `#menu_complete_backward`) is called. Since upward navigation is happening in a method other than `#input_key` now, the `completion_occurs` variable that used to be local to `#input_key` is changed to an instance variable so that the new method can change its value. (I see many examples of mutating such instance variables in `LineEditor`, so I assumed this would be an uncontroversial change consistent with the coding practices already in place.)

Test coverage of this change has been added to the emacs and vi `KeyActor` tests.

Many thanks to @ima1zumi for their very helpful comments on #675 which encouraged me to contribute this work!

ruby/reline@2ccdb374a4
artur-intech pushed a commit to artur-intech/ruby that referenced this issue Apr 26, 2024
navigation
(ruby/reline#677)

Fixes ruby/reline#675

This commit extracts the upward navigation condition in `LineEditor#input_key` to a new private method, and adds a new alias. This change allows Reline to support upward navigation in when a user has configured `inputrc` to map Shift-Tab to `menu-complete-backward`, a common setting in Bash (>= 4.x).

Instead of special-casing upward navigation in `LineEditor#input_key`, we now allow it to be processed by the branch that calls `process_key`. The extracted method no longer includes the editing mode check since this check is already made by `#wrap_method_call` by the time `#completion_journey_up` (or `#menu_complete_backward`) is called. Since upward navigation is happening in a method other than `#input_key` now, the `completion_occurs` variable that used to be local to `#input_key` is changed to an instance variable so that the new method can change its value. (I see many examples of mutating such instance variables in `LineEditor`, so I assumed this would be an uncontroversial change consistent with the coding practices already in place.)

Test coverage of this change has been added to the emacs and vi `KeyActor` tests.

Many thanks to @ima1zumi for their very helpful comments on ruby#675 which encouraged me to contribute this work!

ruby/reline@2ccdb374a4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

2 participants