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

About shell integration on a dynamic prompt (in bash) #4476

Closed
thanhph111 opened this issue Jan 8, 2022 · 21 comments
Closed

About shell integration on a dynamic prompt (in bash) #4476

thanhph111 opened this issue Jan 8, 2022 · 21 comments

Comments

@thanhph111
Copy link

thanhph111 commented Jan 8, 2022

This is definitely not a bug and probably pretty much your intention, but I'd love to know your opinion on this.

I have always left the shell_integration disabled in my config until today. You know, customizations usually take me a loads of time. However, I also had to let the whole month of testing this feature wasted by contributing nothing to the conversations, my apologies.

In the end I decided to spend this weekend trying and realized that my bash prompt is not recognizable because of this (simplified):

_omp_hook() {
    PS1="$(oh-my-posh --config="$POSH_THEME" --shell=bash"
}

PROMPT_COMMAND="_omp_hook; $PROMPT_COMMAND"

I believe cross-platform prompt generators like oh-my-posh and starship have things done similarly as above. Therefore, our escape sequences prepending/appending is not sufficient as it runs only once in a session.

Indeed, this works for me (with mark feature):

# Integrate after having thing done with _oh-my-posh_
if [ -n "$KITTY_INSTALLATION_DIR" ] &&
    [ -f "$KITTY_INSTALLATION_DIR/shell-integration/bash/kitty.bash" ]; then
    _ksi() {
        local secondary_prompt="\n\[\e]133;A;k=s\a\]"
        PS1=${PS1//"\n"/"$secondary_prompt"}
        PS1="\[\e]133;A\a\]$PS1"
        PS2="\[\e]133;A;k=s\a\]$PS2"
        PS0="\[\e]133;C\a\]$PS0"
    }
    PROMPT_COMMAND="$PROMPT_COMMAND; _ksi"
fi

However, this one doesn't work because we lost KITTY_SHELL_INTEGRATION on the run:

# Integrate after having thing done with _oh-my-posh_
if [ -n "$KITTY_INSTALLATION_DIR" ] &&
    [ -f "$KITTY_INSTALLATION_DIR/shell-integration/bash/kitty.bash" ]; then
    _ksi() {
        source "$KITTY_INSTALLATION_DIR/shell-integration/bash/kitty.bash"
    }
    PROMPT_COMMAND="$PROMPT_COMMAND; _ksi"
fi

As you can see, we can have a workaround at the shell configuration level but this makes me feel a bit uncomfortable because of the code repetition. So, what is your stance on this?

I haven't had a look at other shell since I'm only sticking with bash these days. I've searched in the RFC issue but couldn't find a similar talk. If this has been discussed and decided somewhere, please accept my apologies again and guide me on the way out. Thank you.

@kovidgoyal
Copy link
Owner

Automatic shell integration is designed to be minimally invasive. And
bash is the worst shell (provides the least useable hooks for this). It
may be worth changing the the kitty bash shell integration script to use
PROMPT_COMMAND, that can be investigated.

As for your question, the shell integration scripts are not meant to be
repeatedly sourced, they are meant to be sourced once, and when they do
that, they unset KITTY_SHELL_INTEGRATION, so as to not pollute the
system.

To fix your issue, the bash script would probably need to be re-written
to use PROMPT_COMMAND instead of just setting the PS variables once.

Though I would suggest using one of the other supported shells for a
better experience.

@thanhph111
Copy link
Author

Agree with you, I also think of a minimally invasive approach, like eval "$(kitty --shell-integration bash)", but as you said, it needs more investment. So I think this workaround might make me happy this time, I leave it up to you to decide the perfect timing for other changes. Thanks a lot.

@kovidgoyal
Copy link
Owner

I have changed the kitty bash script to handle PROMPT_COMMAND. Please test.

@wiraki
Copy link

wiraki commented Jan 14, 2022

-bash: declare: -g: invalid option
declare: usage: declare [-aAfFilrtux] [-p] [name[=value] ...]

This outputs upon SSHing to remote system that has this new kitty.bash script.

@kovidgoyal
Copy link
Owner

9dd5410

@wiraki
Copy link

wiraki commented Jan 14, 2022

Thanks, it now works for me (I am using starship, locally and on the remote). Using ctrl+shift+z/x does, jumping to the previous or next command, even after SSHing to the remote.

However, ctrl+shift+g opens in the pager the output of the last command I ran locally before SSHing. Not sure if this is related or whether it should be another issue.

Also not sure if it is relevant: I use zsh locally, bash on the remote.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Jan 14, 2022 via email

@wiraki
Copy link

wiraki commented Jan 14, 2022

Hmm, curious, for me ctrl+shift+g opens pager with output of the last command before SSHing.

Here is the relevant part in my remote .bashrc. The kitty.bash file referred to below is the one you just patched.

# Start Starship prompt
eval "$(~/bin/starship init bash)"

export KITTY_INSTALLATION_DIR="$HOME/.config/remote_kitty"
if test -d "$KITTY_INSTALLATION_DIR"; then
    export KITTY_SHELL_INTEGRATION="enabled"
    source "$KITTY_INSTALLATION_DIR/shell-integration/bash/kitty.bash"
else
    echo "could not find Kitty installation directory"
fi

Should I start the Starship prompt after the Kitty integration part, or it doesn't matter?

@kovidgoyal
Copy link
Owner

On your remote shell is _ksi_debug_print defined?

@thanhph111
Copy link
Author

Work great to me, thank you.

@wiraki
Copy link

wiraki commented Jan 14, 2022

On your remote shell is _ksi_debug_print defined?

Nope, it is unset.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Jan 14, 2022 via email

@wiraki
Copy link

wiraki commented Jan 14, 2022

It does run. And I have noticed more strangeness. Bear with me:

kitty.bash runs

I have modified kitty.bash adding an echo at the end:

❯ tail /home/production/mpauper/.config/remote_kitty/shell-integration/bash/kitty.bash
    elif [[ $(declare -p PROMPT_COMMAND 2> /dev/null) =~ 'declare -a PROMPT_COMMAND' ]]; then
        PROMPT_COMMAND+=("_ksi_prompt_command")
    else
        PROMPT_COMMAND+="; _ksi_prompt_command"
    fi
}
_ksi_main
unset -f _ksi_main

echo "kitty.bash has run!"

Now when I login:

❯ alias ssh   
ssh='kitty +kitten ssh'

❯ alias login1                
login1='ssh <username>@<someIPaddress>'

❯ login1
kitty.bash has run!
kitty.bash has run!

Seems like kitty.bash is actually run twice?

Starship is started before the kitty shell integration

Remote .bashrc content:

❯ tail ~/.bashrc
eval "$(~/bin/starship init bash)"

# kitty shell integration
export KITTY_INSTALLATION_DIR="$HOME/.config/remote_kitty"
if test -d "$KITTY_INSTALLATION_DIR"; then
    export KITTY_SHELL_INTEGRATION="enabled"
    source "$KITTY_INSTALLATION_DIR/shell-integration/bash/kitty.bash"
else
    echo "could not find Kitty installation directory"
fi

Then:

  1. Starship's cmd_duration module acts weird. It measures the time between each time I issue a command, instead of the time the command took to execute.
  2. ctrl+shift+z or x works as expected, jumping to previous or next commands I have run in the remote.
  3. ctrl+shift+g opens a pager with two lines "kitty.bash has run!" (the output from the moment I ssh, until I get the remote prompt)
  4. ctrl+shift+h works as expected, pager with all the content before and after SSH login.

Starship is started after the kitty shell integration

Remote .bashrc content:

❯ tail ~/.bashrc
export KITTY_INSTALLATION_DIR="$HOME/.config/remote_kitty"
if test -d "$KITTY_INSTALLATION_DIR"; then
    export KITTY_SHELL_INTEGRATION="enabled"
    source "$KITTY_INSTALLATION_DIR/shell-integration/bash/kitty.bash"
else
    echo "could not find Kitty installation directory"
fi

# Start Starship prompt
eval "$(~/bin/starship init bash)"

Then:

  1. Starship's cmd_duration module works as expected.
  2. ctrl+shift+z or x only jumps to commands I run before SSH login. I cannot jump between the commands I run on the remote.
  3. ctrl+shift+g opens a pager with everything since I SSH login. Everything in remote is treated as one single output of the SSH login command.
  4. ctrl+shift+h works as expected, pager with all the content before and after SSH login.

I hope there is nothing obvious I have done terribly wrong, if so, my apologies. Thanks for the help so far!

@kovidgoyal
Copy link
Owner

That only tells you the file has been sourced not that _ksi_main has actually run. Look at where _ksi_debug_print is defined. Put an echo there.

@kovidgoyal
Copy link
Owner

I can reproduce the issue with starships command duration if it is run before kitty.bash rather than after. But the rest of your issues I cannot reproduce. I dont know how starship command duration works, but since kitty has to run the history command to set the window title, so this is expected. You can add no-title to KITTY_SHELL_INTEGRATION to confirm.

@kovidgoyal
Copy link
Owner

I took a look at the starship command timing issue and the way they've implemented it is rather fragile and overly complex. All they need to do is append $(starship_preexec) to PS0 no need to use debug traps and assorted nonsense. If they do that it will work fine with kitty's shell integration.

@page-down
Copy link
Contributor

Can you reproduce this issue?

kitty --config=NONE -o shell=bash -o remember_window_size=no -o initial_window_height=10c
  • seq 10
  • seq 3
  • ctrl+shift+g, pager: seq 3 output
  • ctrl+shift+z, scroll to seq 10 output
  • ctrl+shift+g
    • expect: seq 3 output
    • actually: seq 10 output + prompt + seq 3 output

I briefly looked at the prompt mark and it is correct. (with dump_lines_with_attrs)

But it still outputs everything that contains seq 10.
I haven't had time to check it in depth yet.

@kovidgoyal
Copy link
Owner

That doesn't have anything to do with bash. It reproduces with any
shell. It will be a bug in find_cmd_output(). That one is your code so
am leaving it for you.

@wiraki
Copy link

wiraki commented Jan 18, 2022

Hi @kovidgoyal, sorry for the late reply. I did not have access to the remote system these days.

That only tells you the file has been sourced not that _ksi_main has actually run. Look at where _ksi_debug_print is defined. Put an echo there.

I have done the following, adding an echo on line 22 before _ksi_debug_print is defined (also inside the definition but that was pointless):

❯ bat --line-range 7:32 .config/remote_kitty/shell-integration/bash/kitty.bash 
───────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: .config/remote_kitty/shell-integration/bash/kitty.bash
───────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   7   │ _ksi_main() {
   8   │     if [[ $- != *i* ]] ; then return; fi  # check in interactive mode
   9   │     if [[ -z "$KITTY_SHELL_INTEGRATION" ]]; then return; fi
  10   │     set -f
  11   │     for i in ${KITTY_SHELL_INTEGRATION[@]}; do
  12   │         set +f
  13   │         if [[ "$i" == "no-cursor" ]]; then _ksi_prompt[cursor]='n'; fi
  14   │         if [[ "$i" == "no-title" ]]; then _ksi_prompt[title]='n'; fi
  15   │         if [[ "$i" == "no-prompt-mark" ]]; then _ksi_prompt[mark]='n'; fi
  16   │         if [[ "$i" == "no-complete" ]]; then _ksi_prompt[complete]='n'; fi
  17   │     done
  18   │     set +f
  19   │ 
  20   │     unset KITTY_SHELL_INTEGRATION
  21   │ 
  22   │     echo "before _ksi_debug_print definition"
  23   │ 
  24   │     _ksi_debug_print() {
  25   │         # print a line to STDOUT of parent kitty process
  26   │         echo "_ksi_debug_print START"
  27   │         local b=$(printf "%s\n" "$1" | base64 | tr -d \\n)
  28   │         printf "\eP@kitty-print|%s\e\\" "$b"
  29   │         # "
  30   │         echo "_ksi_debug_print END"
  31   │     }

Now when I SSH into remote:

kitty.bash has run!
before _ksi_debug_print definition
kitty.bash has run!

It seems to be then, that _ksi_main does run.

Thanks for looking into this and into starship command duration thing. I'll take your word on your assessment, because my knowledge on the matter is not good enough to understand what you said. However, I am still pondering on these:

  1. Why is "kitty.bash has run!" echoed twice?
  2. In an ideal world, should starship be started before or after the KSI part? Apart from the command duration thing that could be improved on their side, the order of loading does seem to affect the rest of the functionality I mentioned.
  3. It is puzzling that you cannot reproduce it, that would mean there is something else in my setup interfering. Could it be the "mix" of shells (zsh on local, bash on remote). Locally, from my zsh shell, I launched a bash shell, do some command in the bash shell, then try to jump between bash commands with ctrl+shift+g but it behaves like I have mentioned before. I can jump only to commands issued in zsh, everything that happens in the bash shell is considered as the output of the bash command I run in zsh.

@kovidgoyal
Copy link
Owner

From all your symptoms, I am guessing something in your bashrc or
similar is running bash twice.

@page-down
Copy link
Contributor

@kovidgoyal

I found that the title feature does not work on debian-based systems, since it is overridden by the default enabled one.

https://sources.debian.org/src/bash/5.1-2/debian/skel.bashrc/#L69

Try to append it to the end of PS1.

# _ksi_set_mark start_title ...
_ksi_prompt_command() {
# ...
PS1=${PS1//\\\[\\e\]133;k;start_title_kitty\\a\\\]*end_title_kitty\\a\\\]}
PS1="$PS1\[\e]133;k;start_title_kitty\a\]\[\e]2;\w\a\]\[\e]133;k;end_title_kitty\a\]"
# ...
}
# builtin unset _ksi_prompt[start_title_mark] ...

Do you think there is a need for improvement here?

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