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

Re-adjust PROMPT_COMMAND when PROMPT_COMMAND is modified elsewhere #143

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

akinomyoga
Copy link
Contributor

@akinomyoga akinomyoga commented Mar 13, 2023

This is the fix for #140. The author of #140 seems to have fixed the issue by directly modifying the C source code of Midnight Commander mc, but we could work around the modified PROMPT_COMMAND at bash-preexec's side.

In this patch, when the PROMPT_COMMAND is modified by users or by another code (such as mc) to have commands outside __bp_precmd_invoke_cmd...__bp_interactive_mode, bash-preexec adjust PROMPT_COMMAND again so that every external command is enclosed within __bp_precmd_invoke_cmd and __bp_interactive_mode.

In this PR, other code adjustments are included as in the following list. I think these commits are closely related to each other, but if you think some of them should be discussed in a separate PR, please let me know.

  • In commit fe5aedb, I refactored __bp_sanitize_string to include some processing that has been done outside of it. (such as that of Try to better handle external modification to $PROMPT_COMMAND #128), to use the function in implementing the string manipulation of PROMPT_COMMAND.
  • Commit 299080e is a workaround for shopt -u extglob. The problem is explained in the code comment.
  • Commit c85de1b is the main part of this patch, which re-adjusts PROMPT_COMMAND.
  • Commit 9c669e6 works around the situation that multiple pairs of our hooks are installed. We try to remove the existing hooks in the re-adjustments of PROMPT_COMMAND, but it may fail when another framework saves the value of PROMPT_COMMAND in another variable. For such a case, we process our hooks only when the hooks are directly called from the top level of PROMPT_COMMAND. The detail is explained in the commit message.

@akinomyoga
Copy link
Contributor Author

akinomyoga commented Mar 13, 2023

A test seems to fail in CI, though it works in my environment with Bash 5.2:

not ok 6 __bp_install_prompt_command should adjust modified PROMPT_COMMAND
# (in test file test/bash-preexec.bats, line [11](https://github.com/rcaloras/bash-preexec/actions/runs/4400455159/jobs/7705766093#step:4:12)6)
#   `if (( ${#PROMPT_COMMAND[@]} == 2 )); then' failed
# /tmp/bats.2795.src: line 116: PROMPT_COMMAND: unbound variable

I guess this would be related to the version difference of Bash, but I'm not sure how to run bats with the specified version of Bash. As far as I have looked inside the source code of bats, the path to Bash seems to be hardcoded...

$ head -1 /usr/bin/bats /usr/libexec/bats-core/*
==> /usr/bin/bats <==
#!/usr/bin/bash

==> /usr/libexec/bats-core/bats <==
#!/usr/bin/bash

==> /usr/libexec/bats-core/bats-exec-file <==
#!/usr/bin/bash

==> /usr/libexec/bats-core/bats-exec-suite <==
#!/usr/bin/bash

==> /usr/libexec/bats-core/bats-exec-test <==
#!/usr/bin/bash

==> /usr/libexec/bats-core/bats-format-cat <==
#!/usr/bin/bash

==> /usr/libexec/bats-core/bats-format-junit <==
#!/usr/bin/bash

==> /usr/libexec/bats-core/bats-format-pretty <==
#!/usr/bin/bash

==> /usr/libexec/bats-core/bats-format-tap <==
#!/usr/bin/bash

==> /usr/libexec/bats-core/bats-format-tap13 <==
#!/usr/bin/bash

==> /usr/libexec/bats-core/bats-preprocess <==
#!/usr/bin/bash

@akinomyoga akinomyoga marked this pull request as draft March 13, 2023 01:38
@akinomyoga akinomyoga force-pushed the adjust-PROMPT_COMMAND branch from 8549ece to 2cee9b9 Compare March 13, 2023 01:55
@akinomyoga akinomyoga marked this pull request as ready for review March 13, 2023 01:56
@akinomyoga
Copy link
Contributor Author

akinomyoga commented Mar 13, 2023

I have directly modified bats and could reproduce the error with Bash 5.0. I have fixed the CI test.

We try to remove the existing hooks before the re-adjustment of
PROMPT_COMMAND, but it might not work when another framework saves the
original value of `PROMPT_COMMAND` in another variable and tries to
call it from inside their `PROMPT_COMMAND` hook.  For example,
Starship does that [1].  In such a case, our hooks would be called
several times unexpectedly.  To avoid this situation, we process our
hooks only when the hooks are called at the top level.

[1] https://github.com/starship/starship/blob/3d474684149e0a7959fb986f8cea1d28b4c69d87/src/init/starship.bash#L97
@akinomyoga akinomyoga force-pushed the adjust-PROMPT_COMMAND branch from 2cee9b9 to 9c669e6 Compare February 3, 2024 06:08
@akinomyoga
Copy link
Contributor Author

Just to clarify, I mentioned mc (Midnight Commander) in the original post to explain the background, but the problem is not specific to mc. The problem happens with any commands that are added to PROMPT_COMMAND after the shell startup.

@rcaloras @dimo414 Could you take a look at this PR? If you'd like separate PR for each commit, I can create separate PRs.

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.

1 participant