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

2022-11-30 Thread Maíra Canal via Phabricator via cfe-commits
mairacanal created this revision.
mairacanal added reviewers: MyDeveloperDay, owenpan, yusuke-kadowaki, 
HazardyKnusperkeks.
Herald added a project: All.
mairacanal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For comments that start after a new line, currently, the comments are
being indented. This happens because the OriginalWhitespaceRange
considers newlines on the range. Therefore, when AlignTrailingComments:
Kind: Leave, deduct the number of newlines before the token to calculate
the number of spaces for trailing comments.

Fixes llvm#59203


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139029

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTestComments.cpp


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3062,6 +3062,18 @@
"int d;// comment\n",
Style));
 
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n\n"
+"   // comment\n"
+"// comment\n\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n\n"
+   "   // comment\n"
+   "// comment\n\n"
+   "// comment\n",
+   Style));
+
   // Just format comments normally when leaving exceeds the column limit
   Style.ColumnLimit = 35;
   EXPECT_EQ("int foo = 12345; // comment\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -957,7 +957,8 @@
 if (Style.AlignTrailingComments.Kind == FormatStyle::TCAS_Leave) {
   auto OriginalSpaces =
   Changes[i].OriginalWhitespaceRange.getEnd().getRawEncoding() -
-  Changes[i].OriginalWhitespaceRange.getBegin().getRawEncoding();
+  Changes[i].OriginalWhitespaceRange.getBegin().getRawEncoding() -
+  Changes[i].NewlinesBefore;
   unsigned RestoredLineLength = Changes[i].StartOfTokenColumn +
 Changes[i].TokenLength + OriginalSpaces;
   // If leaving comments makes the line exceed the column limit, give up to


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3062,6 +3062,18 @@
"int d;// comment\n",
Style));
 
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n\n"
+"   // comment\n"
+"// comment\n\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n\n"
+   "   // comment\n"
+   "// comment\n\n"
+   "// comment\n",
+   Style));
+
   // Just format comments normally when leaving exceeds the column limit
   Style.ColumnLimit = 35;
   EXPECT_EQ("int foo = 12345; // comment\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -957,7 +957,8 @@
 if (Style.AlignTrailingComments.Kind == FormatStyle::TCAS_Leave) {
   auto OriginalSpaces =
   Changes[i].OriginalWhitespaceRange.getEnd().getRawEncoding() -
-  Changes[i].OriginalWhitespaceRange.getBegin().getRawEncoding();
+  Changes[i].OriginalWhitespaceRange.getBegin().getRawEncoding() -
+  Changes[i].NewlinesBefore;
   unsigned RestoredLineLength = Changes[i].StartOfTokenColumn +
 Changes[i].TokenLength + OriginalSpaces;
   // If leaving comments makes the line exceed the column limit, give up to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2022-11-30 Thread Maíra Canal via Phabricator via cfe-commits
mairacanal added a comment.

In D139029#3961372 , 
@HazardyKnusperkeks wrote:

> Can you please add a test with more than one newline?

Hi @HazardyKnusperkeks! Thanks for the feedback. It looks like 
`Changes[i].NewlinesBefore` values 1 even if I put multiple lines before the 
comment. Do you have any input on what could be causing this? From my point of 
view, it looks like the extra new lines are being removed before 
`alignTrailingComments` and then `Changes[i].NewlinesBefore = 1`, but I'm not 
really sure how to solve it.


Repository:
  rG LLVM Github Monorepo

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


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

2022-11-30 Thread Maíra Canal via Phabricator via cfe-commits
mairacanal added a comment.

In D139029#3961446 , 
@HazardyKnusperkeks wrote:

> In D139029#3961444 , 
> @HazardyKnusperkeks wrote:
>
>> In D139029#3961438 , @mairacanal 
>> wrote:
>>
>>> In D139029#3961372 , 
>>> @HazardyKnusperkeks wrote:
>>>
 Can you please add a test with more than one newline?
>>>
>>> Hi @HazardyKnusperkeks! Thanks for the feedback. It looks like 
>>> `Changes[i].NewlinesBefore` values 1 even if I put multiple lines before 
>>> the comment. Do you have any input on what could be causing this? From my 
>>> point of view, it looks like the extra new lines are being removed before 
>>> `alignTrailingComments` and then `Changes[i].NewlinesBefore = 1`, but I'm 
>>> not really sure how to solve it.
>>
>> Try setting `MaxEmptyLinesToKeep` to a higher number.
>
> We can also use two additional test cases, with `MaxEmptyLinesToKeep` and 
> without, always good to see that it works.

If I don't set  `MaxEmptyLinesToKeep` to a higher number, then 
Changes[i].NewlinesBefore = 1 and some spaces will be added by the beginning of 
the line. Can I just create the test case with  `MaxEmptyLinesToKeep`?


Repository:
  rG LLVM Github Monorepo

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


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

2022-11-30 Thread Maíra Canal via Phabricator via cfe-commits
mairacanal updated this revision to Diff 479055.
mairacanal added a comment.

- Instead of using Changes[i].NewlinesBefore use Changes[i].Tok->NewlinesBefore
- Create test cases of multiple blank lines
- Separate \n\n on test cases


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

https://reviews.llvm.org/D139029

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTestComments.cpp


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3062,6 +3062,61 @@
"int d;// comment\n",
Style));
 
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "// comment\n",
+   Style));
+
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "\n"
+   "// comment\n",
+   Style));
+
+
+  // Allow to keep 2 empty lines
+  Style.MaxEmptyLinesToKeep = 2;
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "// comment\n",
+   Style));
+
   // Just format comments normally when leaving exceeds the column limit
   Style.ColumnLimit = 35;
   EXPECT_EQ("int foo = 12345; // comment\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -957,7 +957,8 @@
 if (Style.AlignTrailingComments.Kind == FormatStyle::TCAS_Leave) {
   auto OriginalSpaces =
   Changes[i].OriginalWhitespaceRange.getEnd().getRawEncoding() -
-  Changes[i].OriginalWhitespaceRange.getBegin().getRawEncoding();
+  Changes[i].OriginalWhitespaceRange.getBegin().getRawEncoding() -
+  Changes[i].Tok->NewlinesBefore;
   unsigned RestoredLineLength = Changes[i].StartOfTokenColumn +
 Changes[i].TokenLength + OriginalSpaces;
   // If leaving comments makes the line exceed the column limit, give up to


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3062,6 +3062,61 @@
"int d;// comment\n",
Style));
 
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "// comment\n",
+   Style));
+
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "\n"
+   "// comment\n",
+   Style));
+
+
+  // Allow to keep 2 empty lines
+  Style.MaxEmptyLinesToKeep = 2;
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n"

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

2022-11-30 Thread Maíra Canal via Phabricator via cfe-commits
mairacanal added a comment.

In D139029#3961512 , 
@HazardyKnusperkeks wrote:

> Thanks, nice work.
>
> Do you need someone to push it for you? In that case we need a name and an 
> email address for the commit.

Yes, I would need someone to push for me. My name and address are Maíra Canal 
. Thanks for the help!


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


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

2022-12-01 Thread Maíra Canal via Phabricator via cfe-commits
mairacanal updated this revision to Diff 479312.
mairacanal added a comment.

Reset `Style.MaxEmptyLinesToKeep` after the test case to avoid side effects for 
other test cases.


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

https://reviews.llvm.org/D139029

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTestComments.cpp


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3062,6 +3062,61 @@
"int d;// comment\n",
Style));
 
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "// comment\n",
+   Style));
+
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "\n"
+   "// comment\n",
+   Style));
+
+  // Allow to keep 2 empty lines
+  Style.MaxEmptyLinesToKeep = 2;
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "// comment\n",
+   Style));
+  Style.MaxEmptyLinesToKeep = 1;
+
   // Just format comments normally when leaving exceeds the column limit
   Style.ColumnLimit = 35;
   EXPECT_EQ("int foo = 12345; // comment\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -957,7 +957,8 @@
 if (Style.AlignTrailingComments.Kind == FormatStyle::TCAS_Leave) {
   auto OriginalSpaces =
   Changes[i].OriginalWhitespaceRange.getEnd().getRawEncoding() -
-  Changes[i].OriginalWhitespaceRange.getBegin().getRawEncoding();
+  Changes[i].OriginalWhitespaceRange.getBegin().getRawEncoding() -
+  Changes[i].Tok->NewlinesBefore;
   unsigned RestoredLineLength = Changes[i].StartOfTokenColumn +
 Changes[i].TokenLength + OriginalSpaces;
   // If leaving comments makes the line exceed the column limit, give up to


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3062,6 +3062,61 @@
"int d;// comment\n",
Style));
 
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "// comment\n",
+   Style));
+
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "\n"
+   "// comment\n",
+   Style));
+
+  // Allow to keep 2 empty lines
+  Style.MaxEmptyLinesToKeep = 2;
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment\n",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"

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

2022-12-01 Thread Maíra Canal via Phabricator via cfe-commits
mairacanal updated this revision to Diff 479455.
mairacanal marked 8 inline comments as done.
mairacanal added a comment.

Remove \n from the end of the test cases


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

https://reviews.llvm.org/D139029

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTestComments.cpp


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3062,6 +3062,61 @@
"int d;// comment\n",
Style));
 
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "// comment",
+   Style));
+
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "\n"
+   "// comment",
+   Style));
+
+  // Allow to keep 2 empty lines
+  Style.MaxEmptyLinesToKeep = 2;
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "// comment",
+   Style));
+  Style.MaxEmptyLinesToKeep = 1;
+
   // Just format comments normally when leaving exceeds the column limit
   Style.ColumnLimit = 35;
   EXPECT_EQ("int foo = 12345; // comment\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -957,7 +957,8 @@
 if (Style.AlignTrailingComments.Kind == FormatStyle::TCAS_Leave) {
   auto OriginalSpaces =
   Changes[i].OriginalWhitespaceRange.getEnd().getRawEncoding() -
-  Changes[i].OriginalWhitespaceRange.getBegin().getRawEncoding();
+  Changes[i].OriginalWhitespaceRange.getBegin().getRawEncoding() -
+  Changes[i].Tok->NewlinesBefore;
   unsigned RestoredLineLength = Changes[i].StartOfTokenColumn +
 Changes[i].TokenLength + OriginalSpaces;
   // If leaving comments makes the line exceed the column limit, give up to


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3062,6 +3062,61 @@
"int d;// comment\n",
Style));
 
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "// comment",
+   Style));
+
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "\n"
+   "   // comment\n"
+   "// comment\n"
+   "\n"
+   "\n"
+   "// comment",
+   Style));
+
+  // Allow to keep 2 empty lines
+  Style.MaxEmptyLinesToKeep = 2;
+  EXPECT_EQ("// do not touch\n"
+"int a;  // any comments\n"
+"\n"
+"\n"
+"   // comment\n"
+"// comment\n"
+"\n"
+"// comment",
+format("// do not touch\n"
+   "int a;  // any comments\n"
+   "\n"
+   "\n"
+