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): fix preexec of child Bash session started by enter_accept #2558

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

akinomyoga
Copy link
Contributor

I noticed that if a new child Bash session is started by enter_accept, the preexec hook provided by bash-preexec is not triggered at all. This is because the child Bash session inherits the environment variable READLINE_POINT of the parent shell session, and bash-preexec considers we are still processing the Atuin menu even when a new user command is processed inside the child Bash session.

How to reproduce the issue

Consider the following bashrc:

# bashrc
source ~/.mwg/git/rcaloras/bash-preexec/bash-preexec.sh
preexec() { echo "[$FUNCNAME]"; }
precmd() { echo "[$FUNCNAME]"; }
eval "$(atuin init bash)"

With the current main branch, we first start a new child session by manually typing the command bash and running it:

$ bash        # <-- This starts a new child Bash session
[preexec]    # <-- preexec by parent session
[precmd]
$ echo 1
[preexec]
1
[precmd]
$ exit
[preexec]
exit
[precmd]     # <-- precmd by parent sesssion
$

This registers the history entry "bash" in Atuin's database. Any problems don't happen so far. Then, we next press the keys upupupenter to select the entry "bash" and execute it through the enter_accept feature.

$ bash        # <-- This is executed through Atuin's enter_accept
[preexec]     # <-- preexec by parent session
[precmd]
$ echo 1
1      # <-- preexec is inactive
[precmd]
$ echo 2
2      # <-- preexec is inactive
[precmd]
$ exit
exit      # <-- preexec is inactive
[precmd]     # <-- precmd by parent session
$

Solution

In this PR, the problem is solved by removing the export attribute of READLINE_POINT and READLINE_LINE before running the user command with enter_accept. See the code comments added in this PR for details.

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

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.

nice one - thank you for the detailed explanation too!

@ellie ellie merged commit d63f7df into atuinsh:main Jan 23, 2025
19 checks passed
@akinomyoga akinomyoga deleted the enter_accept-unset-readline-vars branch January 23, 2025 02:21
@akinomyoga
Copy link
Contributor Author

Thanks!

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