[cfe-commits] [PATCH] Addition of color to -ast-dump

[Saleem Abdulrasool]

  I have to say, this looks pretty nice!


================
Comment at: include/clang/AST/DeclBase.h:853
@@ -852,2 +852,3 @@
   // Debuggers don't usually respect default arguments.
   LLVM_ATTRIBUTE_USED void dump() const;
+  // Same as dump(), but forces color printing.
----------------
Why not make colour a default valued parameter for dump?  This would avoid duplication of the method as well as reduce the interface.

================
Comment at: include/clang/AST/Stmt.h:368
@@ +367,3 @@
+  /// dumpColor - same as dump(), but forces color highlighting.
+  LLVM_ATTRIBUTE_USED void dumpColor() const;
+
----------------
Same here.

================
Comment at: lib/AST/ASTDumper.cpp:39
@@ +38,3 @@
+  // Decl kind names (VarDecl, FunctionDecl, etc)
+  static const TerminalColor DeclKindNameColor = { raw_ostream::GREEN, true };
+  // Attr names (CleanupAttr, GuardedByAttr, etc)
----------------
I think it might be nicer to use constant expressions for the various colours rather than inline defining the color, maybe something like the following:

    struct TerminalColor {
        raw_ostream::Colors Color;
        bool Bold;

        static const TerminalColor BoldGreen;
    };
    const TerminalColor::BoldGreen = { raw_ostream::Green, true };



http://llvm-reviews.chandlerc.com/D291