major changes to bash_startup.in

[geoff-nixon]

Well, I started on fish, but then I realized I really needed to figure out what was supposed to be happening, and started looking at bash for the "reference material". But I needed to do a lot of cleanup here so I could read it. By the time I was finished there was so much cruft, it ended up basically a rewrite.

Let me know if I dropped anything that was important. I don't think so, but... :worried:

Anyway lots of these changes are for ksh/POSIX compatibility and will translate over fully. Those will be a little tricky because you have to do all the work within PS1 and PS2, but it usually works out.

Anyway, this should be much faster too, eliminated lots of redundancies. It seems to be working for me. Can you test?

[gnachman]

Thanks for taking on this rather gnarly cleanup. I wrote lots of nitpicky comments because this code is kind of scary (not my baliwick!) and it's really hard to test all the crazy things you find in the wild, so you have to be a little paranoid.

[geoff-nixon]

I wrote lots of nitpicky comments because this code is kind of scary (not my baliwick!) and it's really hard to test all the crazy things you find in the wild, so you have to be a little paranoid.

Yeah, unfortunately I only looked at the original somewhat late in the process, so it was hard to tell your comments from the original in places. I'll bring what we need back in/do some more documentation.

[geoff-nixon]

This is taking a bit longer that I thought... might be a day or two till I can work through it all.

[gnachman]

No worries, thanks for taking the time to fix this :)

[geoff-nixon]

Sorry, I haven't forgot about this! Last week was kinda crazy for me, and this week may be as well. However, a bit of good news:

• I've worked out a lowest-common-denominator POSIX-compliant way of doing basic shell integration, using only PS1 and some functions.

Sourcing the following works with bash-as-sh (version 2+), dash, mksh, ksh (93), pkdsh, yash.

_it2a(){ status=$?; printf '\001'; return $status ;}
_it2b(){ status=$?; printf '\002'; return $status ;}
_it2c(){ status=$?; printf '\015'; return $status ;} 
PS1='$(_it2a)$(_it2c)$(itput -cd $?; itput -a)$(_it2b)'$PS1'$(_it2a)$(itput -b)$(_it2b)'
readonly PS1

itput is just an abstraction of some some of the escape sequences wrapped up in a script to keep me form going nuts; there's no reason this need to live in a separate script per se:

#!/bin/sh
for last; do :; done

iterm2_hostname=$(hostname -f)
iterm2_print_user_vars() { :; }
iterm2_print_state_data() {
  printf "\033]1337;RemoteHost=$USER@$iterm2_hostname\007"
  printf "\033]1337;CurrentDir=$PWD\007"
  iterm2_print_user_vars
}

iterm2_prompt_start(){        printf "\033]133;A\007"    ; }
iterm2_prompt_end(){          printf "\033]133;B\007"    ; }
iterm2_before_cmd_executes(){ printf "\033]133;C\007"    ; }
iterm2_after_cmd_executes(){  printf "\033]133;D;$*\007" 
  iterm2_print_state_data
}

iterm2_set_user_var() {
  printf "\033]1337;SetUserVar=%s=%s\007" "$1" $(printf "%s" "$2" | base64)
  }

OPTIND=1
while getopts "abcd" itput_opt; do
  case "$itput_opt" in
    a) iterm2_prompt_start ;;
    b) iterm2_prompt_end   ;;
    c) iterm2_before_cmd_executes ;;
    d) iterm2_after_cmd_executes "$last" ;;
  esac
done
shift $((OPTIND - 1))

Setting PS1 to readonly is ugly, but if there's no other hook to use, its the only way to keep it from being overwritten. Also, probably should add a SIGINT trap since a 'ctrl-c' on an incomplete line causes the return value to change.

I've also looked into the various features of the most common shells, and there's a means to do a more robust integration in ksh93 (it has the pseudo-signal DEBUG trap) and mksh has a special operator ${| } that can be used to do the same thing.

The ^A/^B/CR trick is from the mksh bible manual, page 12; it accomplishes the same thing as the bash \[ \] construct.

[geoff-nixon]

Also, I believe GNU screen always sets the variable STY, so its probably possible to avoid having to check $TERM at all.

[gnachman]

Regarding the readonly PS1, can the user still change it? With the current code changes to PS1 get picked up, and this was added in response to a lot of user feedback, so it can't be given up.

An environment var list STY is less likely to get propagated by ssh than $TERM; plus you'd need another check for tmux in addition to screen.

This looks pretty elegant and I like where it's going so far.

[geoff-nixon]

Leaving aside tmux and friends for the moment:

Regarding the readonly PS1, can the user still change it? With the current code changes to PS1 get picked up, and this was added in response to a lot of user feedback, so it can't be given up.

The following (just the one file now) works identically in bash, bash-as-sh, ksh[93], and zsh, and abides any changes to PS1. In the rest of the family - pdksh, dash, mksh - which really aren't meant to be used as interactive shells anyway - we can make PS1 read-only, or just let shell integration die if the user changes the prompt.

[ -z "$ZSH_VERSION" ] || setopt PROMPT_SUBST

itput(){
 for last; do :; done

  iterm2_print_user_vars() { :; }

  : ${iterm2_hostname:=${HOSTNAME:-$(hostname -f)}}

  iterm2_print_state_data() {
    printf "\033]1337;RemoteHost=$USER@$iterm2_hostname\007"
    printf "\033]1337;CurrentDir=$PWD\007"
    iterm2_print_user_vars
  }

  iterm2_prompt_start(){        printf '\033]133;A\007'    ;}
  iterm2_prompt_end(){          printf '\033]133;B\007'    ;}
  iterm2_before_cmd_executes(){ printf "\033]133;C\007"    ;}
  iterm2_after_cmd_executes(){  printf "\033]133;D;$*\007" 
                                iterm2_print_state_data ;}

  iterm2_set_user_var() {
    printf "\033]1337;SetUserVar=%s=%s\007" "$1" $(printf "%s" "$2" | base64) ;}

  OPTIND=1
  while getopts "abcd" itput_opt; do
    case "$itput_opt" in
      a) iterm2_prompt_start ;;
      b) iterm2_prompt_end   ;;
      c) iterm2_before_cmd_executes ;;
      d) iterm2_after_cmd_executes "$last" ;;
    esac
  done
  shift $((OPTIND - 1))
}

_it2a(){ retstatus=$?; printf '\001'; return $retstatus ;}
_it2b(){ retstatus=$?; printf '\002'; return $retstatus ;}
_it2c(){ retstatus=$?; printf '\015'; return $retstatus ;} 

it2_rotate_ps1(){
  [ x"$PS1" != x"$OLD_PS1" ] && PS1='$(_it2a)$(_it2c)$(itput -cd $?; itput -a)$(_it2b)'$PS1'$(_it2a)$(itput -b)$(_it2b)' && OLD_PS1=$PS1
}

it2_rotate_ps1

trap 'tput -T $TERM el1' INT # Squash bad return values on ctrl-C.
trap it2_rotate_ps1 DEBUG >/dev/null 2>&1 || readonly PS1 # Or, lose integration.

Basically, the only bit that's needed is the DEBUG psedo-trap to survive changes to PS1. This is still crufty - but compared to the "bash pre-exec" code path, it already makes 1/4 the number of executions per prompt.

So if you think this is a direction you'd like take this (i.e., generic code for bash, zsh, and basically anything else that's not fish or tcsh), I'll try to clean it up and push a new commit to this PR?

[gnachman]

Sorry for the delay in getting back to you. Real life intervened for a bit.

How much does it complicate the script to bring zsh in? I don't mind keeping it separate since there are a lot of zsh users in the world, and it could be useful to do zsh-specific customizations some day.

There are a few things I don't understand. Comments inline:

[ -z "$ZSH_VERSION" ] || setopt PROMPT_SUBST 

Setting PROMPT_SUBST seems like trouble. It'll change how existing PS1's are interpreted. If we had a separate script for zsh could this be avoided? One of the most important goals in my life is to reduce the number of support queries I get.

itput(){
 for last; do :; done

  iterm2_print_user_vars() { :; }

  : ${iterm2_hostname:=${HOSTNAME:-$(hostname -f)}}

Just out of curiosity what does the leading colon do here?

  iterm2_print_state_data() {
    printf "\033]1337;RemoteHost=$USER@$iterm2_hostname\007"
    printf "\033]1337;CurrentDir=$PWD\007"

It's not safe to put user-supplied data in the format string of printf, since it could have %whatevers in it that confuse printf.

    iterm2_print_user_vars
  }

  iterm2_prompt_start(){        printf '\033]133;A\007'    ;}
  iterm2_prompt_end(){          printf '\033]133;B\007'    ;}
  iterm2_before_cmd_executes(){ printf "\033]133;C\007"    ;}
  iterm2_after_cmd_executes(){  printf "\033]133;D;$*\007" 
                                iterm2_print_state_data ;}

  iterm2_set_user_var() {
    printf "\033]1337;SetUserVar=%s=%s\007" "$1" $(printf "%s" "$2" | base64) ;}

  OPTIND=1
  while getopts "abcd" itput_opt; do
    case "$itput_opt" in
      a) iterm2_prompt_start ;;
      b) iterm2_prompt_end   ;;
      c) iterm2_before_cmd_executes ;;
      d) iterm2_after_cmd_executes "$last" ;;
    esac
  done
  shift $((OPTIND - 1))
}

_it2a(){ retstatus=$?; printf '\001'; return $retstatus ;}
_it2b(){ retstatus=$?; printf '\002'; return $retstatus ;}
_it2c(){ retstatus=$?; printf '\015'; return $retstatus ;} 

I don't understand the purpose of putting control characters in the prompt. What's this for?

it2_rotate_ps1(){
  [ x"$PS1" != x"$OLD_PS1" ] && PS1='$(_it2a)$(_it2c)$(itput -cd $?; itput -a)$(_it2b)'$PS1'$(_it2a)$(itput -b)$(_it2b)' && OLD_PS1=$PS1

I think this is wrong. The order this does is:

1. iterm2_before_cmd_executes
2. iterm2_after_cmd_executes
3. iterm2_prompt_start
4. $PS1
5. iterm2_prompt_end
6. User enters command
7. Command produces output

The order it needs to be in is:

1. iterm2_after_cmd_executes
2. iterm2_prompt_start
3. $PS1
4. iterm2_prompt_end
5. User enters command
6. iterm2_before_cmd_executes
7. Command produces output

}

it2_rotate_ps1

trap 'tput -T $TERM el1' INT # Squash bad return values on ctrl-C.

Why do we need this?

trap it2_rotate_ps1 DEBUG >/dev/null 2>&1 || readonly PS1 # Or, lose integration.