Fwd: [PATCH] Remove terminal symbols from non-terminal console

Michael Naumov· Nov 5, 2012, 23:17 UTC · lore

As per discussion on msysgit user group: https://groups.google.com/forum/?fromgroups=#!topic/msysgit/U_a982_a3rc/discussion we found the following patch is required to get rid of weird terminal characters for other tools such as GitExtensions for Windows

---8<---

Show changes to sideband.c +1 −1

diff --git a/sideband.c b/sideband.c
index d5ffa1c..bd3e5a8 100644
--- a/sideband.c
+++ b/sideband.c
@@ -29,7 +29,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(out) && term && strcmp(term, "dumb"))
  suffix = ANSI_SUFFIX;
  else
  suffix = DUMB_SUFFIX;
---8<---

Jeff King· Nov 8, 2012, 16:51 UTC · re: Michael Naumov · lore

Re: Fwd: [PATCH] Remove terminal symbols from non-terminal console

On Tue, Nov 06, 2012 at 10:17:21AM +1100, Michael Naumov wrote:

Show 6 quoted lines

> As per discussion on msysgit user group:
> https://groups.google.com/forum/?fromgroups=#!topic/msysgit/U_a982_a3rc/discussion
> we found the following patch is required to get rid of weird terminal
> characters for other tools such as GitExtensions for Windows
> 
> ---8<---

Please follow SubmittingPatches (missing signoff, weird use of scissors marker that cuts out your commit message, and your commit message should summarize the discussion).

Show 9 quoted lines

> @@ -29,7 +29,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(out) && term && strcmp(term, "dumb"))
>   suffix = ANSI_SUFFIX;
>   else
>   suffix = DUMB_SUFFIX;

Is that right? The "out" fd is where we send sideband 1, and it is almost always not going to be a tty. The suffix should be sent with sideband 2, which goes to stderr. So I'd think you would want to check isatty(2).

Also, most isatty checks also need to cover the case that a pager has already been started. 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).

I think the overall goal is a step in the right direction, though.

-Peff

patchFwd: [PATCH] Remove terminal symbols from non-terminal console