Re: [PATCH 1/6] prompt: don't scream continuation state

From: Jeff King <peff@peff.net>
Date: Jun 4, 2013, 04:38 UTC
Message-ID: <20130604043836.GA3931@sigill.intra.peff.net>
In-Reply-To: <CALkWK0=5XecaMpPPQgvJZTwR2QUUWVqbZK5p7af9R1HjA0U=OQ@mail.gmail.com>

On Tue, Jun 04, 2013 at 09:14:23AM +0530, Ramkumar Ramachandra wrote:

Show 9 quoted lines

> Jeff King wrote:
> > It seems silly to argue about output formats when we are writing a
> > prompt in a convenient Turing-complete scripting language already.
> > What about something like:
> 
> Could you have a look at __git_ps1_colorize_gitstring from
> rr/zsh-color-prompt in pu?  In the general case, wouldn't the user
> need to re-implement this entire function (with so many variables) in
> her ~/.zshrc?  Isn't it horribly painful for too little gain?

It is a lot of ugly code, but then, complexity and flexibility are a tradeoff. You would not _have_ to use a custom function at all. But if we make it an option, and leave the existing code as the default function, then only those who want it have to use it.

Even better, we can hit a middle ground by abstracting away some of the complexity. You could probably write colorize() function to handle colorizing a particular string (and handle bash/zsh magic), and then pull the whole "which color is the branch" function into a colorize_branch(), and so forth.

I haven't looked that closely, so there may be hidden troubles awaiting, but I don't see why you couldn't ultimately let the user write something like:

  __git_ps1_printer() {
          echo ' ('
          test "$bare" = t && echo "BARE:"; # or "bare:" :)
          color_branch "$(short_ref $branch)" "$detached"
          test "$wt_changes" = t && colorize red "*"
          ...
          echo ')'
  }

And if that is too much, you can abstract bits of it out. The point is that if we can expose the lowest-level details, we can compose functions around them at whatever level is appropriate, and callers can mix and match for the balance of complexity and flexibility they want.

Perhaps it is overkill, but I also think it may make the code itself more readable. For example, I put a "..." above because the next line would handle this:

  if [ -n "$i" ]; then
          gitstring="$gitstring\[$ok_color\]$i"
  fi

I have no clue what that does, or what the global variable "$i" contains. Reading through __git_ps1, it looks like it will be "+" for index changes. Being able to write:

  test "$index_changes" = t && colorize green "+"

seems much more readable to me. And we could probably compose a:

  colorize_diff_state() {
          test "$wt_changes" = t && colorize red "*"
          test "$index_changes" = t && colorize green "+"
  }

for people who just want to take the default colors and characters.

-Peff