Re: [PATCH 1/6] prompt: don't scream continuation state
Jeff King 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
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
Re: [PATCH 1/6] prompt: don't scream continuation state
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
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
Thomas Rast writes: > Ramkumar Ramachandra 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
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
Ramkumar Ramachandra 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. When I first saw these displays, I
thought that the point was that this display is more important than all
the others, so it should be louder and easy to distinguish from the
branch name (common ones, anyway).
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?
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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
