Bash Shell Integration: Please co-exist with other trap DEBUG handlers (ie: leverage bash-preexec)

[joerohde]

The vscode shell integration currently needs to be turned off or disabled for anyone with complex prompt commands, or any trap DEBUG.

Please consider supporting some kind of chaining model so we can work with the shell integration as needed.

Even better - use the defacto package that has already been written.
VSCode is the 3rd script that wants to mess with prompts and traps - but it's the first to completely ignore https://github.com/rcaloras/bash-preexec

Please consider shipping this along side shellintegration-bash.sh and sourcing/using it. If that is unacceptable, consider at least
detecting this module has been loaded as documented in the repo - and using it:

if [[ -n "${bash_preexec_imported:-}" ]]; then
    precmd_functions+=(__vsc_chained_precmd)
    preexec_functions+=(__vsc_chained_preexec)
else
    # the existing prompt/trap stuff near the end of the file.
fi

fwiw: iterm2 has very very similar functionality to the vscode shell integration, but I've been able to work with it. Although they embedded the script directly, they kept the compatible function names and mechanisms and interop fine.

[Tyriar]

I believe we have already fixed this:

vscode/src/vs/workbench/contrib/terminal/browser/media/shellIntegration-bash.sh

Lines 91 to 105 in 5357dcf

	# capture any debug trap so it is not overwritten
	__vsc_original_trap="$(trap -p DEBUG)"
	if [[ -n "$__vsc_original_trap" ]]; then
	__vsc_original_trap=${__vsc_original_trap#'trap -- '*}
	__vsc_original_trap=${__vsc_original_trap%'DEBUG'}
	fi
	__vsc_preexec() {
	eval ${__vsc_original_trap}
	PS1="$__vsc_prior_prompt"
	if [ -z "${__vsc_in_command_execution-}" ]; then
	__vsc_in_command_execution="1"
	__vsc_command_output_start
	fi
	}

Adding @meganrogge to make sure

[joerohde]

Well, line 100 PS1=... breaks my prompt.
Adding the line echo "$BASH_COMMAND" as the 1st line of __vsc_preexec shows why.

I think perhaps there needs to be 1 more state 'not in prompt' that causes preexec to bail until all prompt_command processing is done.

Alternatively, if this is all to send __vsc_command_output_start - it seems like using PS0 on bash >= 4.4 would be a lot cleaner?

[joerohde]

Hi @meganrogge. This was on 1.67.2 with https://github.com/microsoft/vscode/blob/5357dcfc04ddfce12661dca80ae9a104640a2c7e/src/vs/workbench/contrib/terminal/browser/media/shellIntegration-bash.sh dropped in place.

[Tyriar]

We've made some big changes in the past few days around debug trap and PS1 handling in bash, I believe the issues here are fixed. We may at some point hook into bash-preexec similar to what starship does but for now that will just increase the number of cases we would need to test for (contributions very welcome).

[Tyriar]

Actually I'll keep this open with help wanted to track integrating with bash-preexec.

[joerohde]

OK. Pulling the bash integration script on June-03 - Things look better, but not quite right.

The problems I see (only when DEBUG trap is being used):

• Still seems to be not preserving $? for prompt_command
• Seems to execute the trap twice. repro with setup instructions below: Do 'cd' into some directory. The command '..' should then pop one directory up, but it will try to process twice and pop twice.
• Instead of 'success dots' in the gutter, all commands seem to think there are fail exit status.

Here are my dotfiles: https://github.com/joerohde/dotfiles and a branch to help debug:
https://github.com/joerohde/dotfiles/tree/jrohde/its-a-trap

• The debug branch just build an array of what commands were that triggered the trap and print them at the start and end of building the prompt. It does show the trap being called twice when shell integration is enabled - and once otherwise.

The repro environment/steps:

• Repros in OSX and WSL Ubuntu 22.04 for me.

• It uses dotbot. In a scratch user directory you can run ./install to link everything into ~/.

• Pass --update to ./install to avoid installing extra tooling it finds missing (pygments/tmux/coreutils)

• There is a script that get's sourced local/bin/cd.sh. This is the root of all evil. It's on the hacky side. The function cd_debug_trap is small and is (I believe) the only relevant part. I should stop using it, but it's sooooo ingrained into my muscle memory.

• To verify: Removing [[ -r "$HOME/.local/bin/cd.sh" ]] && . "$HOME/.local/bin/cd.sh" from .bashrc makes everything work great with none of the above mentioned problems.

• One hack, that seems irrelevant is that cd.sh uses debug trap directly, but also detects and uses the pre-exec infrastructure if it has been loaded (to not break iTerm2's shell integration).

I appreciate the time and effort going into this feature. It's becoming a shockingly advanced embedded shell environment.

[Tyriar]

@joerohde thanks again, planning on tackling this in June

[Tyriar]

I use just merged in a fix for this. It would be great if someone could help verify in tomorrow's insiders build as I'm not normally a user of bash-preexec.

[joerohde]

Still some problems around $?. Very simple repro:

clean environment. My bash and vscode versions are:

BASH_VERSION='5.1.16(1)-release'
TERM_PROGRAM_VERSION=1.69.0-insider

use this .bashrc:

#!/bin/bash
shopt -s extdebug

For me (In WSL, repros in plain OSX):

---

Without extdebug

If extdebug works, then this would be the next simplest test (I think). Same clean environment. The following .bashrc

#!/bin/bash

shopt -s extdebug

function my_debug_trap()
{
    false
    return 0
}
trap my_debug_trap DEBUG

[andreamah]

Are there verif steps for this?

[vscodenpa]

This bug has been fixed in the latest release of VS Code Insiders!

@joerohde, you can help us out by commenting /verified if things are now working as expected.

If things still don't seem right, please ensure you're on version 9cf2fab of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

[rzhao271]

With the more complicated trap my_debug_trap DEBUG scenario, the decorations don't seem to show blue and red anymore.

I realize that in the screenshot I forgot to add (), but that didn't make a difference.

[meganrogge]

@rzhao271 what is your exact debug trap when that happens?