mzeren-vmw marked 2 inline comments as done. mzeren-vmw added inline comments.
================ Comment at: lib/Format/TokenAnnotator.cpp:1756 + if (Alignment == CA_Preprocessor) + (*I)->LevelOffset = 1; } else { ---------------- krasimir wrote: > This feels a bit awkward: we're adding code that implicitly assumes the exact > style the preprocessor directives and comments around them are handled. Maybe > if this could become part of the level itself, it would feel less awkward. I agree that the "long distance coupling" is awkward. Perhaps the new enumerator names make this a bit more palatable? ================ Comment at: lib/Format/TokenAnnotator.h:41 AnnotatedLine(const UnwrappedLine &Line) - : First(Line.Tokens.front().Tok), Level(Line.Level), + : First(Line.Tokens.front().Tok), Level(Line.Level), LevelOffset(0), MatchingOpeningBlockLineIndex(Line.MatchingOpeningBlockLineIndex), ---------------- krasimir wrote: > Is there a way to not introduce `LevelOffset`, but have it part of `Level`? `Level` is an abstract indentation level, while `LevelOffset` is "columns". They have different units. Maybe it would be feasible to change the units of "Level" to columns in order to merge these two variables, but doing so would throw away information. It also seems like a much larger change. We could create a composite type `class AnnotatedLevel { private: unsigned Level, unsigned Offset public: <strongly typed operations> }` but that seems over-engineered. Any other ideas? Repository: rC Clang https://reviews.llvm.org/D42036 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits