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

Bash 5.1 supports making PROMPT_COMMAND an array #130

Open
dimo414 opened this issue Mar 6, 2022 · 5 comments
Open

Bash 5.1 supports making PROMPT_COMMAND an array #130

dimo414 opened this issue Mar 6, 2022 · 5 comments

Comments

@dimo414
Copy link
Collaborator

dimo414 commented Mar 6, 2022

This isn't strictly a bug/FR; if you feel like enabling Discussions for this repo I can repost this as a discussion

I noticed Bash 5.1 allows PROMPT_COMMAND to be an array:

ee. PROMPT_COMMAND: can now be an array variable, each element of which can contain a command to be executed like a string PROMPT_COMMAND variable.

This obviously won't help older releases that bash-preexec continues to support, but it's an interesting new development we might want to utilize. It would help situations like #129 for instance.

@akinomyoga
Copy link
Contributor

FYI, in the bug-bash mailing list, there has been a discussion on how to handle the situation when there are both types of configurations that assumes the scalar PROMPT_COMMAND and the array PROMPT_COMMAND. [ Note that there have been two distinct variables PROMPT_COMMAND and PROMPT_COMMANDS in 5.1-alpha, but PROMPT_COMMANDS are renamed to PROMPT_COMMAND before the release after the suggestion by Martijn. ] If it would use bash-5.1 PROMPT_COMMAND, I recommend adopting Martijn's suggestion (see the original thread for the backgrounds):

if ((BASH_VERSINFO[0] > 5 || BASH_VERSINFO[0] == 5 && BASH_VERSINFO[1] >= 1)); then
  PROMPT_COMMAND=${PROMPT_COMMAND-}
  PROMPT_COMMAND+=("some command here")
else
  # ....
fi

@akinomyoga
Copy link
Contributor

akinomyoga commented Mar 6, 2023

I realized that Bash 5.1 PROMPT_COMMAND breaks bash-preexec if there is an element in PROMPT_COMMAND with its index greater than 0.

The following ~/.bashrc would reproduce the issue:

# ~/.bashrc

PROMPT_COMMAND=
PROMPT_COMMAND[1]='((++prompt_count))'
preexec() { echo PREEXEC >/dev/tty; }
precmd() { echo PRECMD >/dev/tty; }
source ~/.mwg/git/rcaloras/bash-preexec/bash-preexec.sh

With this ~/.bashrc, the result becomes

$ echo hello
hello
PRECMD
PREEXEC
$

This is because PROMPT_COMMAND becomes

declare -a PROMPT_COMMAND=([0]=$'__bp_precmd_invoke_cmd\n__bp_interactive_mode' [1]="((++prompt_count))")

so that ${PROMPT_COMMAND[1]} (((++prompt_count))) is executed just after __bp_interactive_mode. The function call __bp_interactive_mode needs to be put after the last element of PROMPT_COMMAND in Bash 5.1+.

@rcaloras
Copy link
Owner

rcaloras commented Mar 6, 2023

@akinomyoga thanks for reporting. Any idea how prevalent an issue this is? Not sure how many people are already using bash 5.1 and using it in this fashion.

Regardless, do you think this simple enough that we could just introduce a conditional in __bp_install to handle this accordingly?

@akinomyoga
Copy link
Contributor

Not sure how many people are already using bash 5.1 and using it in this fashion.

Now the latest releases of Linux distributions already use Bash 5.1 or 5.2, so I think many people are already using Bash 5.1 & 5.2.

Nevertheless, I guess not so many Bash configurations have started to use PROMPT_COMMAND[1], PROMPT_COMMAND[2], …, yet I think I have seen some scripts using it (e.g. kitty's shell-integration).

Any idea how prevalent an issue this is?

Currently, a small number of configurations use the array PROMPT_COMMAND with a test on the Bash version, but I think people are going to start using the array PROMPT_COMMAND when Bash 5.0 and lower versions mostly disappear from the market.

Regardless, do you think this simple enough that we could just introduce a conditional in __bp_install to handle this accordingly?

Yes, I think so. I'll submit a PR for it (along with a suggested solution to #140).

@zachbai
Copy link

zachbai commented Jun 22, 2023

Was this issue indeed fixed by #141?

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

4 participants