On 24 January 2013 03:24, Matthieu Monrocq <[email protected]> wrote: > > > On Wed, Jan 23, 2013 at 12:12 PM, Philip Craig <[email protected]> > wrote: >> >> >> >> ================ >> 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. >> ---------------- >> Saleem Abdulrasool wrote: >> > Why not make colour a default valued parameter for dump? This would >> > avoid duplication of the method as well as reduce the interface. >> As the comment states, debuggers don't handle default arguments. >> >> ================ >> Comment at: lib/AST/ASTDumper.cpp:113 >> @@ -58,1 +112,3 @@ >> + LastLocFilename(""), LastLocLine(~0U), >> + ShowColors(SM && SM->getDiagnostics().getShowColors()) { } >> >> ---------------- >> I used OS.has_colors() in lib/Frontend/ASTConsumers.cpp. We probably >> should be consistent between these. I don't know which is better. >> >> >> http://llvm-reviews.chandlerc.com/D291 >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > Is OS.has_colors() correctly set in case of pipes ? If not then it makes > sense to use a user available flag.
has_colors() is basically a isatty() call. Using a user available flag is better, but does it make sense to use the diagnostics flag? Strictly speaking this isn't part of diagnostics. I don't feel strongly about it though. Also, if we do use the diagnostics flag, we could still fallback to has_colors() when there is no SourceManager. Would this remove the need for Stmt::dumpColors()? As it is, I don't see the point of Decl::dumpColors(), because we'll always have a SourceManager for Decl, and color dumping will default to on anyway. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
