[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-30 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 456662.
yusuke-kadowaki marked 9 inline comments as done.
yusuke-kadowaki added a comment.

- Fix the newline condition
- Update tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,106 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+   
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+   
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+   
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+  
+  verifyFormat("#include \"a.h\"  // Align these\n"
+   "\n"
+   "#include \"aa.h\" // two comments\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\" // But do not align this because there are two lines without comments above\n",
+   Style);
+  
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+  
+  Style.ColumnLimit = 80;
+  EXPECT_EQ("int a; // line about a\n"
+"\n"
+"// line about b\n"
+"long b;",
+format("int a; // line about a\n"
+   "\n"
+   "   // line about b\n"
+   "   long b;",
+   Style));
+  
+  Style.ColumnLimit = 80;
+  EXPECT_EQ("int a; // line about a\n"
+"\n"
+"// line 1 about b\n"
+"// line 2 about b\n"
+"long b;",
+format("int a; // line about a\n"
+   "\n"
+   "   // line 1 about b\n"
+   "   // line 2 about b\n"
+   "   long b;",
+   Style));   
+}
+
 TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
   EXPECT_EQ("/*\n"
 " */",
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20041,7 +20041,6 @@
 TEST_F(FormatTest, ParsesConfigurationBools) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
-  CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParameter

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-30 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:785
+  - Consecutive
+  - AcrossEmptyLines
+  - AcrossComments

MyDeveloperDay wrote:
> may be AcrossEmptyLines should be a number (to mean the number of empty lines)
We need to introduce a new struct to do that since AlignConsecutiveStyle is 
shared with some options and not possible to be changed. Plus I think more than 
two empty lines are formatted to one empty line anyway.



Comment at: clang/docs/ClangFormatStyleOptions.rst:794
+
+AlignConsecutiveMacros: AcrossEmptyLines
+

MyDeveloperDay wrote:
> Should this say `AlignedConsecutuveCommets`
No. This part is a documentation about AlignConsecutiveStyle type, which is 
appended automatically after all the options that take AlignConsecutiveStyle 
type.



Comment at: clang/docs/ClangFormatStyleOptions.rst:797
+AlignConsecutiveMacros:
+  Enabled: true
+  AcrossEmptyLines: true

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > MyDeveloperDay wrote:
> > > why do we need Enabled?
> > > 
> > > isn't it just
> > > 
> > > ```
> > > false:
> > > AcrossEmptyLines: false
> > > AcrossComments: false
> > > 
> > > true:
> > > AcrossEmptyLines: true
> > > AcrossComments: true
> > > ```
> > The struct is a bit older. And we need `Enabled`, one might wish to align 
> > only really consecutive comments, as the option right now does. (Same for 
> > macros, assignments, etc..)
> I'm still not sure I understand, plus are those use cases you describe 
> tested, I couldn't see that. 
> 
> If feels like what you are saying is that AlignConsecutiveStyle is used 
> elsewhere and it has options that don't pertain to aligning comments? 
> 
> I couldn't tell if feel like this documentation is describing something other 
> than AligningTrailingComments, if I'm confused users could be too? 
As for the Enabled option,
Enabled: true
AcrossEmptyLines: false

is supposed to work in the same way as the old style AlignTrailingComments. So 
the tests of AlignTrailingComments are the test cases.

For the documentation, I also thought it was a bit confusing when I first saw 
it because the description of the option and the style is kind of connected. 
But as I also mentioned above, this part is automatically append and it affects 
all the other options that have AlignConsecutiveStyle if we change. So I think 
it should be another patch even if we are to modify it.



Comment at: clang/lib/Format/Format.cpp:649
 IO.mapOptional("AlignOperands", Style.AlignOperands);
-IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
 IO.mapOptional("AllowAllArgumentsOnNextLine",

HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > HazardyKnusperkeks wrote:
> > > MyDeveloperDay wrote:
> > > > you can't remove an option, otherwise you'll break everyones 
> > > > .clang-format
> > > That's not correct. We have done it:
> > > D108752 -> D108882 -> D127390
> > > 
> > > You can remove (and in this case should), but you still need to parse it 
> > > and act accordingly. Which is done as far as I can see.
> > sorry thats what I meant, you can remove it, but you have to make it turn 
> > on the correct new style that keeps exactly the old user case, and you 
> > can't remove it from the configuration parsing otherwise anyone who has it 
> > in their .clang-format is immediately broken with an unknown option.
> > 
> > to be honest this is an annoyance for introducing new features, which at 
> > some point I'd like to drop, you can't have a new option which is not read
> > 
> > For me, when we introduced new languages, or new features like InsertBraces 
> > etc.. I want to put it in my .clang-format even though other people they 
> > aren't using a version that uses it. (becuase it won't impact them), i.e. 
> > it won't add braces or correct QualifierOrder that doesn't bother me
> > 
> Do you disagree that it actually is parsed?
> 
> But that compatibility parsing has nothing to do with ignoring unknown (new) 
> options.
I confirmed the old style AlignTrailingComments works and it's also tested with 
CHECK_PARSE if I understand correctly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-31 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 456945.
yusuke-kadowaki marked 4 inline comments as done.
yusuke-kadowaki added a comment.

- Remove trailing whitespace
- Update document


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,106 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // Align these\n"
+   "\n"
+   "#include \"aa.h\" // two comments\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\" // But do not align this because there are two lines without comments above\n",
+   Style);
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 80;
+  EXPECT_EQ("int a; // line about a\n"
+"\n"
+"// line about b\n"
+"long b;",
+format("int a; // line about a\n"
+   "\n"
+   "   // line about b\n"
+   "   long b;",
+   Style));
+
+  Style.ColumnLimit = 80;
+  EXPECT_EQ("int a; // line about a\n"
+"\n"
+"// line 1 about b\n"
+"// line 2 about b\n"
+"long b;",
+format("int a; // line about a\n"
+   "\n"
+   "   // line 1 about b\n"
+   "   // line 2 about b\n"
+   "   long b;",
+   Style));
+}
+
 TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
   EXPECT_EQ("/*\n"
 " */",
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20041,7 +20041,6 @@
 TEST_F(FormatTest, ParsesConfigurationBools) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
-  CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
   CHECK_PARSE_BOOL(AllowShortCaseLab

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-31 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:785
+  - Consecutive
+  - AcrossEmptyLines
+  - AcrossComments

HazardyKnusperkeks wrote:
> yusuke-kadowaki wrote:
> > MyDeveloperDay wrote:
> > > may be AcrossEmptyLines should be a number (to mean the number of empty 
> > > lines)
> > We need to introduce a new struct to do that since AlignConsecutiveStyle is 
> > shared with some options and not possible to be changed. Plus I think more 
> > than two empty lines are formatted to one empty line anyway.
> There `MaxEmptyLinesToKeep` which would allow to set it higher.
> 
> If we want to change the `bool` to an `unsigned`, then that should be a 
> different change.
Oh I didn't know `MaxEmptyLinesToKeep`. Thank you for sharing.
I can try to change it that way when we done with this patch. Test cases would 
be complicated tho.



Comment at: clang/unittests/Format/FormatTestComments.cpp:2863
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"

HazardyKnusperkeks wrote:
> Interesting would be a comment which is split, do we continue to align, or 
> not?
Could you give me a specific example?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-01 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 457294.
yusuke-kadowaki added a comment.

- Update tests
- Fix document


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,167 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // Align these\n"
+   "\n"
+   "#include \"aa.h\" // two comments\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\" // But do not align this because there are two lines without comments above\n",
+   Style);
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 35;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234; // This is a very long\n"
+"  // comment which is\n"
+"  // wrapped arround.\n"
+"\n"
+"int x =\n"
+"2; // Is this still aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 40;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-01 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki marked an inline comment as done.
yusuke-kadowaki added inline comments.



Comment at: clang/include/clang/Format/Format.h:141
 
-  /// Alignment options.
-  ///
-  /// They can also be read as a whole for compatibility. The choices are:
+  /// Alignment styles of ``AlignConsecutiveStyle`` are:
   /// - None

HazardyKnusperkeks wrote:
> That I wouldn't change.
reverted



Comment at: clang/include/clang/Format/Format.h:154
   /// \code
   ///   AlignConsecutiveMacros: AcrossEmptyLines
   ///

HazardyKnusperkeks wrote:
> The change/addition has to be here, since here it directly states 
> `AlignConsecutiveMacros`.
What are you meaning to say here? I thought saying `AlignConsecutiveStyle` is 
used for multiple options is what we are trying to do.

Should we say something like, 
> Here AlignConsecutiveMacros is used as an example, but in practice 
> AlignConsecutiveStyle is also used with other options.
in this place?



Comment at: clang/unittests/Format/FormatTestComments.cpp:2863
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"

HazardyKnusperkeks wrote:
> yusuke-kadowaki wrote:
> > HazardyKnusperkeks wrote:
> > > Interesting would be a comment which is split, do we continue to align, 
> > > or not?
> > Could you give me a specific example?
> Something like
> ```
> int foo = 2323234; // Comment
> int bar = 52323;   // This is a very long comment, ...
>// which is wrapped around.
> 
> int x = 2; // Is this still aligned?
> ```
> You may need to make the comment longer or reduce the column limit, as often 
> used for testing the wrapping behavior.
Added that case with some column limits. I think it's working as expected. What 
do you think?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-04 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 457864.
yusuke-kadowaki marked 8 inline comments as done.
yusuke-kadowaki added a comment.

- Revert doc
- Revert rst as well
- Apply format
- Update implementation to deal with the setting of MaxEmptyLinesToKeep
- Add test for the combination with MaxEmptyLinesToKeep


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,200 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing the combination with MaxEmptyLinesToKeep
+  Style.MaxEmptyLinesToKeep = 0;
+  EXPECT_EQ("int a;   // comment\n"
+"int ab;  // comment\n"
+"int abc; // comment\n",
+format("int a; // comment\n"
+   "int ab; // comment\n"
+   "\n"
+   "int abc; // comment\n",
+   Style));
+
+  Style.MaxEmptyLinesToKeep = 1;
+  EXPECT_EQ("int a;   // comment\n"
+"int ab;  // comment\n"
+"\n"
+"int abc; // comment\n",
+format("int a; // comment\n"
+   "int ab; // comment\n"
+   "\n"
+   "\n"
+   "int abc; // comment\n",
+   Style));
+
+  Style.MaxEmptyLinesToKeep = 2;
+  EXPECT_EQ("int a;   // comment\n"
+"int ab;  // comment\n"
+"\n"
+"\n"
+"int abc; // comment\n",
+format("int a; // comment\n"
+   "int ab; // comment\n"
+   "\n"
+   "\n"
+   "\n"
+   "int abc; // comment\n",
+   Style));
+
+  // Reset the setting
+  Style.MaxEmptyLinesToKeep = 1;
+  // End of testing the combination with MaxEmptyLinesToKeep
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-04 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3698
 
-  QualifierOrder: ['inline', 'static', 'type', 'const']
+  QualifierOrder: ['inline', 'static' , 'type', 'const']
 

HazardyKnusperkeks wrote:
> Anyone knows where this comes from?
> You are using the script to generate the rst, right?
Yes, I'm using `dump_format_style.py`.



Comment at: clang/include/clang/Format/Format.h:154
   /// \code
   ///   AlignConsecutiveMacros: AcrossEmptyLines
   ///

HazardyKnusperkeks wrote:
> yusuke-kadowaki wrote:
> > HazardyKnusperkeks wrote:
> > > The change/addition has to be here, since here it directly states 
> > > `AlignConsecutiveMacros`.
> > What are you meaning to say here? I thought saying `AlignConsecutiveStyle` 
> > is used for multiple options is what we are trying to do.
> > 
> > Should we say something like, 
> > > Here AlignConsecutiveMacros is used as an example, but in practice 
> > > AlignConsecutiveStyle is also used with other options.
> > in this place?
> The example mentions explicitly only `AlignConsecutiveMacros` which is a bit 
> misleading if you are only looking at the documentation of 
> `AlignConsecutiveAssignments` for example.
> 
> This is not your fault, and I'm fine with ignoring it here. A separate patch 
> to fix that is welcome. :)
Got it. Reverted for now.



Comment at: clang/lib/Format/WhitespaceManager.cpp:930
   unsigned Newlines = 0;
+  unsigned int NewLineThreshold = 
Style.AlignConsecutiveTrailingComments.AcrossEmptyLines ? 2 : 1;
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {

HazardyKnusperkeks wrote:
> And accompanied by a short test. That should be everything to support the 
> mixture of the options, right?
> That should be everything to support the mixture of the options, right?
I think so.

Done.



Comment at: clang/lib/Format/WhitespaceManager.cpp:984
+}
+else if (BreakBeforeNext || Newlines > NewLineThreshold ||
(ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||

HazardyKnusperkeks wrote:
> Is this clang-format formatted?
I forgot that. Also formatted some other diffs.



Comment at: clang/unittests/Format/FormatTestComments.cpp:4262
 /\
-/ 
+/
   )",

HazardyKnusperkeks wrote:
> Please revert this.
Reverted.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki marked an inline comment as done.
yusuke-kadowaki added a comment.

Can we all agree with the decision whether we use the `AlignConsecutiveStyle` 
or introduce a new struct before I start implementing or updating the document?
IMO, both are reasonable in some respects so I'd like you owners to decide.

> Please also mark the comments as done once addressed so we know you’ve read 
> and fixed our requests

I'm reading all the comments but  leaving some comments not done on purpose 
that I think the discussion is not done. Isn't it a right thing to do here?




Comment at: clang/include/clang/Format/Format.h:298
+  /// Style of aligning consecutive trailing comments.
+  /// This option existed as ``AlignTrailingComments`` since version 3.7.
+  ///

MyDeveloperDay wrote:
> Isn’t this completely wrong?
Could you please be more specific about why you think so when pointing out 
something? I don't see why you think it's wrong. I also would like to mention 
that I followed @HazardyKnusperkeks's request on this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 457987.
yusuke-kadowaki marked 6 inline comments as done.
yusuke-kadowaki added a comment.

- Change to use verifyFormat


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,182 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing the combination with MaxEmptyLinesToKeep
+  Style.MaxEmptyLinesToKeep = 0;
+  verifyFormat("#include \"a.h\"  // comment\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  Style.MaxEmptyLinesToKeep = 1;
+  verifyFormat("#include \"a.h\"  // comment\n"
+   "\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  Style.MaxEmptyLinesToKeep = 2;
+  verifyFormat("#include \"a.h\"  // comment\n"
+   "\n"
+   "\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Reset the setting
+  Style.MaxEmptyLinesToKeep = 1;
+  // End of testing the combination with MaxEmptyLinesToKeep
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 35;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234; // This is a very long\n"
+"  // comment which is\n"
+"  // wrapped arround.\n"
+"\n"
+"int x =\n"
+"2; // Is this still aligned?\

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-05 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added inline comments.



Comment at: clang/lib/Format/Format.cpp:1196
   LLVMStyle.AlignConsecutiveMacros = {};
+  LLVMStyle.AlignConsecutiveTrailingComments = {};
+  LLVMStyle.AlignConsecutiveTrailingComments.Enabled = true;

MyDeveloperDay wrote:
> Won’t you need to initialise the members here as above
Members are `false` when initialized so I don't know why those above are 
initialized explicitly as `false`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-06 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 458176.
yusuke-kadowaki marked 7 inline comments as done.
yusuke-kadowaki added a comment.

- Introduce new struct
- Update document

This change should be ok for everyone (I hope). BTW, please correct my English 
if any or create a whole new one for me since I'm not so sure what I wrote 
delivers well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,182 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignTrailingComments = FormatStyle::TCAS_AlignAcrossEmptyLines;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing the combination with MaxEmptyLinesToKeep
+  Style.MaxEmptyLinesToKeep = 0;
+  verifyFormat("#include \"a.h\"  // comment\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  Style.MaxEmptyLinesToKeep = 1;
+  verifyFormat("#include \"a.h\"  // comment\n"
+   "\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  Style.MaxEmptyLinesToKeep = 2;
+  verifyFormat("#include \"a.h\"  // comment\n"
+   "\n"
+   "\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Reset the setting
+  Style.MaxEmptyLinesToKeep = 1;
+  // End of testing the combination with MaxEmptyLinesToKeep
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 35;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234; // This is a very l

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-06 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added inline comments.



Comment at: clang/unittests/Format/FormatTestComments.cpp:2930
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"

MyDeveloperDay wrote:
> Why not verifyFormat here too and below?
To test the ColumnLimit behavior.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-07 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki marked 3 inline comments as done.
yusuke-kadowaki added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:865
+
+  * ``TCAS_DontAlign`` (in configuration: ``DontAlign``)
+Don't align trailing comments.

MyDeveloperDay wrote:
> Is Don'tAlign the same as Leave that other options support  (for options it 
> best if we try and use the same terminiology
> 
> Always,Never,Leave (if that fits)
IMHO, `Leave` probably fits but `DontAlign`  is more straightforward. Also 
`DontAlign` is used for other alignment styles like `BracketAlignmentStyle` so 
it would be more natural.



Comment at: clang/include/clang/Format/Format.h:373
+  /// Different styles for aligning trailing comments.
+  enum TrailingCommentsAlignmentStyle : int8_t {
+/// Don't align trailing comments.

MyDeveloperDay wrote:
> all options have a life cycle
> 
> bool -> enum -> struct
> 
> One of the thing I ask you before, would we want to align across N empty 
> lines, if ever so they I think
> we should go straight to a struct
> all options have a life cycle

I see your point. But we are checking `Style.MaxEmptyLinesToKeep` to determine 
how many consecutive lines to align. So currently no need to specify it from 
the option. You'll find the implementation below.



Comment at: clang/unittests/Format/FormatTestComments.cpp:2930
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"

MyDeveloperDay wrote:
> yusuke-kadowaki wrote:
> > MyDeveloperDay wrote:
> > > Why not verifyFormat here too and below?
> > To test the ColumnLimit behavior.
> pretty sure you can pass in a style
Style isn't a problem here. But when testing column limit, you want to see 
before and after the line split into multiple lines. Original test cases of 
AlignTrailingComments also use this EXPECT_EQ style not verifyFormat.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-08 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki marked 7 inline comments as done.
yusuke-kadowaki added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:865
+
+  * ``TCAS_DontAlign`` (in configuration: ``DontAlign``)
+Don't align trailing comments.

HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > yusuke-kadowaki wrote:
> > > MyDeveloperDay wrote:
> > > > Is Don'tAlign the same as Leave that other options support  (for 
> > > > options it best if we try and use the same terminiology
> > > > 
> > > > Always,Never,Leave (if that fits)
> > > IMHO, `Leave` probably fits but `DontAlign`  is more straightforward. 
> > > Also `DontAlign` is used for other alignment styles like 
> > > `BracketAlignmentStyle` so it would be more natural.
> > Leave should mean do nothing.. I'm personally not a fan of DontAlign 
> > because obvious there should be a ' between the n and the t but I see we 
> > use it elsewhere
> > 
> > But actually now I see your DontAlign is effectively the as before (i.e. it 
> > will not add extra spaces) 
> > 
> > The point of Leave is what people have been asking for when we introduce a 
> > new option, that is we if possible add an option which means "Don't touch 
> > it at all"  i.e.  if I want to have
> > 
> > ```
> > int a;   //  abc
> > int b;   /// bcd
> > int c;// cde
> > ```
> > 
> > Then so be it
> > 
> > 
> Leave is a nice option, yes.
> I think it is complementary to `Dont`.
> 
> But maybe if the option is called `AlignTrailingComments` one may interpret 
> `Leave` not as "don't touch the position of a comment at all" (what do we do, 
> if the comment is outside of the column limit?), but as "just don't touch 
> them, when they are somewhat aligned". Just a thought.
> Leave should mean do nothing

Ok now I see what `Leave` means. 
But that should be another piece of work no? 

Of course me or someone can add the feature later (I don't really know how to 
implement that though at least for now) 





Comment at: clang/include/clang/Format/Format.h:373
+  /// Different styles for aligning trailing comments.
+  enum TrailingCommentsAlignmentStyle : int8_t {
+/// Don't align trailing comments.

HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > yusuke-kadowaki wrote:
> > > MyDeveloperDay wrote:
> > > > all options have a life cycle
> > > > 
> > > > bool -> enum -> struct
> > > > 
> > > > One of the thing I ask you before, would we want to align across N 
> > > > empty lines, if ever so they I think
> > > > we should go straight to a struct
> > > > all options have a life cycle
> > > 
> > > I see your point. But we are checking `Style.MaxEmptyLinesToKeep` to 
> > > determine how many consecutive lines to align. So currently no need to 
> > > specify it from the option. You'll find the implementation below.
> > I think its a bad idea to hijack a different option to do this..I don't 
> > think it means the same thing and I think what whilst it might work there 
> > will be someone somewhere who doesn't want it to behave like this and will 
> > call it out as a bug.
> > 
> > 
> Since you are strictly against `Enabled` what to put into a struct?
> Since you are strictly against Enabled what to put into a struct?

@MyDeveloperDay 
Can you answer this? Plus it would be helpful if you just write down what your 
ideal struct be like.

> I don't think it means the same thing

How so? There's no empty lines more than `MaxEmptyLinesToKeep` all the time no?



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-10 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 459292.
yusuke-kadowaki marked an inline comment as done.
yusuke-kadowaki added a comment.

Just updated the Style struct field definitions for review. Haven't implemented 
the logics.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,182 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignTrailingComments = FormatStyle::TCAS_AlignAcrossEmptyLines;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing the combination with MaxEmptyLinesToKeep
+  Style.MaxEmptyLinesToKeep = 0;
+  verifyFormat("#include \"a.h\"  // comment\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  Style.MaxEmptyLinesToKeep = 1;
+  verifyFormat("#include \"a.h\"  // comment\n"
+   "\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  Style.MaxEmptyLinesToKeep = 2;
+  verifyFormat("#include \"a.h\"  // comment\n"
+   "\n"
+   "\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Reset the setting
+  Style.MaxEmptyLinesToKeep = 1;
+  // End of testing the combination with MaxEmptyLinesToKeep
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 35;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234; // This is a very long\n"
+"  // comment which is\n"
+"  // wrapped arround.\n"
+"\n"
+

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-10 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki marked 2 inline comments as done.
yusuke-kadowaki added a comment.

Thank you for the detailed explanation. I understood the needs for `unsigned 
OverEmptyLines` field.
Please review the struct definition first. Then I'll implement the rest of the 
code.




Comment at: clang/include/clang/Format/Format.h:428-433
+bool operator==(const TrailingCommentsAlignmentStyle &R) const {
+  return Kind == R.Kind && OverEmptyLines == R.OverEmptyLines;
+}
+bool operator!=(const TrailingCommentsAlignmentStyle &R) const {
+  return !(*this == R);
+}

> I don't understand the need for state as a struct could have multiple options 
> (as enums) each enum should have a state that means "Leave"

@MyDeveloperDay 
Without having state, how can this be implemented?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-12 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki marked 2 inline comments as done.
yusuke-kadowaki added a comment.

So other than the naming, does the struct look good?




Comment at: clang/include/clang/Format/Format.h:406
+/// Specifies the way to align trailing comments
+TrailingCommentsAlignmentKinds Kind;
+/// How many empty lines to apply alignment

MyDeveloperDay wrote:
> Kind? doesn't feel like the right word
What do you recommend?



Comment at: clang/include/clang/Format/Format.h:428-433
+bool operator==(const TrailingCommentsAlignmentStyle &R) const {
+  return Kind == R.Kind && OverEmptyLines == R.OverEmptyLines;
+}
+bool operator!=(const TrailingCommentsAlignmentStyle &R) const {
+  return !(*this == R);
+}

MyDeveloperDay wrote:
> yusuke-kadowaki wrote:
> > > I don't understand the need for state as a struct could have multiple 
> > > options (as enums) each enum should have a state that means "Leave"
> > 
> > @MyDeveloperDay 
> > Without having state, how can this be implemented?
> bool Enabled  =  (Kind != FormatStyle::TCAS_Leave)
Oh ok so I think we are on the same page.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-15 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 460407.
yusuke-kadowaki marked 3 inline comments as done.
yusuke-kadowaki added a comment.

Implementation done except for the `Leave` option.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,187 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignTrailingComments.Kind = FormatStyle::TCAS_Always;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing OverEmptyLines
+  Style.MaxEmptyLinesToKeep = 3;
+  Style.AlignTrailingComments.OverEmptyLines = 2;
+  // Cannot use verifyFormat here
+  // test::messUp removes all new lines which changes the logic
+  EXPECT_EQ("#include \"a.h\" // comment\n"
+"\n"
+"\n"
+"\n"
+"#include \"ab.h\"  // comment\n"
+"\n"
+"\n"
+"#include \"abcdefg.h\" // comment\n",
+format("#include \"a.h\" // comment\n"
+   "\n"
+   "\n"
+   "\n"
+   "#include \"ab.h\" // comment\n"
+   "\n"
+   "\n"
+   "#include \"abcdefg.h\" // comment\n",
+   Style));
+
+  Style.MaxEmptyLinesToKeep = 1;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  // End of testing OverEmptyLines
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 35;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-15 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added inline comments.



Comment at: clang/include/clang/Format/Format.h:428
+
+bool operator==(const TrailingCommentsAlignmentStyle &R) const {
+  return Kind == R.Kind && OverEmptyLines == R.OverEmptyLines;

MyDeveloperDay wrote:
> yusuke-kadowaki wrote:
> > MyDeveloperDay wrote:
> > > yusuke-kadowaki wrote:
> > > > > I don't understand the need for state as a struct could have multiple 
> > > > > options (as enums) each enum should have a state that means "Leave"
> > > > 
> > > > @MyDeveloperDay 
> > > > Without having state, how can this be implemented?
> > > bool Enabled  =  (Kind != FormatStyle::TCAS_Leave)
> > Oh ok so I think we are on the same page.
> do you need this?
Yes. Without this it does not compile.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-09-15 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:865
+
+  * ``TCAS_DontAlign`` (in configuration: ``DontAlign``)
+Don't align trailing comments.

HazardyKnusperkeks wrote:
> yusuke-kadowaki wrote:
> > HazardyKnusperkeks wrote:
> > > MyDeveloperDay wrote:
> > > > yusuke-kadowaki wrote:
> > > > > MyDeveloperDay wrote:
> > > > > > Is Don'tAlign the same as Leave that other options support  (for 
> > > > > > options it best if we try and use the same terminiology
> > > > > > 
> > > > > > Always,Never,Leave (if that fits)
> > > > > IMHO, `Leave` probably fits but `DontAlign`  is more straightforward. 
> > > > > Also `DontAlign` is used for other alignment styles like 
> > > > > `BracketAlignmentStyle` so it would be more natural.
> > > > Leave should mean do nothing.. I'm personally not a fan of DontAlign 
> > > > because obvious there should be a ' between the n and the t but I see 
> > > > we use it elsewhere
> > > > 
> > > > But actually now I see your DontAlign is effectively the as before 
> > > > (i.e. it will not add extra spaces) 
> > > > 
> > > > The point of Leave is what people have been asking for when we 
> > > > introduce a new option, that is we if possible add an option which 
> > > > means "Don't touch it at all"  i.e.  if I want to have
> > > > 
> > > > ```
> > > > int a;   //  abc
> > > > int b;   /// bcd
> > > > int c;// cde
> > > > ```
> > > > 
> > > > Then so be it
> > > > 
> > > > 
> > > Leave is a nice option, yes.
> > > I think it is complementary to `Dont`.
> > > 
> > > But maybe if the option is called `AlignTrailingComments` one may 
> > > interpret `Leave` not as "don't touch the position of a comment at all" 
> > > (what do we do, if the comment is outside of the column limit?), but as 
> > > "just don't touch them, when they are somewhat aligned". Just a thought.
> > > Leave should mean do nothing
> > 
> > Ok now I see what `Leave` means. 
> > But that should be another piece of work no? 
> > 
> > Of course me or someone can add the feature later (I don't really know how 
> > to implement that though at least for now) 
> > 
> > 
> Fine by me.
@HazardyKnusperkeks 
Is this complicated? If it's not I might as well do this.

Also it would be helpful if you could provide some implementation guidance. 
Sorry to ask this even though I haven't tried it myself yet.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-10-18 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added a comment.

Thank you for all the reviews. Appreciate it.

So will some of you land this or am I supposed to do something else?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-10-18 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added a comment.

In D132131#3865004 , @rymiel wrote:

> Please provide a name and an email so someone could commit it on your behalf

name: Yusuke Kadowaki
email: yusuke.kadowaki.1...@gmail.com


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-10-29 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added a comment.

@MyDeveloperDay @HazardyKnusperkeks

Could you land this please if we are not waiting for anything?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137075: [clang-format] Fix document of AlignTrailingComments

2022-10-31 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki created this revision.
Herald added a project: All.
yusuke-kadowaki edited the summary of this revision.
yusuke-kadowaki added reviewers: MyDeveloperDay, HazardyKnusperkeks.
yusuke-kadowaki published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The documentation of the patch https://reviews.llvm.org/D132131 looks 
disorganized on the website 
https://clang.llvm.org/docs/ClangFormatStyleOptions.html.
This patch tries to fix that.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137075

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -402,11 +402,11 @@
 
   /// Alignment options
   struct TrailingCommentsAlignmentStyle {
-/// Specifies the way to align trailing comments
+/// Specifies the way to align trailing comments.
 TrailingCommentsAlignmentKinds Kind;
-/// How many empty lines to apply alignment
-/// With ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 2,
-/// \code
+/// How many empty lines to apply alignment.
+/// When both ``MaxEmptyLinesToKeep`` and ``OverEmptyLines`` are set to 2,
+/// it formats like below. \code
 ///   int a;  // all these
 ///
 ///   int ab; // comments are
@@ -414,8 +414,8 @@
 ///
 ///   int abcdef; // aligned
 /// \endcode
-/// And with ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 1,
-/// \code
+/// When ``MaxEmptyLinesToKeep`` is set to 2 and ``OverEmptyLines`` is set
+/// to 1, it formats like below. \code
 ///   int a;  // these are
 ///
 ///   int ab; // aligned
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -865,7 +865,7 @@
   Alignment options
 
   * ``TrailingCommentsAlignmentKinds Kind``
-Specifies the way to align trailing comments
+Specifies the way to align trailing comments.
 
 Possible values:
 
@@ -903,8 +903,8 @@
 int abcd; // comment
 
 
-  * ``unsigned OverEmptyLines`` How many empty lines to apply alignment
-With ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 2,
+  * ``unsigned OverEmptyLines`` How many empty lines to apply alignment.
+When both ``MaxEmptyLinesToKeep`` and ``OverEmptyLines`` are set to 2, it 
formats like below.
 
 .. code-block:: c++
 
@@ -914,8 +914,7 @@
 
 
   int abcdef; // aligned
-
-And with ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 1,
+When ``MaxEmptyLinesToKeep`` is set to 2 and ``OverEmptyLines`` is set to 
1, it formats like below.
 
 .. code-block:: c++
 


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -402,11 +402,11 @@
 
   /// Alignment options
   struct TrailingCommentsAlignmentStyle {
-/// Specifies the way to align trailing comments
+/// Specifies the way to align trailing comments.
 TrailingCommentsAlignmentKinds Kind;
-/// How many empty lines to apply alignment
-/// With ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 2,
-/// \code
+/// How many empty lines to apply alignment.
+/// When both ``MaxEmptyLinesToKeep`` and ``OverEmptyLines`` are set to 2,
+/// it formats like below. \code
 ///   int a;  // all these
 ///
 ///   int ab; // comments are
@@ -414,8 +414,8 @@
 ///
 ///   int abcdef; // aligned
 /// \endcode
-/// And with ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 1,
-/// \code
+/// When ``MaxEmptyLinesToKeep`` is set to 2 and ``OverEmptyLines`` is set
+/// to 1, it formats like below. \code
 ///   int a;  // these are
 ///
 ///   int ab; // aligned
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -865,7 +865,7 @@
   Alignment options
 
   * ``TrailingCommentsAlignmentKinds Kind``
-Specifies the way to align trailing comments
+Specifies the way to align trailing comments.
 
 Possible values:
 
@@ -903,8 +903,8 @@
 int abcd; // comment
 
 
-  * ``unsigned OverEmptyLines`` How many empty lines to apply alignment
-With ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 2,
+  * ``unsigned OverEmptyLines`` How many empty lines to apply alignment.
+When both ``MaxEmptyLinesToKeep`` and ``OverEmptyLines`` are set to 2, it formats like below.
 
 .. code-block:: c++
 
@@ -914,8 +914,7 @@
 
 
   int abcdef; // aligned
-
-And w

[PATCH] D137075: [clang-format] Fix document of AlignTrailingComments

2022-10-31 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 471977.
yusuke-kadowaki added a comment.

Add a blankline after the endcode to deal with the Buildbot error.
https://lab.llvm.org/buildbot/#/builders/92/builds/34906


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137075/new/

https://reviews.llvm.org/D137075

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -402,10 +402,11 @@
 
   /// Alignment options
   struct TrailingCommentsAlignmentStyle {
-/// Specifies the way to align trailing comments
+/// Specifies the way to align trailing comments.
 TrailingCommentsAlignmentKinds Kind;
-/// How many empty lines to apply alignment
-/// With ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 2,
+/// How many empty lines to apply alignment.
+/// When both ``MaxEmptyLinesToKeep`` and ``OverEmptyLines`` are set to 2,
+/// it formats like below.
 /// \code
 ///   int a;  // all these
 ///
@@ -414,7 +415,9 @@
 ///
 ///   int abcdef; // aligned
 /// \endcode
-/// And with ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 1,
+///
+/// When ``MaxEmptyLinesToKeep`` is set to 2 and ``OverEmptyLines`` is set
+/// to 1, it formats like below.
 /// \code
 ///   int a;  // these are
 ///
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -865,7 +865,7 @@
   Alignment options
 
   * ``TrailingCommentsAlignmentKinds Kind``
-Specifies the way to align trailing comments
+Specifies the way to align trailing comments.
 
 Possible values:
 
@@ -903,8 +903,9 @@
 int abcd; // comment
 
 
-  * ``unsigned OverEmptyLines`` How many empty lines to apply alignment
-With ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 2,
+  * ``unsigned OverEmptyLines`` How many empty lines to apply alignment.
+When both ``MaxEmptyLinesToKeep`` and ``OverEmptyLines`` are set to 2,
+it formats like below.
 
 .. code-block:: c++
 
@@ -915,7 +916,8 @@
 
   int abcdef; // aligned
 
-And with ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 1,
+When ``MaxEmptyLinesToKeep`` is set to 2 and ``OverEmptyLines`` is set
+to 1, it formats like below.
 
 .. code-block:: c++
 


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -402,10 +402,11 @@
 
   /// Alignment options
   struct TrailingCommentsAlignmentStyle {
-/// Specifies the way to align trailing comments
+/// Specifies the way to align trailing comments.
 TrailingCommentsAlignmentKinds Kind;
-/// How many empty lines to apply alignment
-/// With ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 2,
+/// How many empty lines to apply alignment.
+/// When both ``MaxEmptyLinesToKeep`` and ``OverEmptyLines`` are set to 2,
+/// it formats like below.
 /// \code
 ///   int a;  // all these
 ///
@@ -414,7 +415,9 @@
 ///
 ///   int abcdef; // aligned
 /// \endcode
-/// And with ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 1,
+///
+/// When ``MaxEmptyLinesToKeep`` is set to 2 and ``OverEmptyLines`` is set
+/// to 1, it formats like below.
 /// \code
 ///   int a;  // these are
 ///
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -865,7 +865,7 @@
   Alignment options
 
   * ``TrailingCommentsAlignmentKinds Kind``
-Specifies the way to align trailing comments
+Specifies the way to align trailing comments.
 
 Possible values:
 
@@ -903,8 +903,9 @@
 int abcd; // comment
 
 
-  * ``unsigned OverEmptyLines`` How many empty lines to apply alignment
-With ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 2,
+  * ``unsigned OverEmptyLines`` How many empty lines to apply alignment.
+When both ``MaxEmptyLinesToKeep`` and ``OverEmptyLines`` are set to 2,
+it formats like below.
 
 .. code-block:: c++
 
@@ -915,7 +916,8 @@
 
   int abcdef; // aligned
 
-And with ``MaxEmptyLinesToKeep`` is 2 and ``OverEmptyLines`` is 1,
+When ``MaxEmptyLinesToKeep`` is set to 2 and ``OverEmptyLines`` is set
+to 1, it formats like below.
 
 .. code-block:: c++
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D137075: [clang-format] Fix document of AlignTrailingComments

2022-11-01 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added a comment.

Thank you.
Please land when you have time.

name: Yusuke Kadowaki
email: yusuke.kadowaki.1...@gmail.com


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137075/new/

https://reviews.llvm.org/D137075

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-18 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki created this revision.
Herald added a project: All.
yusuke-kadowaki edited the summary of this revision.
yusuke-kadowaki added reviewers: curdeius, MyDeveloperDay, HazardyKnusperkeks.
yusuke-kadowaki published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch addresses https://github.com/llvm/llvm-project/issues/19756


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132131

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20042,6 +20042,7 @@
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE_BOOL(AlignTrailingComments);
+  CHECK_PARSE_BOOL(AlignTrailingCommentsIgnoreEmptyLine);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
@@ -26434,6 +26435,15 @@
"inline bool var = is_integral_v && is_signed_v;");
 }
 
+TEST_F(FormatTest, AlignTrailingCommentsIgnoreEmptyLine) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignTrailingCommentsIgnoreEmptyLine = true;
+  verifyFormat("#include \"a.h\"  //\n"
+   "\n"
+   "#include \"aa.h\" //\n",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -927,6 +927,7 @@
   unsigned StartOfSequence = 0;
   bool BreakBeforeNext = false;
   unsigned Newlines = 0;
+  unsigned int NewLineThr = Style.AlignTrailingCommentsIgnoreEmptyLine ? 2 : 1;
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
 if (Changes[i].StartOfBlockComment)
   continue;
@@ -979,7 +980,7 @@
   MinColumn = ChangeMinColumn;
   MaxColumn = ChangeMinColumn;
   StartOfSequence = i;
-} else if (BreakBeforeNext || Newlines > 1 ||
+else if (BreakBeforeNext || Newlines > NewLineThr ||
(ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||
// Break the comment sequence if the previous line did not end
// in a trailing comment.
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -647,6 +647,7 @@
 IO.mapOptional("AlignEscapedNewlines", Style.AlignEscapedNewlines);
 IO.mapOptional("AlignOperands", Style.AlignOperands);
 IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
+IO.mapOptional("AlignTrailingCommentsIgnoreEmptyLine", Style.AlignTrailingCommentsIgnoreEmptyLine);
 IO.mapOptional("AllowAllArgumentsOnNextLine",
Style.AllowAllArgumentsOnNextLine);
 IO.mapOptional("AllowAllParametersOfDeclarationOnNextLine",
@@ -1182,6 +1183,7 @@
   LLVMStyle.AlignArrayOfStructures = FormatStyle::AIAS_None;
   LLVMStyle.AlignOperands = FormatStyle::OAS_Align;
   LLVMStyle.AlignTrailingComments = true;
+  LLVMStyle.AlignTrailingCommentsIgnoreEmptyLine = false;
   LLVMStyle.AlignConsecutiveAssignments = {};
   LLVMStyle.AlignConsecutiveAssignments.Enabled = false;
   LLVMStyle.AlignConsecutiveAssignments.AcrossEmptyLines = false;
@@ -1448,6 +1450,7 @@
 GoogleStyle.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
 GoogleStyle.AlignOperands = FormatStyle::OAS_DontAlign;
 GoogleStyle.AlignTrailingComments = false;
+GoogleStyle.AlignTrailingCommentsIgnoreEmptyLine = false;
 GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
 GoogleStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
@@ -1596,6 +1599,7 @@
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
   Style.AlignOperands = FormatStyle::OAS_DontAlign;
   Style.AlignTrailingComments = false;
+  Style.AlignTrailingCommentsIgnoreEmptyLine = false;
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
   Style.BreakBeforeBraces = FormatStyle::BS_WebKit;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -378,6 +378,8 @@
   /// \version 3.7
   bool AlignTrailingComments;
 
+  bool AlignTrailingCommentsIgnoreEmptyLine;
+
   /// \brief If a function call or braced initializer list doesn't fit on a
   /// line, allow putting all arguments onto the next line, even i

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-23 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 454864.
yusuke-kadowaki edited the summary of this revision.
yusuke-kadowaki added a comment.

- [clang-format] Adds a formatter for aligning trailing comments over empty 
lines
- Remove unnecessary style specifications
- Add more tests
- Port AlignTrailingComments to AlignConsecutiveStyle
- Move tests to comments tests
- Remove AlignTrailingCommentsIgnoreEmptyLine
- Removed old AlignTrailingComments
- Add parsing config test
- Add more tests
- Add documentation


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,107 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  //\n"
+   "\n"
+   "#include \"aa.h\" //\n",
+   Style);
+   
+  verifyFormat("#include \"a.h\"   //\n"
+   "\n"
+   "#include \"aa.h\"  //\n"
+   "\n"
+   "#include \"aaa.h\" //\n",   
+   Style);
+   
+  verifyFormat("#include \"a.h\"  //\n"
+   "#include \"aa.h\" //\n"
+   "#include \"aaa.h\"//\n"
+   "\n"
+   "#include \".h\"   //\n"
+   "#include \"a.h\"  //\n"
+   "#include \"aa.h\" //\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  //\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" //\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   //\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  //\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" //\n",
+   Style);
+   
+  verifyFormat("#include \"a.h\"  //\n"
+   "#include \"aa.h\" //\n"
+   "#include \"aaa.h\"//\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   //\n"
+   "#include \"a.h\"  //\n"
+   "#include \"aa.h\" //\n",
+   Style);
+  
+  verifyFormat("#include \"a.h\"  //\n"
+   "\n"
+   "#include \"aa.h\" //\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\" // Do not align this because there are two lines without comments above\n",
+   Style);
+  
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+  
+  // FIXME: I think we need to change the implementations to pass tests below.
+  Style.ColumnLimit = 80;
+  EXPECT_EQ("int a; // line about a\n"
+"\n"
+"// line about b\n"
+"long b;",
+format("int a; // line about a\n"
+   "\n"
+   "   // line about b\n"
+   "   long b;",
+   Style));
+  
+  Style.ColumnLimit = 80;
+  EXPECT_EQ("int a; // line about a\n"
+"\n"
+"// line 1 about b\n"
+"// line 2 about b\n"
+"long b;",
+format("int a; // line about a\n"
+   "\n"
+   "   // line 1 about b\n"
+   "   // line 2 about b\n"
+   "   long b;",
+   Style));
+}
+
 TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
   EXPECT_EQ("/*\n"
 " */",
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -71,7 +71,10 @@
 ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Expected.str(), format(Expected, Style))
 << "Expected code is not stable

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-23 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added inline comments.



Comment at: clang/include/clang/Format/Format.h:379
   /// \version 3.7
   bool AlignTrailingComments;
 

HazardyKnusperkeks wrote:
> Maybe you can port this to `AlignConsecutiveStyle`? `AcrossComments` would 
> not affect anything in this context.
I took this approach



Comment at: clang/unittests/Format/FormatTest.cpp:74-77
+
+// FIXME: expectedとCodeをformatしたものがEQ
 EXPECT_EQ(Expected.str(), format(Code, Style));
+

Sorry I will remove these next time I update this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-25 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 455579.
yusuke-kadowaki marked 9 inline comments as done.
yusuke-kadowaki added a comment.

- Remove my comments
- Update documentation
- Sort


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,107 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  //\n"
+   "\n"
+   "#include \"aa.h\" //\n",
+   Style);
+   
+  verifyFormat("#include \"a.h\"   //\n"
+   "\n"
+   "#include \"aa.h\"  //\n"
+   "\n"
+   "#include \"aaa.h\" //\n",   
+   Style);
+   
+  verifyFormat("#include \"a.h\"  //\n"
+   "#include \"aa.h\" //\n"
+   "#include \"aaa.h\"//\n"
+   "\n"
+   "#include \".h\"   //\n"
+   "#include \"a.h\"  //\n"
+   "#include \"aa.h\" //\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  //\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" //\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   //\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  //\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" //\n",
+   Style);
+   
+  verifyFormat("#include \"a.h\"  //\n"
+   "#include \"aa.h\" //\n"
+   "#include \"aaa.h\"//\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   //\n"
+   "#include \"a.h\"  //\n"
+   "#include \"aa.h\" //\n",
+   Style);
+  
+  verifyFormat("#include \"a.h\"  //\n"
+   "\n"
+   "#include \"aa.h\" //\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\" // Do not align this because there are two lines without comments above\n",
+   Style);
+  
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+  
+  // FIXME: I think we need to change the implementations to pass tests below.
+  Style.ColumnLimit = 80;
+  EXPECT_EQ("int a; // line about a\n"
+"\n"
+"// line about b\n"
+"long b;",
+format("int a; // line about a\n"
+   "\n"
+   "   // line about b\n"
+   "   long b;",
+   Style));
+  
+  Style.ColumnLimit = 80;
+  EXPECT_EQ("int a; // line about a\n"
+"\n"
+"// line 1 about b\n"
+"// line 2 about b\n"
+"long b;",
+format("int a; // line about a\n"
+   "\n"
+   "   // line 1 about b\n"
+   "   // line 2 about b\n"
+   "   long b;",
+   Style));
+}
+
 TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
   EXPECT_EQ("/*\n"
 " */",
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20041,7 +20041,6 @@
 TEST_F(FormatTest, ParsesConfigurationBools) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
-  CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
@@ -20212,6 +20211,10 @@
   CHECK_ALIGN_CONSECUTIVE(AlignConsecutiveBitFields);
   CHECK_ALIGN_CONSECUTIVE(Align

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-10-10 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 466504.
yusuke-kadowaki added a comment.

Implment trailing comments Leave option

There is two options, I think, when leaving comments exceeds the column limit.

1. Just format the trailing comments
2. Ignore the column limit for that line.

This implementation does the first option because ignoring column limit semms 
too much for just taking care of trailing comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,213 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignTrailingComments.Kind = FormatStyle::TCAS_Always;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing OverEmptyLines
+  Style.MaxEmptyLinesToKeep = 3;
+  Style.AlignTrailingComments.OverEmptyLines = 2;
+  // Cannot use verifyFormat here
+  // test::messUp removes all new lines which changes the logic
+  EXPECT_EQ("#include \"a.h\" // comment\n"
+"\n"
+"\n"
+"\n"
+"#include \"ab.h\"  // comment\n"
+"\n"
+"\n"
+"#include \"abcdefg.h\" // comment\n",
+format("#include \"a.h\" // comment\n"
+   "\n"
+   "\n"
+   "\n"
+   "#include \"ab.h\" // comment\n"
+   "\n"
+   "\n"
+   "#include \"abcdefg.h\" // comment\n",
+   Style));
+
+  Style.MaxEmptyLinesToKeep = 1;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  // End of testing OverEmptyLines
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// w

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-10-10 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki marked an inline comment as done.
yusuke-kadowaki added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:865
+
+  * ``TCAS_DontAlign`` (in configuration: ``DontAlign``)
+Don't align trailing comments.

HazardyKnusperkeks wrote:
> yusuke-kadowaki wrote:
> > HazardyKnusperkeks wrote:
> > > yusuke-kadowaki wrote:
> > > > HazardyKnusperkeks wrote:
> > > > > MyDeveloperDay wrote:
> > > > > > yusuke-kadowaki wrote:
> > > > > > > MyDeveloperDay wrote:
> > > > > > > > Is Don'tAlign the same as Leave that other options support  
> > > > > > > > (for options it best if we try and use the same terminiology
> > > > > > > > 
> > > > > > > > Always,Never,Leave (if that fits)
> > > > > > > IMHO, `Leave` probably fits but `DontAlign`  is more 
> > > > > > > straightforward. Also `DontAlign` is used for other alignment 
> > > > > > > styles like `BracketAlignmentStyle` so it would be more natural.
> > > > > > Leave should mean do nothing.. I'm personally not a fan of 
> > > > > > DontAlign because obvious there should be a ' between the n and the 
> > > > > > t but I see we use it elsewhere
> > > > > > 
> > > > > > But actually now I see your DontAlign is effectively the as before 
> > > > > > (i.e. it will not add extra spaces) 
> > > > > > 
> > > > > > The point of Leave is what people have been asking for when we 
> > > > > > introduce a new option, that is we if possible add an option which 
> > > > > > means "Don't touch it at all"  i.e.  if I want to have
> > > > > > 
> > > > > > ```
> > > > > > int a;   //  abc
> > > > > > int b;   /// bcd
> > > > > > int c;// cde
> > > > > > ```
> > > > > > 
> > > > > > Then so be it
> > > > > > 
> > > > > > 
> > > > > Leave is a nice option, yes.
> > > > > I think it is complementary to `Dont`.
> > > > > 
> > > > > But maybe if the option is called `AlignTrailingComments` one may 
> > > > > interpret `Leave` not as "don't touch the position of a comment at 
> > > > > all" (what do we do, if the comment is outside of the column limit?), 
> > > > > but as "just don't touch them, when they are somewhat aligned". Just 
> > > > > a thought.
> > > > > Leave should mean do nothing
> > > > 
> > > > Ok now I see what `Leave` means. 
> > > > But that should be another piece of work no? 
> > > > 
> > > > Of course me or someone can add the feature later (I don't really know 
> > > > how to implement that though at least for now) 
> > > > 
> > > > 
> > > Fine by me.
> > @HazardyKnusperkeks 
> > Is this complicated? If it's not I might as well do this.
> > 
> > Also it would be helpful if you could provide some implementation guidance. 
> > Sorry to ask this even though I haven't tried it myself yet.
> > @HazardyKnusperkeks 
> > Is this complicated? If it's not I might as well do this.
> > 
> > Also it would be helpful if you could provide some implementation guidance. 
> > Sorry to ask this even though I haven't tried it myself yet.
> 
> I think you refer to the non aligning of comments following r_braces. I don't 
> think so, because at least the cases I think about the r_brace should be the 
> first token on a unwrapped line, so you just have to check for Line->First.
Isn't this for https://github.com/llvm/llvm-project/issues/57504?
I thought that's different than leaving all the trailing comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-10-13 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 467458.
yusuke-kadowaki marked 3 inline comments as done.
yusuke-kadowaki added a comment.

Added more tests
Removed braces


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,224 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignTrailingComments.Kind = FormatStyle::TCAS_Always;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing OverEmptyLines
+  Style.MaxEmptyLinesToKeep = 3;
+  Style.AlignTrailingComments.OverEmptyLines = 2;
+  // Cannot use verifyFormat here
+  // test::messUp removes all new lines which changes the logic
+  EXPECT_EQ("#include \"a.h\" // comment\n"
+"\n"
+"\n"
+"\n"
+"#include \"ab.h\"  // comment\n"
+"\n"
+"\n"
+"#include \"abcdefg.h\" // comment\n",
+format("#include \"a.h\" // comment\n"
+   "\n"
+   "\n"
+   "\n"
+   "#include \"ab.h\" // comment\n"
+   "\n"
+   "\n"
+   "#include \"abcdefg.h\" // comment\n",
+   Style));
+
+  Style.MaxEmptyLinesToKeep = 1;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  // End of testing OverEmptyLines
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 35;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-10-15 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki updated this revision to Diff 468015.
yusuke-kadowaki marked 2 inline comments as done.
yusuke-kadowaki added a comment.

- Rebased
- Remove more braces
- Update rst


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132131/new/

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,224 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignTrailingComments.Kind = FormatStyle::TCAS_Always;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing OverEmptyLines
+  Style.MaxEmptyLinesToKeep = 3;
+  Style.AlignTrailingComments.OverEmptyLines = 2;
+  // Cannot use verifyFormat here
+  // test::messUp removes all new lines which changes the logic
+  EXPECT_EQ("#include \"a.h\" // comment\n"
+"\n"
+"\n"
+"\n"
+"#include \"ab.h\"  // comment\n"
+"\n"
+"\n"
+"#include \"abcdefg.h\" // comment\n",
+format("#include \"a.h\" // comment\n"
+   "\n"
+   "\n"
+   "\n"
+   "#include \"ab.h\" // comment\n"
+   "\n"
+   "\n"
+   "#include \"abcdefg.h\" // comment\n",
+   Style));
+
+  Style.MaxEmptyLinesToKeep = 1;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  // End of testing OverEmptyLines
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 35;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int ba

[PATCH] D139029: [clang-format] Don't move comments if AlignTrailingComments: Kind: Leave

2022-12-01 Thread Yusuke Kadowaki via Phabricator via cfe-commits 
yusuke-kadowaki added a comment.

@mairacanal 
Thank you for the catch!




Comment at: clang/unittests/Format/FormatTestComments.cpp:3101
+  // Allow to keep 2 empty lines
+  Style.MaxEmptyLinesToKeep = 2;
+  EXPECT_EQ("// do not touch\n"

This should probably be reset after this testcase to avoid side effects for 
other test cases.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139029/new/

https://reviews.llvm.org/D139029

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits