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

vi-search (/) does not search in history - regression in 1.4.7 #405

Closed
harig opened this issue Jan 15, 2023 · 3 comments
Closed

vi-search (/) does not search in history - regression in 1.4.7 #405

harig opened this issue Jan 15, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@harig
Copy link

harig commented Jan 15, 2023

  1. clink mode with vi keybindings/mode set as default input mode.
  2. / mapped to vi-search, and n, N repeats that search (default)
  3. If clink has some history entries, from vi-command mode, typing "/item" searches for "item" in history, This can be repeated with n/N. (On pressing / the search prompt is shown, then you type "item" and press Enter)
  4. In 1.4.7 doing the above does not search within history, it shows some random (?) unrelated match.

Works as mentioned above until 1.4.6 build. After that (in 1.4.7) I've to use Ctrl-R/S from any of the vi modes to search likewise.

@chrisant996
Copy link
Owner

I'll take a closer look in the morning, but I would expect that it works the same as bash now, and works how Readline is meant for it to work.

This is the change from the release notes:

- Enabled Readline's support for non-incremental vi-mode search (Nn) to search for a shell pattern using fnmatch(), as Posix specifies.

I'll double check whether somehow there's a bug in Readline. All I did was tell the Readline library that the fnmatch function was available. But maybe there's somehow a glitch in the fnmatch function.

@chrisant996 chrisant996 added investigation needed Deeper investigation is needed needs more information The issue needs clarifying information bug Something isn't working and removed investigation needed Deeper investigation is needed needs more information The issue needs clarifying information labels Jan 15, 2023
@chrisant996
Copy link
Owner

Confirmed:

It turns out there are two forms of fnmatch(), with the parameters in opposite orders.
Readline expects one order, and the fnmatch I used as reference uses the opposite order.

Swapping the order makes it work as expected.

Thanks for reporting this!

@harig
Copy link
Author

harig commented Jan 16, 2023

Verified the fix in 1.4.10; thank you for the quick turnaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants