Various bash improvements

[calestyo]

Again... someone should thoroughly double check these, especially I have no bash versions from the primordial times of the universe around. ;-)

Speaking of which... does it really make sense to keep code for bash versions < 4? I mean bash 4 was something like early 2009.

I'd just drop all code for those ancient versions, especially as all the use of \C-b\C-k, etc. may anyway easily break, if a user has re-assigned them to something else.

[calestyo]

btw: There's also one further difference in the 3:

fzf/shell/key-bindings.bash

Line 111 in 0080990

bind -m emacs-standard '"\C-t": " \C-b\C-k \C-u`__fzf_select__`\e\C-e\er\C-a\C-y\C-h\C-e\e \C-y\ey\C-x\C-x\C-f"'

and

fzf/shell/key-bindings.bash

Line 132 in 0080990

bind -m emacs-standard '"\ec": " \C-b\C-k \C-u`__fzf_cd__`\e\C-e\er\C-m\C-y\C-h\e \C-y\ey\C-x\C-x\C-d"'

use:

" \C-b…`__fzf_select__`\e…"

while:

fzf/shell/key-bindings.bash

Line 116 in 0080990

bind -m emacs-standard '"\C-r": "\C-e \C-u\C-y\ey\C-u"$(__fzf_history__)"\e\C-e\er"'

uses

"\C-e …"$(__fzf_history__)"\e…"

So no extra leading and some extra " (apart from the different style of command substitution)… is that by intention?

[calestyo]

@junegunn … any ideas on these? :-D

[calestyo]

Added another minor improvement.

[junegunn]

Using $(...) instead of backticks was a mistake. We intentionally use backticks to avoid delay from blink-matching-paren.

See #580

Fixed it on master: a3ff49a

[junegunn]

Speaking of which... does it really make sense to keep code for bash versions < 4?

Yes, because macOS ships with bash 3. https://apple.stackexchange.com/a/208405/125495

The code is short and it has been stable for quite a while. And considering that I have no plans to extend the feature set, I don't see much benefit in pruning a few lines of code and breaking backward compatibility for some, albeit small, number of users.

[calestyo]

Using $(...) instead of backticks was a mistake. We intentionally use backticks to avoid delay from blink-matching-paren.

Funny... did you ever find out, why they behave different there?

Fixed it on master: a3ff49a

Thx.

Yes, because macOS ships with bash 3. https://apple.stackexchange.com/a/208405/125495

Sigh ^^... but okay for me.

I've rebased my branch onto current master and dropped the now obsolete commit that touched command substitution.

Could you please review the remaining commits? :-)

[junegunn]

Funny... did you ever find out, why they behave different there?

Yes, if you put set blink-matching-paren on in your .inputrc and type something like echo $(...), you'll see that the cursor momentarily moves to the opening parenthesis to visually assist the user and it causes a small delay, and that's exactly what happens with $(__fzf__history__).

Could you please review the remaining commits? :-)

Will do. I'm kind of busy with other stuff now.

[junegunn]

We decided to keep the pre-bash4 implementation for ALT-C because of a critical UX issue. See #2674.

Also, your version doesn't work and the test cases are failing. Have you tested it?

[calestyo]

Ooops... I hadn't noticed that this didn’t change the CWD, and worked on other branches since then and not looked back.

As for the tests, I've seen failures and flaky tests even when working on master, see #3449 (comment).
Not sure what I'm doing wrong there.

Okay, using bind -x seems indeed not so easy. redraw-current-line only re-draws the line, but does not re-evaluate PS1, so while the CWD has in fact changed, the old one is still shown in the prompt.

Anything that causes enter to be printed wouldn’t be much better than the current solution (and cause the current line to be executed.

While searching for solutions, I stumbled over ${PS1@P} - not sure if that could somehow be used to re-draw the prompt manually.

Some people suggested sending SIGWINCH to the shell, but that didn’t work for me.

https://superuser.com/a/1662149 worked for me, but it would also require us override more bindings, and needs more shell vars. Same with https://stackoverflow.com/a/40426702.

The current implementation still seems fragile to me (in case anyone has chanced the meaning of any of those keybindings, which I for example actually have for C-h).

Maybe we could somehow save the current keybindings, remove them (to be sure defaults are used), and restore them afters wards?

[calestyo]

@junegunn, I've removed all commits except the minor fa6b43f.

As for switching Alt-C to use bind -x:

I still think there is some value in doing this, namely to get it working for any systems where the user has reassigned the keyseqs you use in:

fzf/shell/key-bindings.bash

Lines 132 to 134 in 4fdc082

	bind -m emacs-standard '"\ec": " \C-b\C-k \C-u`__fzf_cd__`\e\C-e\er\C-m\C-y\C-h\e \C-y\ey\C-x\C-x\C-d"'
	bind -m vi-command '"\ec": "\C-z\ec\C-z"'
	bind -m vi-insert '"\ec": "\C-z\ec\C-z"'

and also to avoid having to set up helper bindings as in:

fzf/shell/key-bindings.bash

Lines 102 to 107 in 4fdc082

	# Required to refresh the prompt after fzf
	bind -m emacs-standard '"\er": redraw-current-line'
	bind -m vi-command '"\C-z": emacs-editing-mode'
	bind -m vi-insert '"\C-z": emacs-editing-mode'
	bind -m emacs-standard '"\C-z": vi-editing-mode'

which may overwrite the user’s own bindings.

I think I may have found a working solution, at least for bash versions that already have bind -x.

It's not yet perfect and I'm not enough of a bash/readline expert to determine whether it really works properly and in all cases, so I wrote mails to help-bash, asking for their advice:

• https://lists.gnu.org/archive/html/help-bash/2023-10/msg00000.html
• https://lists.gnu.org/archive/html/help-bash/2023-10/msg00001.html
  if you're curious, you can already look there, how I think we could perhaps do it.

At least this time I actually did some testing O:-) ... and it looked good, except for the open points mentioned in the mail to help-bash.

I think we should handle any possible improvements to Alt-C in a separate issue, so if you're fine with the remaining minor commit of this PR, please merge it.

At least the tests seem to ran through this time.

[junegunn]

Merged, thanks.

I think we should handle any possible improvements to Alt-C in a separate issue

Ideally yes, but we need to evaluate whether the added complexity is really worth it. Users can make all sorts of unconventional choices and our bindings are not immune to them. But I have maintained this code for many years, yet no one has ever complained that ALT-C does not work because of their creative bindings. I think that says something.

[calestyo]

Ideally yes

I've opened #3461

but we need to evaluate whether the added complexity is really worth it. Users can make all sorts of unconventional choices and our bindings are not immune to them. But I have maintained this code for many years, yet no one has ever complained that ALT-C does not work because of their creative bindings. I think that says something.

Well, I for example, have already assigned C-h and it breaks... so: "complain".😜

Seriously, I think the added complexity is still manageable, and IMO we should at least try our best to avoid extreme cases, where accidental execution of a binding who was assigned a non-default action may cause damage.

It doesn't immediately have to be the:
bind -m emacs-standard -x '"\C-b": 'launch_nuclear_B_ombs' case 😉, but even it causes just minor damage, we should prevent it at least when possible.

[calestyo]

Oh and thanks for merging.

[junegunn]

Well, I for example, have already assigned C-h and it breaks... so: "complain".😜

Can you tell me what you are binding it to, and how it makes ALT-C behave?

[calestyo]

Can you tell me what you are binding it to, and how it makes ALT-C behave?
I have in my .inptrc:

#<Control-Backspace>: remove all characters before the cursor to the start of the line
"\C-h": backward-kill-line

#<Control-Delete>: remove all characters from the cursor to the end of the line
"\e[3;5~": kill-line

As far as I can see, the effect is by coincidence the same as the example in the end of the main post of #3461, i.e. if I have a readline buffer with date and do Alt-C, I get one with just ate.