[PATCH] D31263: Add option to control whether llvm-pdbdump outputs in color

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth marked an inline comment as done.
amccarth added inline comments.


================
Comment at: llvm/tools/llvm-pdbdump/LinePrinter.h:27
 public:
-  LinePrinter(int Indent, raw_ostream &Stream);
+  LinePrinter(int Indent, raw_ostream &Stream, bool UseColor = true);
 
----------------
zturner wrote:
> Does this need a default argument if we're always going to specify it anyway?
I moved the UseColor parameter to the middle, in order to keep the "options" together and avoid burying the stream in the middle.


================
Comment at: llvm/tools/llvm-pdbdump/llvm-pdbdump.cpp:130-131
+cl::opt<cl::boolOrDefault>
+    ColorOutput("color-output", cl::desc("Use color to display the results"),
+                cl::cat(OtherOptions), cl::sub(PrettySubcommand));
 cl::list<std::string> ExcludeTypes(
----------------
zturner wrote:
> We should probably come up with some wording that makes it clear that the user does not need to specify this to get color output if they're just running from the terminal.  A way to do this concisely escapes me at the moment, however.
Acknowledged, but I haven't come up with a concise phrasing either.  Happy to change in the future if a better idea comes along.


https://reviews.llvm.org/D31263