amccarth added a comment.

I'm just responding to the questions @labath raised without really looking at 
the details of the code.

I agree that there's nothing wrong with having the ability to output color 
codes to a file, but it's a surprising default and experience tells me it would 
break a lot of tests that are rightfully looking just at the content rather 
than the styling.  But having the ability to turn on color for file output 
would make it possible to also test styling, an ability we don't currently have.

Plumbing the option for color and other niceties down to a low level sounds 
like a great idea, as long as we get the defaults right.  For Windows (for now) 
and for output redirected to a file, the default should be off.  That said, I 
could see lowering of the color option being a separate effort outside the 
scope of this patch.  For one thing, we'd have to get buy-in from the more 
general LLVM folks.  And several of the llvm utilities currently use the 
`WithColor` approach, so there could be a lot of work updating those (or they 
would continue to use the old approach indefinitely).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D81058



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

Reply via email to