[PATCH] D46757: [clang-format] text protos: put entries on separate lines if there is a submessage

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

LG



================
Comment at: lib/Format/TokenAnnotator.cpp:2942
+  // }
+  //
+  // Be careful to exclude the case  [proto.ext] { ... } since the `]` is
----------------
One more high-level piece could be added to this comment, e.g.
```
// We enforce this by breaking before a submessage field that has previous siblings, *and*
// breaking before a field that follows a submessage field.```


================
Comment at: lib/Format/TokenAnnotator.cpp:2943
+  //
+  // Be careful to exclude the case  [proto.ext] { ... } since the `]` is
+  // a TT_SelectorName there, but we do not want to put a line break before `]`
----------------
nit: the `]` is **the** TT_SelectorName


================
Comment at: lib/Format/TokenAnnotator.cpp:2944
+  // Be careful to exclude the case  [proto.ext] { ... } since the `]` is
+  // a TT_SelectorName there, but we do not want to put a line break before `]`
+  // (in this case the [proto.ext] would already be put on a new line as
----------------
This could be slightly clearer I think...

```... but we don't want to break inside the brackets. We ensure elsewhere that extensions are always on their own line.```


================
Comment at: lib/Format/TokenAnnotator.cpp:2953
+    // selector: { ...
+    FormatToken *LParen =
+        Right.Next->is(tok::colon) ? Right.Next->Next : Right.Next;
----------------
nit: this can't be a paren. calling it "brace" is probably ok


================
Comment at: lib/Format/TokenAnnotator.cpp:2983
+    // If this is an entry immediately following a submessage, it will be
+    // preceeded by a closing paren of that submessage, like in:
+    //     left---.  .---right
----------------
nit: preceded


Repository:
  rC Clang

https://reviews.llvm.org/D46757