[PATCH] D53567: [Support] Enable color diagnostics for mintty

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a subscriber: thakis.
rnk added a comment.

In https://reviews.llvm.org/D53567#1273729, @kristina wrote:

> Another major problem I see is the difficulty in adding a test for this specific patch, due to its complicated nature this would require a fairly complex unit test that mocks that specific aspect of mintty, which has not been provided either. Given that this will be linked in by all Windows tools, and the complexity of the patch, I think at the very least this should provide said test.
>
> Let me add that the maintenance burden added by this is fairly significant as it depends on compatibility with a project unrelated to LLVM, very specific naming conventions and the use of native APIs to use the named pipe and should it change, the code will stop working as well as potentially cause regressions for Cygwin/MSYS based toolchains (as they use mintty as well)? So at the very least, this very least this should require a test.
>
> Personally I don't think this should make it into the support library at all, I can see a lot of things going wrong here requiring reverting this. I'm adding @rnk as a reviewer as well, as he is more qualified in this area than me, and I think his opinion would be of high value here (regarding the cost-benefit aspect of this patch, even if other comments are addressed).


Thanks!

So, funny story, I wanted to do this for ninja (https://github.com/ninja-build/ninja/pull/659) and @thakis didn't like the complexity it added.

If we do decide to do this, I don't think it's the kind of thing we can reasonably test in LLVM, for the reasons @kristina mentioned. If people agree it's a valuable feature that's worth the complexity, I think we can commit it, and rely on mintty users to tell us when the colors don't end up printing.



================
Comment at: lib/Support/Windows/Process.inc:231
 
+#if defined(__MINGW32__)
+// Hack to detect mintty, ported from vim
----------------
This is plain old win32 API code. Why make it mingw-specific? Then clang-cl will use colors in mintty. I previously used it as my primary development environment, even for targeting MSVC. Today I use cmd, since it's improved greatly, but it's still pretty janky.


Repository:
  rL LLVM

https://reviews.llvm.org/D53567