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

fix(bash): rework #1509 to recover from the preexec failure #1729

Merged
merged 1 commit into from
Feb 17, 2024

Conversation

akinomyoga
Copy link
Contributor

@akinomyoga akinomyoga commented Feb 17, 2024

Explanations are in the commit message. I've quickly tested it locally and confirmed that the user commands are at least not missing, but I think @ellie can also test it locally to carefully see the behavior.

Refs. #1003, #1509, #1727, #1728, Forum 175

Checks

  • I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle
  • I have checked that there are no existing pull requests for the same thing

In GitHub atuinsh#1509, we blocked the unintended preexec event caused by the
keybinding of Atuin.  However, with that fix, the preexec event for
the intended user command is still missing.  In this patch, we try to
manually run the preexec hook when we detected the unintended preexec
(which means the missing intended preexec).

References:

atuinsh#1509
https://forum.atuin.sh/t/atuin-bash-and-ble-sh/175
atuinsh#1003 (comment)
atuinsh#1727
atuinsh#1728
Copy link
Member

@ellie ellie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works great! I tried a few variations that did not work previously and this seems to have resolved the issue. Thank you 🙏

@ellie ellie merged commit 38dfaab into atuinsh:main Feb 17, 2024
15 checks passed
@akinomyoga akinomyoga deleted the workaround-keybinding-preexec branch February 17, 2024 21:16
ellie pushed a commit that referenced this pull request Feb 26, 2024
In GitHub #1509, we blocked the unintended preexec event caused by the
keybinding of Atuin.  However, with that fix, the preexec event for
the intended user command is still missing.  In this patch, we try to
manually run the preexec hook when we detected the unintended preexec
(which means the missing intended preexec).

References:

#1509
https://forum.atuin.sh/t/atuin-bash-and-ble-sh/175
#1003 (comment)
#1727
#1728
@ellie ellie mentioned this pull request Feb 26, 2024
2 tasks
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

Successfully merging this pull request may close these issues.

2 participants