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: seeing extra leading character '[' in command output #4494

Closed
jparise opened this issue Jan 3, 2025 · 24 comments
Closed

bash: seeing extra leading character '[' in command output #4494

jparise opened this issue Jan 3, 2025 · 24 comments

Comments

@jparise
Copy link
Collaborator

jparise commented Jan 3, 2025

Discussed in #3566

Originally posted by jharshman December 27, 2024
After compiling from source and running I am seeing an extra character in the output of every command. Is anyone else experiencing this? Shell is Bash and no modifications to the PS1.

[josh@serenity ghostty]$ echo hi
[hi
[josh@serenity ghostty]$ pwd
[/home/josh/Projects/ghostty
info: ghostty version=1.0.1-dev+0000000
info: ghostty build optimize=ReleaseFast
info: runtime=apprt.Runtime.gtk
info: font_backend=font.main.Backend.fontconfig_freetype
info: dependency harfbuzz=8.4.0
info: dependency fontconfig=21402
info: renderer=renderer.OpenGL
info: libxev backend=main.Backend.io_uring
info(os): setlocale from env result=en_US.UTF-8
info(gtk): GTK version build=4.12.4 runtime=4.16.5
```</div>
@mitchellh
Copy link
Contributor

@jparise Thanks for converting. I can't tell from the discussion but did you get a lead on what is causing this? More importantly: do you have an easy reproduction?

@mitchellh mitchellh changed the title Seeing extra leading character '[' in command output bash: seeing extra leading character '[' in command output Jan 3, 2025
@jparise
Copy link
Collaborator Author

jparise commented Jan 3, 2025

@jparise Thanks for converting. I can't tell from the discussion but did you get a lead on what is causing this? More importantly: do you have an easy reproduction?

Unfortunately, no to both. I tried a bunch of things myself (details in the discussion), but I haven't been able to reproduce or identify the root cause. Multiple people have seen it in various Linux environments though.

@jharshman
Copy link

@jparise I'm happy to assist in creating an environment in which this is reproducible. Let me know if you want an extra set of hands on this.

@jparise
Copy link
Collaborator Author

jparise commented Jan 5, 2025

@jparise I'm happy to assist in creating an environment in which this is reproducible. Let me know if you want an extra set of hands on this.

Yes, that would be very helpful. Thanks for the offer!

@vhodges
Copy link

vhodges commented Jan 5, 2025

I have replicated the issue outside of Ghostty (eg in Gnome Terminal) on NixOS.

Setup:

mkdir -p temp/share/ghostty/shell-integration/bash/
cp $GHOSTTY_RESOURCES_DIR/shell-integration/bash/* temp/share/ghostty/shell-integration/bash/ #(or from src/shell-integration/bash)

# Then run the following (I put them into a test.sh file I could 'source' repeatably)

export GHOSTTY_BASH_INJECT=true
export GHOSTTY_RESOURCES_DIR=$HOME/temp/share/ghostty
builtin source $GHOSTTY_RESOURCES_DIR/shell-integration/bash/ghostty.bash

I tried putting traces before and after all of the printf calls as well as the PS* entries and the '[' still prints before all of them. I don't believe the issue to be related to the prompt.

@vhodges
Copy link

vhodges commented Jan 6, 2025

I had a bit of time this morning and have tracked down the issue. Long story short, It's an interaction with vte.sh and the PS0 prompt it sets, however I do not understand what the exact issue their PS0 which is set to \e]133;C\e\\\r

Setting PS0 to "" at the top of the if [ -n "$GHOSTTY_BASH_INJECT" ]; then block makes the issue go away (though I am not sure this is the right fix).

This explains why it exhibits the issue when launched from gnome-terminal (or any VTE based terminal emulator) but not from the DE launcher menu. Launching from xterm also works as expected without the extra output.

@jparise
Copy link
Collaborator Author

jparise commented Jan 6, 2025

I had a bit of time this morning and have tracked down the issue. Long story short, It's an interaction with vte.sh and the PS0 prompt it sets, however I do not understand what the exact issue their PS0 which is set to \e]133;C\e\\\r

Great! That's super helpful, and I really appreciate the time you spent tracking down the root cause.

I'll spend some time thinking about the best solution.

@jparise
Copy link
Collaborator Author

jparise commented Jan 6, 2025

A few things I've learned:

  1. In (all?) of these environments, vte.sh appears to be installed system-wide (e.g. as /etc/profile.d/vte.sh). This means its "automatically" sourced as part of bash's invocation, although this behavior seems to vary across different distributions' interpretation of "login shell". Ghostty takes the position that "profile" scripts are only sourced for login shells (shopt login_shell), which I believe to be correct with regard to bash's native behavior.
    # See also: run_startup_files() in shell.c in the Bash source code
    if builtin shopt -q login_shell; then
    if [[ $ghostty_bash_inject != *"--noprofile"* ]]; then
    [ -r /etc/profile ] && builtin source "/etc/profile"
    for rcfile in "$HOME/.bash_profile" "$HOME/.bash_login" "$HOME/.profile"; do
    [ -r "$rcfile" ] && { builtin source "$rcfile"; break; }
    done
    fi
  2. However, it appears to only do work when VTE_VERSION is set, so I would expect it to not do anything when run within Ghostty, unless we're inheriting the previous environment? We should verify this.
  3. vte.sh appears to add its functionality to PROMPT_COMMAND. I don't see where it manipulates PS0.
  4. The string you're seeing in PS0 (\e]133;C\e\\\r) is a semantic prompt sequence. Is this being set by vte.sh or something else? We send a similar sequence (but not in PS0):
    builtin printf "\e]133;C;\a"

@vhodges, would you be able to take a closer look at these items to verify my understanding?

@jparise
Copy link
Collaborator Author

jparise commented Jan 6, 2025

These are the two places where we use PS0 ourselves:

We could also narrow down the source of the conflict by disabling each/both of those shell integration features. @mattxtaz noticed that this could be limited to cursor: #3566 (comment)

@vhodges
Copy link

vhodges commented Jan 6, 2025

Oh sorry, I forgot to mention I isolated it to the cursor related code.

On my system, line 116 sets PS0

vte.sh does bailout early if it's a non-interactive shell. It also exits if it's not running in a VTE terminal emulator.

This is why I think the issue does not occur when running Ghostty from the DE or from an xterm. Launching a gnome-terminal (by definition interactive) and starting Ghostty from that shell is inheriting the env from it (and so PS0). It's not a conflict so much as a weird side effect of launching it from an existing terminal.

Launching Ghostty from a gnome-terminal and running the following from the Ghostty terminal window indicates the env is inherited. Perhaps the fix is to unset VTE_VERSION so the vte.sh code just exits. Let me try that... [EDIT: Does not work and of course it would never work since it's already been run]

[vhodges@sulu:~]$ env | grep  VTE
[VTE_VERSION=7802

Here is the bash specific code from vte.sh (on my system)

  74 if [[ -n "${BASH_VERSION:-}" ]]; then
     75 
     76     # Newer bash versions support PROMPT_COMMAND as an array. In this case
     77     # only add the __vte_osc7 function to it, and leave setting the terminal
     78     # title to the outside setup.
     79     # On older bash, we can only overwrite the whole PROMPT_COMMAND, so must
     80     # use the __vte_prompt_command function which also sets the title.
     81 
     82     if [[ "$(declare -p PROMPT_COMMAND 2>&1)" =~ "declare -a" ]]; then
     83         PROMPT_COMMAND+=(__vte_precmd)
     84         PROMPT_COMMAND+=(__vte_osc7)
     85     else
     86         PROMPT_COMMAND="__vte_prompt_command"
     87     fi
     88     PS0=$(__vte_termprop_signal "vte.shell.preexec")
     89 
     90     # Shell integration
     91     if [[ "$PS1" != *\]133\;* ]]; then
     92 
     93         # Enclose the primary prompt between
     94         # ← OSC 133;D;retval ST (report exit status of previous command)
     95         # ← OSC 133;A ST (mark beginning of prompt)
     96         # → OSC 133;B ST (mark end of prompt, beginning of command line)
     97         PS1='\[\e]133;D;$?\e\\\e]133;A\e\\\]'"$PS1"'\[\e]133;B\e\\\]'
     98 
     99         # Prepend OSC 133;L ST for a conditional newline if the previous
    100         # command's output didn't end in one.
    101         # This is not done here by default, in order to provide the default
    102         # visual behavior of shells. Uncomment if you want this feature.
    103         #PS1='\[\e]133;L\e\\\]'"$PS1"
    104 
    105         # iTerm2 doesn't touch the secondary prompt.
    106         # Konsole encloses it between 133;A and 133;B.
    107         # For efficient jumping between commands, we follow iTerm2 by default
    108         # and don't mark PS2 as prompt. Uncomment if you want to mark it.
    109         #PS2='\[\e]133;A\e\\\]'"$PS2"'\[\e]133;B\e\\\]'
    110 
    111         # Mark the beginning of the command's output by OSC 133;C ST.
    112         # '\r' ensures that the kernel's cooked mode has the right idea of
    113         # the column, important for handling TAB followed by BS keypresses.
    114         # Prepend to the user's PS0 to preserve whether it ends in '\r'.
    115         # Note that bash doesn't support the \[ \] markers here.
    116         PS0='\e]133;C\e\\\r'"${PS0:-}"
    117     fi
    118 
    119 elif [[ -n "${ZSH_VERSION:-}" ]]; then
 

@jcollie
Copy link
Collaborator

jcollie commented Jan 6, 2025

Perhaps the fix is to unset VTE_VERSION so the vte.sh code just exits. Let me try that...

We already have some code to "clean up" certain environment variables before launching a new shell, perhaps we should unset VTE_VERSION there?

ghostty/src/termio/Exec.zig

Lines 836 to 864 in 3461204

// Set environment variables used by some programs (such as neovim) to detect
// which terminal emulator and version they're running under.
try env.put("TERM_PROGRAM", "ghostty");
try env.put("TERM_PROGRAM_VERSION", build_config.version_string);
// When embedding in macOS and running via XCode, XCode injects
// a bunch of things that break our shell process. We remove those.
if (comptime builtin.target.isDarwin() and build_config.artifact == .lib) {
if (env.get("__XCODE_BUILT_PRODUCTS_DIR_PATHS") != null) {
env.remove("__XCODE_BUILT_PRODUCTS_DIR_PATHS");
env.remove("__XPC_DYLD_LIBRARY_PATH");
env.remove("DYLD_FRAMEWORK_PATH");
env.remove("DYLD_INSERT_LIBRARIES");
env.remove("DYLD_LIBRARY_PATH");
env.remove("LD_LIBRARY_PATH");
env.remove("SECURITYSESSIONID");
env.remove("XPC_SERVICE_NAME");
}
// Remove this so that running `ghostty` within Ghostty works.
env.remove("GHOSTTY_MAC_APP");
}
// Don't leak these environment variables to child processes.
if (comptime build_config.app_runtime == .gtk) {
env.remove("GDK_DEBUG");
env.remove("GDK_DISABLE");
env.remove("GSK_RENDERER");
}

@vhodges
Copy link

vhodges commented Jan 6, 2025

unsetting VTE_VERSION does not work since PS0 is already set by vte.sh before running Ghostty (obvious in hindsight)

@jcollie
Copy link
Collaborator

jcollie commented Jan 6, 2025

unsetting VTE_VERSION does not work since PS0 is already set by vte.sh before running Ghostty (obvious in hindsight)

What if you unset VTE_VERSION and PS* before launching Ghostty? It woudn't be hard to check for that in the code.

@vhodges
Copy link

vhodges commented Jan 6, 2025

Ah yes, unsetting VTE_VERSION before starting Ghostty works.

@jparise
Copy link
Collaborator Author

jparise commented Jan 6, 2025

Ah yes, unsetting VTE_VERSION before starting Ghostty works.

But this is outside the scope of what we can do (as a project) to help this situation, right?

@jcollie
Copy link
Collaborator

jcollie commented Jan 6, 2025

Ah yes, unsetting VTE_VERSION before starting Ghostty works.

But this is outside the scope of what we can do (as a project) to help this situation, right?

We already do some of this. It's arguable if this is "in scope" or "out of scope". I think that by injecting our integration into the shell it's in scope to make this work as it's obviously caused a lot of issues for people.

Also, removing the "VTE_VERSION" string prevents us from advertising that the TUI app is running in a VTE-based terminal emulator.

ghostty/src/termio/Exec.zig

Lines 836 to 864 in 3461204

// Set environment variables used by some programs (such as neovim) to detect
// which terminal emulator and version they're running under.
try env.put("TERM_PROGRAM", "ghostty");
try env.put("TERM_PROGRAM_VERSION", build_config.version_string);
// When embedding in macOS and running via XCode, XCode injects
// a bunch of things that break our shell process. We remove those.
if (comptime builtin.target.isDarwin() and build_config.artifact == .lib) {
if (env.get("__XCODE_BUILT_PRODUCTS_DIR_PATHS") != null) {
env.remove("__XCODE_BUILT_PRODUCTS_DIR_PATHS");
env.remove("__XPC_DYLD_LIBRARY_PATH");
env.remove("DYLD_FRAMEWORK_PATH");
env.remove("DYLD_INSERT_LIBRARIES");
env.remove("DYLD_LIBRARY_PATH");
env.remove("LD_LIBRARY_PATH");
env.remove("SECURITYSESSIONID");
env.remove("XPC_SERVICE_NAME");
}
// Remove this so that running `ghostty` within Ghostty works.
env.remove("GHOSTTY_MAC_APP");
}
// Don't leak these environment variables to child processes.
if (comptime build_config.app_runtime == .gtk) {
env.remove("GDK_DEBUG");
env.remove("GDK_DISABLE");
env.remove("GSK_RENDERER");
}

@vhodges-ttc
Copy link

My thought was that it's a reasonably clean solution as suggested by @jcollie to unset it before starting the shell since there's no reason to have that code run for Ghostty and this would ensure that (unless they change vte.sh to not check for it).

I do agree that it's a less than ideal thing to have to do to work around something that is so invasive to peoples systems that is unrelated otherwise to this project.

@jparise
Copy link
Collaborator Author

jparise commented Jan 6, 2025

Ah, I took "before starting Ghostty" as literally meaning "before any part of Ghostty runs". It sounds like "before starting Ghostty" means "before executing the command process".

I'm totally aligned with removing VTE_VERSION from the command environment for the reasons given above. I just wasn't sure if that would solve this particular problem based on what @vhodges said here:

unsetting VTE_VERSION does not work since PS0 is already set by vte.sh before running Ghostty

@vhodges-ttc
Copy link

vhodges-ttc commented Jan 6, 2025

Sorry, again I was not clear. Unsetting it in ghostty.bash did not work but doing that before starting ghostty did but this just removes it from the env of the command process without having to recompile. (Posting from work)

@anund
Copy link
Contributor

anund commented Jan 7, 2025

@vhodges would you mind looking over this vm gist. I can't reproduce the behaviour you're seeing with that vm definition. Perhaps I'm missing something else specific to your system?

Testing steps taken:

  • launch gnome-terminal from the system start menu
  • source test-vte.sh
  • see cursor change from block to line (so the script did ~somethings)
  • don't see extra characters

mitchellh added a commit that referenced this issue Jan 7, 2025
This variable is set by gnome-terminal and other VTE-based terminals. We
don't want our child processes to think we're running under VTE.

See #4494
@vhodges
Copy link

vhodges commented Jan 7, 2025

@anund I ran the VM and indeed it does not exhibit the issue! One difference (that I would not think matter): I am running X11/Cinnamon DE. Other than that I am at a loss.

@jparise
Copy link
Collaborator Author

jparise commented Jan 7, 2025

The latest tip release includes #4710. It would be fantastic if that addressed the root cause of this issue.

@vhodges
Copy link

vhodges commented Jan 7, 2025

I can confirm 1.0.2 fixes the issue I was seeing.

@jparise
Copy link
Collaborator Author

jparise commented Jan 7, 2025

Given that confirmation, I'm going to close this as fixed. Yay!

We can reopen/recreate if we discover any other cases that need further investigation.

@jparise jparise closed this as completed Jan 7, 2025
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

7 participants