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

Tracking bash-preexec issue #2059

Closed
LecrisUT opened this issue May 30, 2024 · 5 comments
Closed

Tracking bash-preexec issue #2059

LecrisUT opened this issue May 30, 2024 · 5 comments

Comments

@LecrisUT
Copy link
Contributor

Right now between bash-preexec and ble.sh, the former has less side-effects and it might be easier to package and ship, at least as the default dependency for the Fedora package. But the warning on the front-page raises questions. Is there any issue to be tracked either on atuin or bash-preexec about the failures mentioned?

@ellie
Copy link
Member

ellie commented May 30, 2024

It depends massively on the users setup, and how much they've configured their prompt/shell in general

bash-preexec is built on traps and PS1, in order to add shell hooks. It generally works fine so long as the user has an up-to-date bash, but there might be edge cases here and there. There isn't really much that can be done to workaround or improve this, as it is in essence a hack.

ble.sh provides full functionality as we'd expect, but totally replaces the line editor. We cannot and should not install that for users as the default, as it does so much more than they might expect

@LecrisUT
Copy link
Contributor Author

Hmm, but at least you reckon a clean installation will not have unintended consequences?

And how obvious is it when they encounter a breakage? I myself have not encountered anything (or at least I don't know what I'm supposed to look out for), but I have a vanilla setup.

@akinomyoga
Copy link
Contributor

akinomyoga commented May 30, 2024

a clean installation will not have unintended consequences? ... And how obvious is it when they encounter a breakage?

There are still minor problems even with clean installation when the user uses older versions of bash-preexec, which includes 0.5.0 you mentioned in rcaloras/bash-preexec#157. With bash-preexec before rcaloras/bash-preexec#152, the command duration will not be correctly measured after using Atuin's keybindings. I'm not sure if we can say that is an obvious breakage, but users report it as a problem.

The setup can be broken more easily with any other customizations. The problems are all related to the inherent limitations in the approach bash-preexec relies on. With older versions of bash-preexec, when the user has any custom keybindings to a shell command (with bind -x), commands can be missing in Atuin's history after using the keybindings. Even with the latest versions of bash-preexec, appending settings to PROMPT_COMMAND after the shell startup will break bash-preexec (see rcaloras/bash-preexec#143). You can look around at all the other issues and PRs in bash-preexec. Atuin can be affected by any of them.

With the latest bash-preexec on top of vanilla Bash, I cannot soon come up with an obvious breakage but cannot ensure that there is no problem as things are complicated with the bash-preexec approach.

@LecrisUT
Copy link
Contributor Author

Thanks for the overview @akinomyoga. This is good because for downstream as long as we know what are the bugs that need to be fixed and tracked, we can apply the patches on top of the release, and poke upstream to cut releases more often.

@ellie
Copy link
Member

ellie commented Jun 7, 2024

I'm going to close this one - our install script and recommended install method use the bash-preexec master version, so we're not really affected by the lack of release there

Otherwise, the state of bash-preexec can be nicely summarized by their issue tracker

@ellie ellie closed this as not planned Won't fix, can't repro, duplicate, stale Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants