[PATCH] D44203: [clang-format] Improve Incomplete detection for (text) protos

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.


================
Comment at: lib/Format/TokenAnnotator.cpp:773
     case tok::r_brace:
-      // Lines can start with '}'.
-      if (Tok->Previous)
+      // Lines can start with '}' except in text protos.
+      if (Tok->Previous || Style.Language == FormatStyle::LK_TextProto)
----------------
sammccall wrote:
> krasimir wrote:
> > sammccall wrote:
> > > In what sense of "line" is this true?
> > This is the UnwrappedLine concept of clang-format: roughly a maximal sequence of tokens that we could have put on the same line if there was no column limit.
> I still don't understand why it's true for say C or JS then.
> (kinda relevant here because I would like to understand why proto is an exception)
In C++ stuff like global functions or namespaces result in multiple unwrapped lines, like:
```
line 1: namespace a {
line 2: int x = 3;
line 3: }
```

In text protos, the top-level essentially can only contain key-value pairs, where the value can be a submessage, which is like an init list, which is put on a single unwrapped line, like:
```
line 1: key: { item: data }
```

I don't know too much more about why this is the case.


================
Comment at: unittests/Format/FormatTestProto.cpp:21
 class FormatTestProto : public ::testing::Test {
+  enum StatusCheck { SC_ExpectComplete, SC_ExpectIncomplete, SC_DoNotCheck };
+
----------------
sammccall wrote:
> sammccall wrote:
> > DoNotCheck is never used, consider removing it
> (nit: is the SC_ prefix really needed in tests?)
The `SC_` prefix is consistent with other test files here, like:
https://github.com/llvm-mirror/clang/blob/456e35cbb384bd7fb97cf995163fd9d17e319b77/unittests/Format/FormatTest.cpp#L34


Repository:
  rC Clang

https://reviews.llvm.org/D44203