Page MenuHomePhabricator

[SourceMgr][FileCheck] Obey -color by extending WithColor
ClosedPublic

Authored by jdenny on Oct 19 2018, 12:41 AM.

Details

Summary

While this change specifically targets FileCheck, it affects any tool
using the same SourceMgr facilities.

Previously, -color was documented in FileCheck's -help output, but
-color had no effect. Now, -color obeys its documentation: it forces
colors to be used in FileCheck diagnostics even when stderr is not a
terminal.

-color is especially helpful when combined with FileCheck's -v, which
can produce a long series of diagnostics that you might wish to pipe
to a pager, such as less -R. The WithColor extensions here will also
help to clean up color usage in FileCheck's annotated dump of input,
which is proposed in D52999.

Diff Detail

Repository
rL LLVM

Event Timeline

jdenny created this revision.Oct 19 2018, 12:41 AM
This revision is now accepted and ready to land.Oct 19 2018, 9:06 AM

LGTM.

Thanks. Will commit in a few days when I'm available to watch the buildbots.

This revision was automatically updated to reflect the committed changes.
jdenny updated this revision to Diff 170749.Oct 23 2018, 2:12 PM
jdenny added reviewers: teemperor, xbolva00, nrieck.

The patch was reverted because the test it introduces fails on some Windows bots where color isn't supported as expected.

I've added a small change: at the start of FileCheck's main, call llvm::sys::Process::UseANSIEscapeCodes(true) unconditionally. This is a no-op for Linux, but it fixes the test for my Windows 10 install, and it seems to make FileCheck colors behave ok in my brief cmd.exe experiments.

However, I'm not a Windows developer. I see that each of clang and lldb handles Windows colors in its own unique and more complex way, which I haven't fully grasped.

I've added some reviewers who, according to git blame, have worked with Windows colors for LLVM before. Before I commit again, could someone please comment as to whether this change looks ok for FileCheck under Windows?

Thanks.

jdenny reopened this revision.Oct 23 2018, 2:12 PM
This revision is now accepted and ready to land.Oct 23 2018, 2:12 PM
jdenny requested review of this revision.Oct 23 2018, 2:13 PM
zturner accepted this revision.Oct 23 2018, 2:41 PM

Seems ok to me, my only concern is that something could fail on Windows 8 or Windows 7, but I don't have a system to test that on.

This revision is now accepted and ready to land.Oct 23 2018, 2:41 PM

Seems ok to me, my only concern is that something could fail on Windows 8 or Windows 7, but I don't have a system to test that on.

Thanks. I'll try to push tomorrow when I can watch the bots more carefully. Hopefully any problems will be caught there.

This revision was automatically updated to reflect the committed changes.