akinomyoga / ble.sh

Bash Line Editor―a line editor written in pure Bash with syntax highlighting, auto suggestions, vim modes, etc. for Bash interactive sessions.
BSD 3-Clause "New" or "Revised" License
2.33k stars 77 forks source link

tab completion quotes weird chars by surrounding with single quotes, menu selection with backslash escapes #406

Closed bkerin closed 4 months ago

bkerin commented 4 months ago

GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu) [Ubuntu 22.04.3 LTS] ble.sh, version 0.4.0-devel4+e7e23854 (noarch) [git 2.43.0, GNU Make 4.3, GNU Awk 5.1.0, API: 3.0 (GNU MPFR 4.1.0, GNU MP 6.2.1)] bash-completion, version 2.11 (hash:b42f5d6a7ad6d4921ec73838ba54a96d6bd30936, 77071 bytes) (noarch) locale: LANG=en_US.UTF-8 terminal: TERM=xterm-256color wcwidth=14.0-west/15.1-2+ri, vte:6800 (65;6800;1)

I'm not sure this is even a bug but when I tab complete and keep going to unique and tab for a stupid file name I end up with e.g.:

cd '/tmp/junky/ugly_dir" two with spaces'/

But when I select the item from a list and hit space I get:

cd /tmp/junky/ugly_dir\"\ two\ with\ spaces

Maybe there's some good reason that different quoting gets used here (it looks like @Q interpolation modifier in the first case and printf "%q" in the second) and they're both valid, it's just a bit odd.

akinomyoga commented 4 months ago

That is intentional. ble.sh filters the candidates by the second form, and after a candidate is selected, it converts the selected candidate to the first form.

For this reason, if the completion is started from the unquoted form /tmp/junky/ugly..., we need to use the second form. However, I don't like the second form with many backslashes, so if the word becomes shorter when the quoting is changed, ble.sh converts the quoting to use the first form.

Or do you request to always use the second form with backslashes? If so, you can check bleopt complete_requote_threshold.

when I select the item from a list and hit space

You need to hit RET to select the item. Usually, hitting RET handles other conversions such as the insertion of the space. When you instead hit other keys, ble.sh leaves the command line as-is and lets the user continue to edit the candidate word itself.

bkerin commented 4 months ago

That is intentional. ble.sh filters the candidates by the second form, and after a candidate is selected, it converts the selected candidate to the first form.

  • The filtering is performed by prefix matching, so if the candidates are generated in the first form when the command line starts with /tmp/junky/ugly, those candidates are filtered out and do not appear on the list.
  • As another option, if the first form is used but the filtering is performed by considering the quoting, the temporary insertion of the matched part replaces the original text. It turned out to cause an issue of generating different sets of candidates before and after the temporary insertion. For example, some candidates on the list are lost after pressing TAB. So this is not a viable option either.
  • As even another option, the completion can generate and insert the candidates in the form /tmp/junky/ugly'....'. This doesn't break the prefix searching and also doesn't need to quote the generated part with many backslashes. At some point, I also tested this implementation, but it didn't look nice that the strange way of quoting remains in the command history. It also inhibits later history-based completion because the actual form of the word varies depending on the starting point of the completion.

For this reason, if the completion is started from the unquoted form /tmp/junky/ugly..., we need to use the second form. However, I don't like the second form with many backslashes, so if the word becomes shorter when the quoting is changed, ble.sh converts the quoting to use the first form.

Or do you request to always use the second form with backslashes? If so, you can check bleopt complete_requote_threshold.

Thanks. For some reason I was thinking the backslash form would do better when adding incrementally to some completed thing (so no need to re-open quotes) but now I can't reproduce the exact situation where it mattered, so maybe I was wrong in the first place.

when I select the item from a list and hit space

You need to hit RET to select the item. Usually, hitting RET handles other conversions such as the insertion of the space. When you instead hit other keys, ble.sh leaves the command line as-is and lets the user continue to edit the candidate word itself.

Interesting. Since going through the completions then "selecting" with space comes so close (because it puts the selected item on the command line) I never consciously noticed it was working differently when single candidate vs. menu. This is part of what makes the change in quote styles jarring for me I guess. At this point I'd have to remap space to change this habit, maybe I should change all completion to work like space instead :)

Actually thinking about this more and playing with it a bit I think ideal behavior would be for (unescaped!) space to complete like enter does in menu context, then append a space. This is safer than Enter since it doesn't end up firing an incomplete command when hit in wrong context (immediately an issue for me at least). It's probably also usually more efficient, since a menu-completed item is probably the end of a word significantly more often than not (so only sometimes backspace, versus usually an additional space to type).

akinomyoga commented 4 months ago

Actually thinking about this more and playing with it a bit I think ideal behavior would be

Thank you for thinking about it.

for (unescaped!) space to complete like enter does in menu context, then append a space.

The current RET (menu_complete/accept) doesn't always insert just a space. For example, when the generated candidate is a directory name, it suffixes a slash instead of a space. For another example, when the current context is inside a quotation, it also inserts the ending quotes. Consider the following case

$ touch file{1..9}.txt
$ echo 'file[TAB][TAB]    # <-- start menu-complete by pressing TAB twice

Moving to e.g. file3.txt and performing menu_complete/accept result in

$ echo 'file3.txt' [pos]    # <-- [pos] is the cursor position

If you just hit SP, it instead becomes

$ echo 'file3.txt [pos]

In this sense, I'm not sure if just binding menu_complete/accept to SP is a good solution.

This is safer than Enter since it doesn't end up firing an incomplete command when hit in wrong context

I've been always using RET to select the item in menu-complete, but I've never ended up in such a situation nor become almost doing that, probably because I am used to the binding. I'm always careful when I press RET.

bkerin commented 4 months ago

Actually thinking about this more and playing with it a bit I think ideal behavior would be

Thank you for thinking about it.

for (unescaped!) space to complete like enter does in menu context, then append a space.

The current RET (menu_complete/accept) doesn't always insert just a space. For example, when the generated candidate is a directory name, it suffixes a slash instead of a space. For another example, when the current context is inside a quotation, it also inserts the ending quotes. Consider the following case

I should have known/remembered you would have binding set up for menu_complete, very nice :) I know about append / and ', this is one reason I tend to like the backslash escaped form, it's incremental.

$ touch file{1..9}.txt
$ echo 'file[TAB][TAB]    # <-- start menu-complete by pressing TAB twice

Moving to e.g. file3.txt and performing menu_complete/accept result in

$ echo 'file3.txt' [pos]    # <-- [pos] is the cursor position

If you just hit SP, it instead becomes

$ echo 'file3.txt [pos]

In this sense, I'm not sure if just binding menu_complete/accept to SP is a good solution.

Well when combined with complete_requote_threshold=-1 it seems interesting.

This is safer than Enter since it doesn't end up firing an incomplete command when hit in wrong context

I've been always using RET to select the item in menu-complete, but I've never ended up in such a situation nor become almost doing that, probably because I am used to the binding. I'm always careful when I press RET.

Yeah, with current git am --directory=/some/garbage then C-c it leaves you in a broken git rebase that git rebase --abort won't terminate and you have to go into .git and start deleting things to recover, always be careful with Enter :)

But in practice I think many users are sort of feeling their way as far as completion goes (indeed that's part of the point of the feature). So even people without my particular space habit might like the space setup, though admittedly it may not be worth the additional documentation space to write about it. Would a recipes entry on this be welcome?

bkerin commented 4 months ago

I went to set up space-instead-of-return and ran into something I don't get. With:

ble-bind -m 'menu_complete' -f 'SP' 'menu_complete/accept'
ble-bind -m 'menu_complete' -f 'RET' ''

or even

ble-bind -m 'menu_complete' -f 'SP' 'menu_complete/accept'
ble-bind -m 'menu_complete' -f 'RET' 'menu_complete/cancel'

Space now complete like Return, but Return still seems to complete as it does by default?

akinomyoga commented 4 months ago

So even people without my particular space habit might like the space setup, though admittedly it may not be worth the additional documentation space to write about it. Would a recipes entry on this be welcome?

Yeah, you can edit the wiki page. Anyone can edit the wiki page currently. But after I set up the blesh-contrib repository, I tend to think we can include such settings there. I think this type of setting can go into contrib/config.

Space now complete like Return, but Return still seems to complete as it does by default?

If it doesn't work, you need to bind it to C-m instead of RET. RET is only effective when the terminal supports it with an enhanced keyboard protocol. The details are described in §3.1.1.

akinomyoga commented 4 months ago

I've now noticed your edit on wiki/Recipes. I've translated it into the Japanese version of the Recipes page and also fixed the point mentioned by FIXME.

By the way, I think the setting is related to the feature request #138. I asked about the detailed design to the OP, but the OP haven't replied. Maybe you have some idea about the design. I'm not currently thinking about the possibility of integrating it into the history expansion feature, so that the sabbrev expansions are also subject to shopt -s histverify, etc.

akinomyoga commented 4 months ago

@bkerin I forgot to tell you, but the compopt issue reported in the help-bash list was fixed in commit 51f9f4f619e6a6e39b1b328a64a2c014b498945b.

bkerin commented 4 months ago

compopt works as expected now

akinomyoga commented 4 months ago

Thanks!