atuinsh / atuin

✨ Magical shell history
https://atuin.sh
MIT License
21.03k stars 565 forks source link

`enter_accept` breaks liquidprompt with bash-preexec #1668

Closed cmm closed 9 months ago

cmm commented 9 months ago

"breaks" as in:

after interactively selecting-and-running a command from atuin's TUI the previous PS1 is shown, and there are leftover characters right of the cursor if that previous PS1 happened to be shorter than the current one (if that formulation even makes sense? LP does the seemingly-normal thing of putting logic in ${pre{cmd,exec}_functions[@]} that does stuff and sets PS1 to something).

I tried flipping atuin & LP inits around in .bashrc to see if that's some sort of an order issue, but that didn't help. no idea what else to try or if this is even a tractable problem; suggestions welcome!

akinomyoga commented 9 months ago

I don't have a good solution, but here I just describe what is happening there.

This is related to the timing of when the prompt content is updated using PS1. Bash updates the prompt content only after the execution of the user command from the command line. For example, even if PS1 is changed, its content will not be reflected in the prompt until the user presses Enter to run the current command line.

With enter_accept, Atuin tries to emulate the execution of the user command, and update the prompt content by the new PS1 in the following line:

https://github.com/atuinsh/atuin/blob/374255dd5856465b30d4c90d5882b150fa6c47de/atuin/src/shell/atuin.bash#L157-L161

However, the prompt content that Bash internally holds is not yet updated because enter_accept is not an authentic command execution that Bash recognizes. Here, inconsistency between "the prompt content from the new PS1 rendered by Atuin" and "the one from the previous PS1 held by Bash" arises.


One solution is to erase the last line of the prompt drawn by Atuin so that Bash totally redraws the last line. In this case, the prompt content would be the old one. For example, even if cd xxx is performed by enter_accept, the prompt still shows the previous directory (if the current directory is shown in the last line of the prompt). I think this is confusing and not a very good solution.

Another possibility could be to combine a readline keyboard macro with dynamic bind to conditionally induce an authentic command execution by Bash. However, I've never tried that, and I'm not sure if it is actually possible.

akinomyoga commented 9 months ago

In this case, the prompt content would be the old one. For example, even if cd xxx is performed by enter_accept, the prompt still shows the previous directory (if the current directory is shown in the last line of the prompt). I think this is confusing and not a very good solution.

Ah, wait. This also applies to the current implementation, so I think there is no drawback in erasing the last line.

cmm commented 9 months ago

1669 removes the bad visual artifacts, but the value of PS1 being shown is still the previous one. but if I understand the explanation above correctly, that just can't be helped.

akinomyoga commented 9 months ago

but the value of PS1 being shown is still the previous one.

Yeah, that is another issue. The current version also has the same problem, so it is not the problem introduced by #1669.

Another possibility could be to combine a readline keyboard macro with dynamic bind to conditionally induce an authentic command execution by Bash. However, I've never tried that, and I'm not sure if it is actually possible.

I'll think about this idea when I have time, but I'm not sure if this is actually possible and if this wouldn't cause other issues. I need careful experiments.

akinomyoga commented 9 months ago

Another possibility could be to combine a readline keyboard macro with dynamic bind to conditionally induce an authentic command execution by Bash. However, I've never tried that, and I'm not sure if it is actually possible.

I'll think about this idea when I have time, but I'm not sure if this is actually possible and if this wouldn't cause other issues. I need careful experiments.

I created an experimental version at https://github.com/akinomyoga/atuin/commit/aa00b00d2d5c0b906b57250aea72bbf6b22350bf. It seems to be working in a quick test, but it's tricky to explain to others how this is working, and therefore it doesn't seem to be a maintainable technique. It is also tricky to explain how to set up the custom keybinding to users when the users want to manually choose the keys.

cmm commented 9 months ago

The good news is that with ble.sh the prompt gets updated in time, so I guess might as well try to get used to that :) (can I use both ble.sh and bash-preexec? looks like I can, but should I?)

akinomyoga commented 9 months ago

The good news is that with ble.sh the prompt gets updated in time,

Yeah, those issues are all specific to bash-preexec.

(can I use both ble.sh and bash-preexec? looks like I can, but should I?)

ble.sh provides the same feature as bash-preexec. It is provided by the integration/bash-preexec module of ble.sh and is enabled by Atuin's initialization. Actually, even if you load both blesh (with integration/bash-preexec) and bash-preexec, ble.sh removes the hooks set by bash-preexec when ble.sh detects it. So loading both is essentially the same as loading only ble.sh (with integration/bash-preexec). Or maybe it causes problems when ble.sh fails to detect and remove the hooks introduced by bash-preexec.