[PATCH] D35683: [clang-format] Put '/**' and '*/' on own lines in multiline jsdocs

[Martin Probst via Phabricator via cfe-commits]

mprobst added inline comments.


================
Comment at: lib/Format/BreakableToken.cpp:435
+  // Detect a multiline jsdoc comment and set DelimitersOnNewline in that case.
+  if (Style.Language == FormatStyle::LK_JavaScript) {
+    if ((Lines[0] == "*" || Lines[0].startswith("* ")) && Lines.size() > 1) {
----------------
I think I'd also enable this for at least Java. But tentatively I wonder - wouldn't just about every language do it like this for `/**` comments?


================
Comment at: lib/Format/BreakableToken.cpp:436
+  if (Style.Language == FormatStyle::LK_JavaScript) {
+    if ((Lines[0] == "*" || Lines[0].startswith("* ")) && Lines.size() > 1) {
+      // This is a multiline jsdoc comment.
----------------
Wouldn't we also want to do this for `/**blah`, i.e. no whitespace after `*`?


================
Comment at: lib/Format/BreakableToken.cpp:436
+  if (Style.Language == FormatStyle::LK_JavaScript) {
+    if ((Lines[0] == "*" || Lines[0].startswith("* ")) && Lines.size() > 1) {
+      // This is a multiline jsdoc comment.
----------------
mprobst wrote:
> Wouldn't we also want to do this for `/**blah`, i.e. no whitespace after `*`?
Would we also want to do this for simple block comments, e.g. `/*blah*/`?  That's a bit more tricky as they can be used inline, not sure about the corner cases there.


================
Comment at: lib/Format/BreakableToken.cpp:606
+    if (DelimitersOnNewline) {
+        size_t BreakLength = Lines[0].substr(1).find_first_not_of(Blanks);
+        if (BreakLength != StringRef::npos) {
----------------
isn't this BreakPosition rather than length?


================
Comment at: unittests/Format/FormatTestJS.cpp:136
+                   " * line long long long */",
+                   getGoogleJSStyleWithColumns(20)));
+}
----------------
do you need a test where the entire comment block is indented?


https://reviews.llvm.org/D35683