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

2013-06-04 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Even better, we can hit a middle ground by abstracting away some of the
 complexity.

The latter half of __git_ps1 is already fairly nice; w/i/s/u/c/p and
friends can serve as the basis of such an abstraction, even though r
does want to be separated further.

I tried the GIT_PS1_SHOW* variables several times, but every time I
did so, I eventually had to give up because I simply couldn't read
and/or remember what */+#/$/% line noises meant.  If we had a custom
collect these pieces and form the final presentation mechanism, I
could easily replace these with mnemonic letters that can be more
easily memorable.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-06-03 Thread Ramkumar Ramachandra
Thomas Rast wrote:
 Do you have other ways of distinguishing the branch and the state?
 Colors?  I'm a bit too lazy to check.  Perhaps it could be made to only
 use caps if not in colored mode?

Currently, no.   See git-prompt.sh:401, 403, 409; we don't have a
separate color for $r.  I didn't introduce a color in this series
because it would conflict with rr/zsh-color-prompt which is in pu: we
can introduce it after that graduates.  I was thinking yellow, since
that's not taken?

You really should use colors.  I don't think it's worth the extra
ugliness to scream in the no-color case.

As such, the prompt is a fine bikeshedding target.  I even introduced
GIT_PS1_SEPARATOR, because some people were unhappy with me stripping
one whitespace (yes, one).  If we go down the configurability road,
we'll either end up with a ton of environment variables, or be made to
write a generalized custom formatter which splices together tons of
arguments using color (super painful).  While I do agree that it is a
matter of taste, we have to make a few compromises for the sake of
sanity.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-06-03 Thread Junio C Hamano
Thomas Rast tr...@inf.ethz.ch writes:

 Ramkumar Ramachandra artag...@gmail.com writes:

 Currently, when performing any operation that saves the state and
 expects the user the continue (like rebase, bisect, am), the prompt
 screams:

   artagnon|completion|REBASE-i 2/2:~/src/git$

 Lowercase the words, so we get a more pleasant

   artagnon|completion|rebase-i 2/2:~/src/git$

 So I'm not sure whether this falls under bikeshedding or actual
 features, but I like the screaming.

Sounds like bikeshedding.

My personal opinion is that detached in the middle of something is
not a normal state, and it deserves to be in bold as a reminder.
Let's leave it as-is.
 

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-06-03 Thread Jeff King
On Mon, Jun 03, 2013 at 03:17:27PM +0530, Ramkumar Ramachandra wrote:

 Thomas Rast wrote:
  Do you have other ways of distinguishing the branch and the state?
  Colors?  I'm a bit too lazy to check.  Perhaps it could be made to only
  use caps if not in colored mode?
 
 Currently, no.   See git-prompt.sh:401, 403, 409; we don't have a
 separate color for $r.  I didn't introduce a color in this series
 because it would conflict with rr/zsh-color-prompt which is in pu: we
 can introduce it after that graduates.  I was thinking yellow, since
 that's not taken?
 
 You really should use colors.  I don't think it's worth the extra
 ugliness to scream in the no-color case.
 
 As such, the prompt is a fine bikeshedding target.  I even introduced
 GIT_PS1_SEPARATOR, because some people were unhappy with me stripping
 one whitespace (yes, one).  If we go down the configurability road,
 we'll either end up with a ton of environment variables, or be made to
 write a generalized custom formatter which splices together tons of
 arguments using color (super painful).  While I do agree that it is a
 matter of taste, we have to make a few compromises for the sake of
 sanity.

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:

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index eaf5c36..1da3833 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -404,6 +404,8 @@ __git_ps1 ()
fi
gitstring=$(printf -- $printf_format $gitstring)
PS1=$ps1pc_start$gitstring$ps1pc_end
+   elif test $(type -t __git_ps1_printer) = function; then
+   __git_ps1_printer
else
# NO color option unless in PROMPT_COMMAND mode
printf -- $printf_format $c${b##refs/heads/}${f:+ 
$f}$r$p

and then you can define your own custom function if you like, which will
inherit all of the variables from __git_ps1. E.g.:

  __git_ps1_printer() {
  echo ${r,,}
  }

yields a lower-case rebase-i. Of course there are many other variables
you want to put in your prompt, too. The only problem I see is that the
current set of variables is not suitable as a user-visible interface.
The names are not good (the branch should be $branch rather than $b,
the operation-in-progress should be $operation or similar rather than
$r, etc), and the formats are not as convenient ($r contains the
full |REBASE-i 6/10, whereas it should be broken down so the printing
function can easily use a case statement).

And of course the variables would need documentation.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-06-03 Thread Ramkumar Ramachandra
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?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-06-03 Thread Jeff King
On Tue, Jun 04, 2013 at 09:14:23AM +0530, Ramkumar Ramachandra wrote:

 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html