[PATCH] D31246: Send ANSI color codes only to TTYs

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth added a comment.

In https://reviews.llvm.org/D31246#707786, @chandlerc wrote:

> In https://reviews.llvm.org/D31246#707747, @amccarth wrote:
>
> > In https://reviews.llvm.org/D31246#707746, @chandlerc wrote:
> >
> > > In https://reviews.llvm.org/D31246#707661, @zturner wrote:
> > >
> > > > In https://reviews.llvm.org/D31246#707656, @chandlerc wrote:
> > > >
> > > > > But we use this to send color codes to temporary files that capture output for build systems and such when -fcolor-diagnostics is explicitly provided? (As an example...)
> > > >
> > > >
> > > > That's strange, because everyone I asked thought that the `raw_ostream` stuff already worked this way -- i.e. doesn't send color if the destination doesn't support it.  Is there a currently accepted practice for writing FileCheck test against tools that output color?  Should every such tool have an explicit `-no-color` option?
> > >
> > >
> > > Possibly.... What's the failure?
> >
> >
> > I added some functionality to llvm-pdbdump and included a test for it that pipes the output of llvm-pdbdump through FileCheck.  Worked on Windows, which doesn't use ANSI codes to achieve colors, but failed on Linux because the ANSI codes weren't expected.
> >
> > I'm happy to add a -no-color option to llvm-pdbdump that I can use to make the test output consistent on all platforms if you think that's a better way to go.
>
>
> I would look at how things like FileCheck tests for Clang's AST dump avoid tripping over ANSI codes.
>
> My memory was that Clang itself checked a flag controlling color and whether the output was a TTY, and then set the flags to control color, and I would expect something similar here.
>
> Essentially, that there would be a '-color-output' flag for llvm-pdbdump that defaults by detecting TTY but can be explicitly set to "on" and explicitly set to "off".
>
> Without that kind of behavior, how would you write a test of the ANSI escape codes themselves with FileCheck, which you might want to do?


Well, any test of ANSI escape codes themselves would necessarily be platform specific, since not all the platforms use ANSI codes to control color and other rendering attributes.

Anyway, a -color-output flag with the default behavior that you suggest seems like the way to go.  I'll abandon this and work on that.  Thanks.


https://reviews.llvm.org/D31246