<div dir="ltr">On Wed, Aug 7, 2013 at 10:03 AM, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com" target="_blank" class="cremed">clattner@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><div>On Aug 7, 2013, at 8:34 AM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank" class="cremed">chandlerc@gmail.com</a>> wrote:</div>

<blockquote type="cite"><p dir="ltr">You can always pass the flag. This just requires a system library to autodetect that system's color support which doesn't seem unreasonable.</p></blockquote><blockquote type="cite">

<p dir="ltr">The test against the string "dumb" doesn't really make any sense to me. The TERM environment variable is only really meaningful as a key into termcaps or term info which is what we're now querying.</p>

</blockquote></div></div><div>I think that James' point here is that this is a regression for systems that don't autodetect color support.  It might be reasonable for systems to do this, but until someone implements support in libSupport for detecting them, you're regressing the experience for people.  Telling them to just pass a flag to the compiler doesn't seem like the right thing.</div>

</div></blockquote><div><br></div><div>The suggestion to pass the flag was specifically to address that this isn't the *only* way to get colors. Anyways, let's continue discussing the problem of auto-detection, as that seems the more relevant issue.</div>

<div><br></div><div>The first question is whether there is any known practical concern. I'm not aware of anyone using LLVM on a Unix system which doesn't support curses in some form (darwin, linux, and modern BSDs are all in the clear from my limited testing) after doing a reasonable amount of research on the subject. If anyone can report an actual system where this doesn't work, I'll happily add a solution to auto-detect things on that system to libSupport. I don't indent to just drop this on the floor. Thus far, this handles all the Unix systems I'm aware of.</div>

<div><br></div><div>Now, there is a more theoretical question of what should the behavior be if there is not a working curses installation for some reason. Before this patch, Clang would show colors if the TERM environment variable was set unless it was set to magical value, and would not show them if the TERM environment variable was not set. The latter case in unchanged. In the former case, my argument is that if we have a TERM environment variable and are incapable of querying the standard databases for interpreting it we have two wrong choices:</div>

<div>1) Show colors, causing some terminals to show a really ugly set of escape codes if they don't support colors.</div><div>2) Don't show colors, causing some  terminals to have a monochromatic diagnostic experience when they could have a colorful one.</div>

<div><br></div><div>Before, we picked #1, and now we pick #2. I think that is the right decision. It is a much worse experience to have escape codes in your diagnostics than to not have colors. Yes, we regress some people, but we also fix a bug others, one that was actually reported to our team from users. I also have a suspicion (although I have no data as this is a somewhat theoretical question) that systems which don't support the curses library in some form are less likely to have a color-supporting terminal, and thus more likely to be impacted by #1 than #2.</div>

</div></div></div>