Fallback option for colorized output when terminfo isn't available

[phosek]

Summary

Try to detect the terminal color support by checking the value of the
TERM environment variable. This is not great, but it's better than
nothing when terminfo library isn't available, which may still be the
case on some Linux distributions.

[rprichard]

FWIW: Someone filed an issue against the Android NDK complaining about the ncurses/libtinfo dependency. It prevented Clang from running out-of-the-box on Arch Linux: https://github.com/android-ndk/ndk/issues/574.

[jyknight]

I'd like it better if you just removed the "#ifdef HAVE_TERMINFO" branch of code entirely, along the cmake support for causing it to be defined and adding TERMINFO_LIBS to llvm. (lldb still looks like it uses it, so can't just remove everything related to terminfo from the cmake support files)

Both GCC and Git simply use the TERM is set and != "dumb" check, and never touch curses. If it's good enough for them, it's good enough for llvm too, and we don't need to extra complexity and dependencies. Also, the hacks in this code are ugly (writing our own function signatures for these functions instead of using a proper #include!).

[phosek]

In D42055#980742, @jyknight wrote:

I'd like it better if you just removed the "#ifdef HAVE_TERMINFO" branch of code entirely, along the cmake support for causing it to be defined and adding TERMINFO_LIBS to llvm. (lldb still looks like it uses it, so can't just remove everything related to terminfo from the cmake support files)

Both GCC and Git simply use the TERM is set and != "dumb" check, and never touch curses. If it's good enough for them, it's good enough for llvm too, and we don't need to extra complexity and dependencies. Also, the hacks in this code are ugly (writing our own function signatures for these functions instead of using a proper #include!).

This is exactly what LLVM did prior to r187874 so this would mean basically reverting that change, I'm fine with that.

[joerg]

I completely disagree with this approach. A lot of GNU tools (including GCC) are completely broken. We shouldn't follow them. There are a lot more terminals around than just "dumb", "xterm" and "linux". It is completely non-acceptable to just assume ANSI escape sequences work. If Android doesn't ship a usable terminfo implementation, I consider that an Android bug. Wouldn't be the first portability nightmare with Android.

[jyknight]

In D42055#980865, @joerg wrote:

I completely disagree with this approach. A lot of GNU tools (including GCC) are completely broken. We shouldn't follow them. There are a lot more terminals around than just "dumb", "xterm" and "linux". It is completely non-acceptable to just assume ANSI escape sequences work. If Android doesn't ship a usable terminfo implementation, I consider that an Android bug. Wouldn't be the first portability nightmare with Android.

I completely disagree with your disagreement. Also, this is not an android issue.

Again note that Clang doesn't even _use_ the terminfo database, it just checks it to see if it has an attribute named "colors". Of course this is totally wrong if you are actually using a non-VTxxx-compatible terminal that supports colors. Which I assure you, do exist. Of course, nobody _uses_ any of these other crazy non-VTxxx terminals. (No, I don't care if you have such a paperweight sitting under your desk...) We should delete this code.

Nevertheless, the patch proposed here to add a fallback case is clearly fine, at a minimum. So, fine, let's not let a side-argument about removing useless code hold this up.

[joerg]

That's no excuse for making the situation even worse.

[jyknight]

Ehhhhh? This patch (without my proposed change of entirely removing the HAVE_TERMINFO codepath) certainly does not.

[joerg]

Do you see the comment just following the code? The patch completely violates that basic design principle. It would be perfectly sensible to hard-code a list of dumb terminals and explicitly default to no colors for them. The reverse (hard-coding a list and assuming it is fine for everything else) is not.

[jyknight]

You win by virtue of using up my willingness to argue on this subject.

[phosek]

I've implemented what @joerg suggested, is this acceptable?

[joerg]

Good enough for me.

llvm/trunk/lib/Support/Unix/Process.inc

#ifdef HAVE_TERMINFO
   // Now extract the structure allocated by setupterm and free its memory
   // through a really silly dance.
   struct term *termp = set_curterm(nullptr);
   (void)del_curterm(termp); // Drop any errors here.
 
   // Return true if we found a color capabilities for the current terminal.
   if (HasColors)
     return true;
+#else
+  // When the terminfo database is not available, check if the current terminal
+  // is one of terminals that are known to support ANSI color escape codes.
+  if (const char *TermStr = std::getenv("TERM")) {
+    return StringSwitch<bool>(TermStr)
+      .Case("ansi", true)
+      .Case("cygwin", true)
+      .Case("linux", true)
+      .StartsWith("screen", true)
+      .StartsWith("xterm", true)
+      .StartsWith("vt100", true)
+      .StartsWith("rxvt", true)
+      .EndsWith("color", true)
+      .Default(false);
+  }
 #endif
 
   // Otherwise, be conservative.
   return false;
 }
 
 bool Process::FileDescriptorHasColors(int fd) {
   // A file descriptor has colors if it is displayed and the terminal has