steveire added inline comments.

================
Comment at: clang-tools-extra/clang-query/tool/ClangQuery.cpp:58
+standard output supports colors.)"),
+             cl::init(false), cl::cat(ClangQueryCategory));
+
----------------
tomrittervg wrote:
> steveire wrote:
> > tomrittervg wrote:
> > > steveire wrote:
> > > > aaron.ballman wrote:
> > > > > steveire wrote:
> > > > > > Doesn't this set the default to false?
> > > > > It does, but it does not disable colors by default (I had to 
> > > > > double-check this myself), which is why I was asking for the 
> > > > > regression tests. That will clearly show the behavior and help ensure 
> > > > > we don't regress it in the future.
> > > > Did you build and run with the patch? 
> > > > 
> > > > Current `clang-query` dumps with color. After this patch, when not 
> > > > using the command line option, there is now no color. with 
> > > > `-use-color`, the color is restored.
> > > > 
> > > > @tomrittervg I know this is based on our discussions, but it's not 
> > > > clear to me what the intent of the patch is. Do you want to be able to 
> > > > disable color?
> > > Let me double check - my current clang-query did not dump with color 
> > > (which is why I made the patch). Maybe this was a mix-up I had.
> > https://reviews.llvm.org/D62056 already made dumps use color.
> Is that what you're running in your CE instance? You're right, I forgot about 
> that; and that does make it output color on the terminal. But not CE.
> 
> I _think_ what's happening is that CE isn't acting like a tty, so clang won't 
> output it in color even if I specify `--extra-arg="-fdiagnostics-color"` 
> (which the analogous `--extra-arg="-fno-diagnostics-color"` _will_ turn off 
> color on the command-line.)
Yes, you're probably right that it's tty related. 

It looks to me like the only problem is that you specified the default as 
`false` instead of `true`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94624/new/

https://reviews.llvm.org/D94624

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to