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

mcfly 0.9.2 conflict with zoxide in WarpTerminal #435

Open
aminya opened this issue Aug 12, 2024 · 12 comments · May be fixed by #436
Open

mcfly 0.9.2 conflict with zoxide in WarpTerminal #435

aminya opened this issue Aug 12, 2024 · 12 comments · May be fixed by #436

Comments

@aminya
Copy link

aminya commented Aug 12, 2024

I just upgraded mcfly to 0.9.2, but when zoxide is enabled, the bash prompt doesn't work anymore.
Here's the error I get when I open a new shell

bash: PROMPT_COMMAND: line 706: syntax error near unexpected token `;;'
bash: PROMPT_COMMAND: line 706: `__zoxide_hook;;mcfly_prompt_command'

.bashrc

eval "$(zoxide init bash)"
eval "$(mcfly init bash)"
> echo $PROMPT_COMMAND
__zoxide_hook;;mcfly_prompt_command __bp_trap_string="$(trap -p DEBUG)" trap - DEBUG __bp_install
bash --version
GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu)

This seems to be happening only in Warp. I can't reproduce it using KDE Konsole.

This looks like the faulty commit:
b8e86c5
#430

@akinomyoga
Copy link
Contributor

I'll take a look.

@aminya aminya linked a pull request Aug 12, 2024 that will close this issue
@aminya
Copy link
Author

aminya commented Aug 12, 2024

I made a PR that reverts the change, and it works as expected. The proposed change for using arrays needs more testing in my opinion.
#436

@akinomyoga
Copy link
Contributor

akinomyoga commented Aug 12, 2024

I don't use WarpTerminal, but the problem doesn't reproduce in my environment with several other terminals. In addition, I don't have an idea about the reason why the change in #430 would cause the reported behavior.

What I can tell from the content of PROMPT_COMMAND are

  • bash-preexec is loaded. Neither mcfly nor zoxide loads bash-preexec, so bash-preexec should be loaded in another place. I also tried the combination of source /path/to/bash-preexec.bash, eval "$(zoxide init bash)", and eval "$(mcfly init bash)", but I cannot still reproduce the reported behavior.
  • I suspect somewhere the content of array PROMPT_COMMAND joined into a single element (something like PROMPT_COMMAND="${PROMPT_COMMAND[*]}"), which explicitly breaks the PROMPT_COMMAND mechanism.

I suspect some settings in your environment performs PROMPT_COMMAND="${PROMPT_COMMAND[*]}". If that is the case, I have to say such a setting is broken. In Bash >= 5.1, Bash officially supported the array PROMPT_COMMAND, but such a setting would be broken for any configurations using the officially supported feature.

I first suspected WarpTerminal's shell integration (if any). WarpTerminal seems to be closed-source, so I don't have a way to quickly check (without installing WarpTerminal) whether WarpTerminal has a shell integration and, if any, whether it is related to the present problem. They seem to declare that they will gradually make the codebase open-source (see warpdotdev/Warp#400 and warpdotdev/Warp@876cca9), but there seems to be no progress after two years.

@aminya I have questions:

  • Q1: Does the problem happen within other terminals with the same .bashrc?
  • Q2: What is the output of the following command? I'd like to get a precise state of the shell variable PROMPT_COMMAND.
$ declare -p PROMPT_COMMAND
  • Q3: Could you put the following lines in ~/.bashrc instead of zoxide and mcfly initialization and see the behavior? Are the lines foo and bar shown before every prompt as expected?
PROMPT_COMMAND=('(echo foo)' '(echo bar)')

@akinomyoga
Copy link
Contributor

  • Q4: Maybe even another setting is interfering. Do you have anything else in your .bashrc/.bash_profile settings? Does the problem reproduce with just the two lines eval "$(zoxide init bash)" and eval "$(mcfly init bash)"?

@aminya
Copy link
Author

aminya commented Aug 12, 2024

Q1: Does the problem happen within other terminals with the same .bashrc?

No, I could not reproduce it in KDE Konsole.

Q2: What is the output of the following command? I'd like to get a precise state of the shell variable PROMPT_COMMAND

The output with only zoxide and mcfly in Warp

declare -p PROMPT_COMMAND
declare -- PROMPT_COMMAND="__zoxide_hook;;mcfly_prompt_command
__bp_trap_string=\"\$(trap -p DEBUG)\"
trap - DEBUG
__bp_install"

The output with only zoxide and mcfly in KDE Console:

> declare -p PROMPT_COMMAND
declare -a PROMPT_COMMAND=([0]="__zoxide_hook;" [1]="mcfly_prompt_command")

The above shows that the check for the array doesn't seem to be enough. Warp, uses a more complex entry in its PROMPT_COMMAND hooks.

Q3: Could you put the following lines in ~/.bashrc instead of zoxide and mcfly initialization and see the behavior? Are the lines foo and bar shown before every prompt as expected?

Yes

foo
bar
  • Q4: Maybe even another setting is interfering. Do you have anything else in your .bashrc/.bash_profile settings? Does the problem reproduce with just the two lines eval "$(zoxide init bash)" and eval "$(mcfly init bash)"?

Yes, I tested with only those two lines, and the issue happened to me.

@akinomyoga
Copy link
Contributor

akinomyoga commented Aug 12, 2024

Thank you for the information! From the answers to Q1, Q4, and Q2 for KDE Konsole, it seems Warp Terminal is at least related. I'm currently trying to install it in my Arch (I'm not sure, but Warp Terminal hangs with the startup window of "Sign up" in my environment). While I attempt the installation, could you also check the behavior with the following setting?

  • Q5: Can a similar error message be reproducible with the following single-line bashrc (without any other settings)?
# bashrc
PROMPT_COMMAND=('(echo foo);' '(echo bar)')

edit:

  • Q6: What is the output of declare -p PROMPT_COMMAND after initializing Bash with the Q5 setup?

@akinomyoga
Copy link
Contributor

Although the suffix ; in PROMPT_COMMAND should be harmless, it seems to trigger the problem when someone (probably Warp Terminal) tries to join the elements of PROMPT_COMMAND by a semicolon ; (which is actually too naive).

When PROMPT_COMMAND is empty at the point when Zoxide is initialized, the suffix ; seems to be added by the Zoxide integration:

if [[ ${PROMPT_COMMAND:=} != *'__zoxide_hook'* ]]; then
    PROMPT_COMMAND="__zoxide_hook;${PROMPT_COMMAND#;}"
fi

@aminya
Copy link
Author

aminya commented Aug 12, 2024

Here's the output with the above in bashrc. I face the same error. I don't get foo/bar

bash: PROMPT_COMMAND: line 706: syntax error near unexpected token `;;'
bash: PROMPT_COMMAND: line 706: `(echo foo);;(echo bar)'
> declare -p PROMPT_COMMAND
declare -- PROMPT_COMMAND="(echo foo);;(echo bar)
__bp_trap_string=\"\$(trap -p DEBUG)\"
trap - DEBUG
__bp_install"

@akinomyoga
Copy link
Contributor

akinomyoga commented Aug 12, 2024

Thank you! Then, I'll have to conclude that this is a bug of Warp Terminal (or a shell setting loaded by Warp Terminal).

Although I still cannot launch the Warp Terminal in my environment (probably because of some X11 Forwarding issue with the SSH connection), but I could extract the Bash integration script from the warp binary file by the following command:

$ strings /path/to/warp | sed -n '/# We need to prepend a space/,/^ *warp_bootstrapped/p' > warp.bash

This script has 1307 lines, but I'll examine suspicious parts related to PROMPT_COMMAND.

@akinomyoga
Copy link
Contributor

akinomyoga commented Aug 12, 2024

I found the following section starting on line 1211 of the Warp shell integration script:

    # If the user's rc files turned PROMPT_COMMAND into an array, we must undo that.
    # Since Bash 5.1, the PROMPT_COMMAND variable can be an array, see:
    #   https://tiswww.case.edu/php/chet/bash/NEWS
    # Unfortunately, doing so will break Warp because of the way it interacts with bash-preexec.
    # When PROMPT_COMMAND is an array, the DEBUG signal fires for each array element, and since
    # bash-preexec uses a DEBUG trap to trigger the preexec functions, it will run our preexec
    # functions before the command at PROMPT_COMMAND[1], PROMPT_COMMAND[2], etc. This means our
    # Preexec hook gets called without the user submitting a command, putting the input block into
    # a broken state, e.g. see https://github.com/warpdotdev/Warp/issues/2636
    # If they end up fixing this, we may be able to remove this at some point, check this:
    #   https://github.com/rcaloras/bash-preexec/issues/130
    #
    # If PROMPT_COMMAND is set and it's an array, "join" the array into a semicolon-delimited string,
    # then overwrite PROMPT_COMMAND
    if [[ -n $PROMPT_COMMAND && "$(declare -p PROMPT_COMMAND)" =~ "declare -a" ]]; then
        PROMPT_COMMAND_FLATTENED=$(IFS="; "; echo "${PROMPT_COMMAND[*]}")
        unset PROMPT_COMMAND
        PROMPT_COMMAND=$PROMPT_COMMAND_FLATTENED
    fi

I have to say the line PROMPT_COMMAND_FLATTENED=$(IFS="; "; echo "${PROMPT_COMMAND[*]}") is wrong. Shell commands cannot be always connected by ;. In the present case, we may drop ; from the first element. However, we cannot connect the commands with ; also when the element ends with &, but we cannot drop & because it changes the meaning.

I searched for the first line of the extracted script on the internet and in the GitHub code search, but nothing was found. This means that the Bash ingtegration script of Warp is indeed closed-source, yet we may ask the upstream Warp project about this issue.

@akinomyoga
Copy link
Contributor

warpdotdev/Warp#5219

@akinomyoga
Copy link
Contributor

@aminya For the time being, could you put the following line at the end of ~/.bashrc?

PROMPT_COMMAND=("${PROMPT_COMMAND[@]%;}")

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 a pull request may close this issue.

2 participants