Add option to control whether llvm-pdbdump outputs in color

[amccarth]

Summary

Adds -color-output option to llvm-pdbdump pretty commands that lets you specify whether the output should have color. The default depends on whether the output is going to a TTY (per prior discussion in https://reviews.llvm.org/D31246).

This will enable tests that pipe llvm-pdbdump output to FileCheck to work across platforms without regard to the differences in ANSI codes.

[zturner]

llvm/tools/llvm-pdbdump/LinePrinter.h

27 ↗

(On Diff #92719)

Does this need a default argument if we're always going to specify it anyway?

llvm/tools/llvm-pdbdump/llvm-pdbdump.cpp

130–131 ↗

(On Diff #92719)

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.

[amccarth]

llvm/tools/llvm-pdbdump/LinePrinter.h

27 ↗

(On Diff #92719)

I moved the UseColor parameter to the middle, in order to keep the "options" together and avoid burying the stream in the middle.

llvm/tools/llvm-pdbdump/llvm-pdbdump.cpp

130–131 ↗

(On Diff #92719)

Acknowledged, but I haven't come up with a concise phrasing either. Happy to change in the future if a better idea comes along.

[zturner]

I think we need something before you commit, because as it is people will be specifying --color-output unnecessarily. Even if you say "default = on" or "default = best-effort"

[amccarth]

In D31263#708121, @zturner wrote:

I think we need something before you commit, because as it is people will be specifying --color-output unnecessarily. Even if you say "default = on" or "default = best-effort"

OK, I came up with "Override use of color (default = isatty)", which I assume should be clear enough for typical users of this tool.

I'll let it bake tonight and submit in the morning, in case Chandler or anyone else has additional ideas.

[chandlerc]

In D31263#708137, @amccarth wrote:

In D31263#708121, @zturner wrote:

I think we need something before you commit, because as it is people will be specifying --color-output unnecessarily. Even if you say "default = on" or "default = best-effort"

OK, I came up with "Override use of color (default = isatty)", which I assume should be clear enough for typical users of this tool.

I'll let it bake tonight and submit in the morning, in case Chandler or anyone else has additional ideas.

I have no better suggestion. Thanks for reworking with this approach though! =D

llvm/trunk/tools/llvm-pdbdump/LinePrinter.h

 
 namespace llvm {
 namespace pdb {
 
 class LinePrinter {
   friend class WithColor;
 
 public:
-  LinePrinter(int Indent, raw_ostream &Stream);
+  LinePrinter(int Indent, bool UseColor, raw_ostream &Stream);
 
   void Indent();
   void Unindent();
   void NewLine();
 
+  bool hasColor() const { return UseColor; }
   raw_ostream &getStream() { return OS; }
   int getIndentLevel() const { return CurrentIndent; }
 
   bool IsTypeExcluded(llvm::StringRef TypeName);
   bool IsSymbolExcluded(llvm::StringRef SymbolName);
   bool IsCompilandExcluded(llvm::StringRef CompilandName);
 
 private:
   template <typename Iter>
   void SetFilters(std::list<Regex> &List, Iter Begin, Iter End) {
     List.clear();
     for (; Begin != End; ++Begin)
       List.emplace_back(StringRef(*Begin));
   }
 
   raw_ostream &OS;
   int IndentSpaces;
   int CurrentIndent;
+  bool UseColor;
 
   std::list<Regex> ExcludeCompilandFilters;
   std::list<Regex> ExcludeTypeFilters;
   std::list<Regex> ExcludeSymbolFilters;
 
   std::list<Regex> IncludeCompilandFilters;
   std::list<Regex> IncludeTypeFilters;
   std::list<Regex> IncludeSymbolFilters;

llvm/trunk/tools/llvm-pdbdump/LinePrinter.cpp

if (any_of(ExcludeFilters, match_pred))
     return true;
 
   return false;
 }
 }
 
 using namespace llvm;
 
-LinePrinter::LinePrinter(int Indent, llvm::raw_ostream &Stream)
-    : OS(Stream), IndentSpaces(Indent), CurrentIndent(0) {
+LinePrinter::LinePrinter(int Indent, bool UseColor, llvm::raw_ostream &Stream)
+    : OS(Stream), IndentSpaces(Indent), CurrentIndent(0), UseColor(UseColor) {
   SetFilters(ExcludeTypeFilters, opts::pretty::ExcludeTypes.begin(),
              opts::pretty::ExcludeTypes.end());
   SetFilters(ExcludeSymbolFilters, opts::pretty::ExcludeSymbols.begin(),
              opts::pretty::ExcludeSymbols.end());
   SetFilters(ExcludeCompilandFilters, opts::pretty::ExcludeCompilands.begin(),
              opts::pretty::ExcludeCompilands.end());
 
   SetFilters(IncludeTypeFilters, opts::pretty::IncludeTypes.begin(),

 }
 
 bool LinePrinter::IsCompilandExcluded(llvm::StringRef CompilandName) {
   return IsItemExcluded(CompilandName, IncludeCompilandFilters,
                         ExcludeCompilandFilters);
 }
 
 WithColor::WithColor(LinePrinter &P, PDB_ColorItem C) : OS(P.OS) {
+  if (P.hasColor())
     applyColor(C);
 }
 
 WithColor::~WithColor() { OS.resetColor(); }
 
 void WithColor::applyColor(PDB_ColorItem C) {
   switch (C) {
   case PDB_ColorItem::None:
     OS.resetColor();

llvm/trunk/tools/llvm-pdbdump/llvm-pdbdump.cpp

cl::opt<bool>
     All("all", cl::desc("Implies all other options in 'Symbol Types' category"),
         cl::cat(TypeCategory), cl::sub(PrettySubcommand));
 
 cl::opt<uint64_t> LoadAddress(
     "load-address",
     cl::desc("Assume the module is loaded at the specified address"),
     cl::cat(OtherOptions), cl::sub(PrettySubcommand));
 cl::opt<bool> Native("native", cl::desc("Use native PDB reader instead of DIA"),
                      cl::cat(OtherOptions), cl::sub(PrettySubcommand));
-
+cl::opt<cl::boolOrDefault>
+    ColorOutput("color-output",
+                cl::desc("Override use of color (default = isatty)"),
+                cl::cat(OtherOptions), cl::sub(PrettySubcommand));
 cl::list<std::string> ExcludeTypes(
     "exclude-types", cl::desc("Exclude types by regular expression"),
     cl::ZeroOrMore, cl::cat(FilterCategory), cl::sub(PrettySubcommand));
 cl::list<std::string> ExcludeSymbols(
     "exclude-symbols", cl::desc("Exclude symbols by regular expression"),
     cl::ZeroOrMore, cl::cat(FilterCategory), cl::sub(PrettySubcommand));
 cl::list<std::string> ExcludeCompilands(
     "exclude-compilands", cl::desc("Exclude compilands by regular expression"),
static void dumpPretty(StringRef Path) {
 
   const auto ReaderType =
       opts::pretty::Native ? PDB_ReaderType::Native : PDB_ReaderType::DIA;
   ExitOnErr(loadDataForPDB(ReaderType, Path, Session));
 
   if (opts::pretty::LoadAddress)
     Session->setLoadAddress(opts::pretty::LoadAddress);
 
-  LinePrinter Printer(2, outs());
+  auto &Stream = outs();
+  const bool UseColor = opts::pretty::ColorOutput == cl::BOU_UNSET
+                            ? Stream.has_colors()
+                            : opts::pretty::ColorOutput == cl::BOU_TRUE;
+  LinePrinter Printer(2, UseColor, Stream);
 
   auto GlobalScope(Session->getGlobalScope());
   std::string FileName(GlobalScope->getSymbolsFileName());
 
   WithColor(Printer, PDB_ColorItem::None).get() << "Summary for ";
   WithColor(Printer, PDB_ColorItem::Path).get() << FileName;
   Printer.Indent();
   uint64_t FileSize = 0;