[PATCH] D29300: [clang-format] Refactor WhitespaceManager and friends

[Daniel Jasper via Phabricator via cfe-commits]

djasper marked an inline comment as done.
djasper added inline comments.


================
Comment at: lib/Format/UnwrappedLineFormatter.cpp:904-907
+void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine &Line,
                                               const AnnotatedLine *PreviousLine,
-                                              unsigned IndentLevel,
-                                              unsigned Indent,
-                                              bool InPPDirective) {
+                                              unsigned Indent) {
+  FormatToken& RootToken = *Line.First;
----------------
klimek wrote:
> I'm not sure I understand the change in the function signature. Given that we really only need InPPDirective and FirstToken, it seems unnecessary to hand in the whole line? (in the spirit of minimal interfaces)
I think this interface makes more sense:

- I don't think it's significantly "larger" as we already have access to the whole previous line.
- Having a function that's called formatFirstToken, but can actually be called with an arbitrary token seems weird.



================
Comment at: lib/Format/WhitespaceManager.h:109-110
+    // this change inserts whitespace.
+    // FIXME: Currently this is not set correctly for breaks inside comments, as
+    // the \c BreakableToken is still doing its own alignment.
+    const FormatToken *Tok;
----------------
klimek wrote:
> What's the proposed fix?
Removed InToken (added that at first, but found the other one later).

I don't have a proposed fix. I just moved this comment from the original line 122.


https://reviews.llvm.org/D29300