Re: [PATCH 2/4] git-prompt.sh: refactor colored prompt code

2013-06-24 Thread Eduardo R. D'Avila
2013/6/23 SZEDER Gábor :
> I'm wary of relying on tput's availability.  It's part of ncurses,
> which is an essential package in many (most? all?) linux distros, but
> I don't know how it is with other supported platforms.  So I think
> we'd have to stick to the hard-coded escape sequences as a fallback
> anyway. (...)
>
> However, I don't know much about the caveats of terminals, so I can't
> judge the benefits of using tput instead of the escape sequences.

I'm exactly in the same situation...

> considering the additional delay that would be caused by fork()ing
> four subshells and fork()+exec()ing four external commands on Windows.

Well... That would be only once, during script loading.


Given the concerns raised by Gábor (edited and quoted above) and that
there is no known issue (afaik) with the current implementation, I'm
tending to revert to the escape sequences.
--
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 2/4] git-prompt.sh: refactor colored prompt code

2013-06-23 Thread SZEDER Gábor
On Sat, Jun 22, 2013 at 01:45:38PM -0300, Eduardo D'Avila wrote:
> 2013/6/22 Øystein Walle :
> > I've gotten the impression it's better to use tput to generate the escape
> > sequences instead of hardcoding them. So something like:
> >
> > local c_red='\['"$(tput setaf 1)"'\]'
> > local c_green='\['"$(tput setaf 2)"'\]'
> > local c_green='\['"$(tput setaf 4)"'\]'
> > local c_clear='\['"$(tput sgr0)"'\]'
> >
> > which is technically cleaner, if not visually.
> >
> > The problem with that approach is that tput will be run several times for
> > each prompt, so it would be best if the color variables were global. Another
> > thing is that you rely on tput being available.
> 
> Are there any guidelines regarding global shell script variables?

There are a couple of global variables in the completion script, they
are all prefixed with "__git_", e.g. $__git_porcelain_commands.  In
the prompt script we don't have any global variables at the moment,
but if we were to create some, then they should be prefixed similarly.
Or perhaps with "__git_ps1_".  Anyway, in case of a global variable
I'd spell out "color" completely instead of abbreviating it as "c".

> I'm considering doing this:
>__git_c_red="\[$(tput setaf 1 || echo -e '\e[31m')\]"
> which handles the case where tput is not available.

To me 'tput setaf 1' is just a little bit less greek than '\e[31m'.

I'm wary of relying on tput's availability.  It's part of ncurses,
which is an essential package in many (most? all?) linux distros, but
I don't know how it is with other supported platforms.  So I think
we'd have to stick to the hard-coded escape sequences as a fallback
anyway.  And if we can't get rid of these escape sequences completely,
then I don't really see the point of using tput, especially
considering the additional delay that would be caused by fork()ing
four subshells and fork()+exec()ing four external commands on Windows.

However, I don't know much about the caveats of terminals, so I can't
judge the benefits of using tput instead of the escape sequences.  


Gábor

--
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 2/4] git-prompt.sh: refactor colored prompt code

2013-06-22 Thread Eduardo D'Avila
2013/6/22 Øystein Walle :
> I've gotten the impression it's better to use tput to generate the escape
> sequences instead of hardcoding them. So something like:
>
> local c_red='\['"$(tput setaf 1)"'\]'
> local c_green='\['"$(tput setaf 2)"'\]'
> local c_green='\['"$(tput setaf 4)"'\]'
> local c_clear='\['"$(tput sgr0)"'\]'
>
> which is technically cleaner, if not visually.
>
> The problem with that approach is that tput will be run several times for
> each prompt, so it would be best if the color variables were global. Another
> thing is that you rely on tput being available.

Are there any guidelines regarding global shell script variables?

I'm considering doing this:
   __git_c_red="\[$(tput setaf 1 || echo -e '\e[31m')\]"
which handles the case where tput is not available.


Eduardo
--
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 2/4] git-prompt.sh: refactor colored prompt code

2013-06-22 Thread Øystein Walle
Eduardo R. D'Avila  gmail.com> writes:

> + local c_red='\[\e[31m\]'
> + local c_green='\[\e[32m\]'
> + local c_lblue='\[\e[1;34m\]'
> + local c_clear='\[\e[0m\]'
>   fi
> - local c_red='\e[31m'
> - local c_green='\e[32m'
> - local c_lblue='\e[1;34m'
> - local c_clear='\e[0m'

I've gotten the impression it's better to use tput to generate the escape 
sequences instead of hardcoding them. So something like:

local c_red='\['"$(tput setaf 1)"'\]'
local c_green='\['"$(tput setaf 2)"'\]'
local c_green='\['"$(tput setaf 4)"'\]'
local c_clear='\['"$(tput sgr0)"'\]'

which is technically cleaner, if not visually.
 
The problem with that approach is that tput will be run several times for 
each prompt, so it would be best if the color variables were global. Another 
thing is that you rely on tput being available.

Øsse

--
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