[clang-format] PR48539 ReflowComments breaks Qt translation comments

[MyDeveloperDay]

Summary

https://bugs.llvm.org/show_bug.cgi?id=48539

Add support for Qt Translator Comments to reflow

When reflown and a part of the comments are added on a new line, it should repeat these extra characters as part of the comment token.

[curdeius]

LGTM! Thanks for fixing this.

[HazardyKnusperkeks]

I don't know if this really fixes the problem.

From the Docs:

An alternative way to attach meta-data is to use the following syntax:

//~ <field name> <field contents>

With the example

//~ loc-layout_id foo_dialog
//~ loc-blank False
//~ magic-stuff This might mean something magic.

So I would say breaking the ~ comment is a no go, but I would have to test it. (Which I can do next week.)
//: should be fine, //= would have the same problem, with the exception that it seems not to be used normally with long comments.

[MyDeveloperDay]

Maybe @JVApen can help as they logged https://bugs.llvm.org/show_bug.cgi?id=48539

I'm not a Qt developer so I can't really comment beyond fixing what was in the bug rpeort

[JVApen]

In D93490#2463068, @MyDeveloperDay wrote:

Maybe @JVApen can help as they logged https://bugs.llvm.org/show_bug.cgi?id=48539

I'm not a Qt developer so I can't really comment beyond fixing what was in the bug rpeort

We actually only use //: in our code, I found the others when looking at the docs. Maybe it's better to skip //~ and add a to-do with the open question if we even can correctly reflow this?

[HazardyKnusperkeks]

So, I've tested a bit:

//= Die ID
//~ foo bar
//~ EinWort Ein langer Text der fortgesetzt wird
//: Dies ist ein langer Kommentar
//: der umgebrochen wird
tr("Foo");

Results in

<message id="Die ID">
    <location filename="wid.cpp" line="9"/>
    <source>Foo</source>
    <extracomment>Dies ist ein langer Kommentar der umgebrochen wird</extracomment>
    <translation type="unfinished"></translation>
    <extra-EinWort>Ein langer Text der fortgesetzt wird</extra-EinWort>
    <extra-foo>bar</extra-foo>
</message>

And breaking //= or //~ as followed

//= Die
//= ID
//~ foo bar
//~ EinWort Ein langer Text der
//~ fortgesetzt wird
//: Dies ist ein langer Kommentar
//: der umgebrochen wird
tr("Foo");

changes the result

<message id="ID">
    <location filename="wid.cpp" line="11"/>
    <source>Foo</source>
    <extracomment>Dies ist ein langer Kommentar der umgebrochen wird</extracomment>
    <translation type="unfinished"></translation>
    <extra-EinWort>Ein langer Text der</extra-EinWort>
    <extra-foo>bar</extra-foo>
    <extra-fortgesetzt>wird</extra-fortgesetzt>
</message>

So my conclusion is //: can and should be wrapped like in this patch, but //= and //~ should not be touched anyhow, this can be accomplished with the CommentPragmas Setting.

[MyDeveloperDay]

remove = and ~ cases

[JVApen]

In D93490#2467733, @MyDeveloperDay wrote:

remove = and ~ cases

Wouldn't it make sense to explicitly document that //= and //~ are not put in here as reflowing them results in wrong results?

[HazardyKnusperkeks]

This looks good to me.

In D93490#2468351, @JVApen wrote:

In D93490#2467733, @MyDeveloperDay wrote:

remove = and ~ cases

Wouldn't it make sense to explicitly document that //= and //~ are not put in here as reflowing them results in wrong results?

I think this would fit in a separate change which would also modify the documentation of CommentPragmas, so that who ever needs it could set that accordingly.

I'm currently trying to fix a misformating with AlignConsecutiveDeclarations with Qts emit, maybe there should be at Qt-Mode or something similar? As far as I can see there are some code paths to accommodate Qt.

clang/lib/Format/BreakableToken.cpp

case '\r':
     return true;
   default:
     return false;
   }
 }
 
 static StringRef getLineCommentIndentPrefix(StringRef Comment,
                                             const FormatStyle &Style) {
-  static const char *const KnownCStylePrefixes[] = {"///<", "//!<", "///", "//",
-                                                    "//!"};
+  static const char *const KnownCStylePrefixes[] = {"///<", "//!<", "///",
+                                                    "//",   "//!",  "//:"};
   static const char *const KnownTextProtoPrefixes[] = {"//", "#", "##", "###",
                                                        "####"};
   ArrayRef<const char *> KnownPrefixes(KnownCStylePrefixes);
   if (Style.Language == FormatStyle::LK_TextProto)
     KnownPrefixes = KnownTextProtoPrefixes;
 
   StringRef LongestPrefix;
   for (StringRef KnownPrefix : KnownPrefixes) {

clang/unittests/Format/FormatTestComments.cpp

EXPECT_EQ("{\n"
             "  // 6\n"
             "}",
             format("{\n"
                    "  //\n"
                    "  //\\\n"
                    "  // long 1 2 3 4 5 6\n"
                    "}",
                    getLLVMStyleWithColumns(20)));
+
+  EXPECT_EQ("//: A comment that\n"
+            "//: doesn't fit on\n"
+            "//: one line",
+            format("//: A comment that doesn't fit on one line",
+                   getLLVMStyleWithColumns(20)));
 }
 
 TEST_F(FormatTestComments, PreservesHangingIndentInCxxComments) {
   EXPECT_EQ("//     A comment\n"
             "//     that doesn't\n"
             "//     fit on one\n"
             "//     line",
             format("//     A comment that doesn't fit on one line",