On Mon, Jan 14, 2013 at 11:57 AM, Richard Trieu <[email protected]> wrote: > On Sat, Jan 12, 2013 at 5:01 AM, Matthieu Monrocq > <[email protected]> wrote: >> >> >> >> On Sat, Jan 12, 2013 at 1:31 AM, Richard Smith <[email protected]> >> wrote: >>> >>> >>> >>> ================ >>> Comment at: lib/AST/ASTDumper.cpp:36-76 >>> @@ +35,43 @@ >>> + // Decl kind names (VarDecl, FunctionDecl, etc) >>> + static const raw_ostream::Colors declKindNameColor = >>> raw_ostream::GREEN; >>> + static const bool declKindNameBold = true; >>> + // Attr names (CleanupAttr, GuardedByAttr, etc) >>> + static const raw_ostream::Colors attrColor = raw_ostream::BLUE; >>> + static const bool attrBold = true; >>> + // Statement names (DeclStmt, ImplicitCastExpr, etc) >>> + static const raw_ostream::Colors stmtColor = raw_ostream::MAGENTA; >>> + static const bool stmtBold = true; >>> + >>> + // Type names (int, float, etc, plus user defined types) >>> + static const raw_ostream::Colors typeColor = raw_ostream::GREEN; >>> + static const bool typeBold = false; >>> + >>> + // Pointer address >>> + static const raw_ostream::Colors addressColor = raw_ostream::YELLOW; >>> + static const bool addressBold = false; >>> + // Source locations >>> + static const raw_ostream::Colors locationColor = >>> raw_ostream::YELLOW; >>> + static const bool locationBold = true; >>> + >>> + // lvalue/xvalue >>> + static const raw_ostream::Colors valueKindColor = raw_ostream::CYAN; >>> + static const bool valueKindBold = false; >>> + // bitfield/objcproperty/objcsubscript/vectorcomponent >>> + static const raw_ostream::Colors objectKindColor = >>> raw_ostream::CYAN; >>> + static const bool objectKindBold = false; >>> + >>> + // Null statements >>> + static const raw_ostream::Colors nullColor = raw_ostream::BLUE; >>> + static const bool nullBold = false; >>> + >>> + // CastKind from CastExpr's >>> + static const raw_ostream::Colors castColor = raw_ostream::RED; >>> + static const bool castBold = false; >>> + >>> + // Value of the statement >>> + static const raw_ostream::Colors valueColor = raw_ostream::CYAN; >>> + static const bool valueBold = true; >>> + // Decl names >>> + static const raw_ostream::Colors declNameColor = raw_ostream::CYAN; >>> + static const bool declNameBold = true; >>> + >>> ---------------- >>> These names should all have a leading capital letter. >>> >>> Maybe add a struct wrapping a Colors value and a Bold flag? >>> >>> >>> http://llvm-reviews.chandlerc.com/D291 >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> >> I will let the technical details to my better... just wanted to say that >> the output looks awesome with the colors, it's amazing how a little touch-up >> makes spotting similarly kinded elements easily. >> >> If I could just make a remark, it's that I would tune up the *important* >> parts and tune down those who matter less: here the lines/columns in bright >> yellow really are "in your face", are they so important that they should be >> more noticeable than say... variable names ? >> >> Just bikeshedding sure, but since it does not look like it'll be >> configurable (and what herd of options it would require...) it might be >> worth tuning it right. >> >> -- Matthieu > > To be honest, I did expect a bit of bikeshedding since there are several > different things to paint, so bikeshed away. > > Do you prefer no highlighting or just a different color for source > locations? > Or maybe turning off the bold so it has the same color as the pointer > address before it? > Are there some parts that you would like highlighted but aren't? And what > color should they be? > > I also would like getting the colors correct instead of having lots of churn > later.
For what it's worth I don't think changing the colors later would be a bad thing - people will get some experience with them & decide that some things feel awkward/hard to read & it's not like changing the colors will be terribly "churny" changes. But, sure, if we can get reasonably close/good to start with there's no reason not to. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
