"Naumov, Michael (North Sydney)" writes:
You either want to correct the "From: " header that appears in
e-mails from you, or want to start your body of your patch message
like this:
From: Michael Naumov
Some git tools such as GitExtensions for Windows...
if you want the author of the patch and the name you used to sign it
off to match, which is almost always what you want.
> Some git tools such as GitExtensions for Windows use environment
> variable TERM=msys which causes the weird ANSI sequence shown for the
> messages returned from server-side hooks
The above sentence, while it may be telling a truth, feels more or
less irrelevant, especially the part that talks about TERM=msys.
Even if GitExtensions used TERM=vt100, the end result would be the
same, wouldn't it?
If you are suggesting a fix to GitExtensions to make it export
TERM=dumb like everybody else does, that would be a different
story. Mentioning "TERM=msys causes the problem" would be a very
relevant thing to do. But this patch is not that.
> We add those ANSI sequences to help format sideband data on the user's
> terminal. However, GitExtensions is not using a terminal, and the ANSI
> sequences just confuses it. We can recognize this use by checking
> isatty().
This on the other hand is very readable. How about rephrasing these
two like so:
Diagnostic messages received on the sideband #2 from the server side
are sent to the standard error with ANSI terminal control sequence
"\033[K" that erases to the end of line appended at the end of each
line.
However, some programs (e.g. GitExtensions for Windows) read and
interpret and/or show the message without understanding the terminal
control sequences, resulting them to be shown to their end users.
To help these programs, squelch the control sequence when the
standard error stream is not being sent to a tty.
There are programs that drive other programs (not limited to Git)
through pty (hence satisfying isatty(2)) without interpreting the
ANSI terminal control sequences, and it is conventional for these
programs to export TERM=dumb, so and your patch still checks for
TERM=dumb to help them, which is very good.
> See https://github.com/gitextensions/gitextensions/issues/1313 for
> more details
And if you explain it like the above, I do not think this external
reference is very useful.
> NOTE: I considered to cover the case that a pager has already been
> started. But decided that is probably not worth worrying about here,
> though, as we shouldn't be using a pager for commands that do network
> communications (and if we do, omitting the magic line-clearing signal
> is probably a sane thing to do).
Sensible.
> Signed-off-by: Michael Naumov
> Thanks-to: Erik Faye-Lund
> Thanks-to: Jeff King
> ---
> sideband.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sideband.c b/sideband.c
> index d1125f5..7f9dc22 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -30,7 +30,7 @@ int recv_sideband(const char *me, int in_stream, int out)
>
> memcpy(buf, PREFIX, pf);
> term = getenv("TERM");
> - if (term && strcmp(term, "dumb"))
> + if (isatty(2) && term && strcmp(term, "dumb"))
> suffix = ANSI_SUFFIX;
> else
> suffix = DUMB_SUFFIX;
--
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
