[PATCH] D124452: [clang-format] Adjust editor cursor position past #include blocks

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
owenpan marked an inline comment as done.
Closed by commit rGdb57acff2647: [clang-format] Adjust editor cursor position 
past #include blocks (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124452

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -890,6 +890,21 @@
   EXPECT_EQ(10u, newCursor(Code, 43));
 }
 
+TEST_F(SortIncludesTest, CalculatesCorrectCursorPositionWithRegrouping) {
+  Style.IncludeBlocks = Style.IBS_Regroup;
+  std::string Code = "#include \"b\"\n"  // Start of line: 0
+ "\n"// Start of line: 13
+ "#include \"aa\"\n" // Start of line: 14
+ "int i;";   // Start of line: 28
+  std::string Expected = "#include \"aa\"\n" // Start of line: 0
+ "#include \"b\"\n"  // Start of line: 14
+ "int i;";   // Start of line: 27
+  EXPECT_EQ(Expected, sort(Code));
+  EXPECT_EQ(12u, newCursor(Code, 26)); // Closing quote of "aa"
+  EXPECT_EQ(26u, newCursor(Code, 27)); // Newline after "aa"
+  EXPECT_EQ(27u, newCursor(Code, 28)); // Start of last line
+}
+
 TEST_F(SortIncludesTest, DeduplicateIncludes) {
   EXPECT_EQ("#include \n"
 "#include \n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2638,10 +2638,10 @@
 StringRef Code, tooling::Replacements ,
 unsigned *Cursor) {
   tooling::IncludeCategoryManager Categories(Style.IncludeStyle, FileName);
-  unsigned IncludesBeginOffset = Includes.front().Offset;
-  unsigned IncludesEndOffset =
+  const unsigned IncludesBeginOffset = Includes.front().Offset;
+  const unsigned IncludesEndOffset =
   Includes.back().Offset + Includes.back().Text.size();
-  unsigned IncludesBlockSize = IncludesEndOffset - IncludesBeginOffset;
+  const unsigned IncludesBlockSize = IncludesEndOffset - IncludesBeginOffset;
   if (!affectsRange(Ranges, IncludesBeginOffset, IncludesEndOffset))
 return;
   SmallVector Indices =
@@ -2685,7 +2685,7 @@
   // the entire block. Otherwise, no replacement is generated.
   // In case Style.IncldueStyle.IncludeBlocks != IBS_Preserve, this check is 
not
   // enough as additional newlines might be added or removed across #include
-  // blocks. This we handle below by generating the updated #imclude blocks and
+  // blocks. This we handle below by generating the updated #include blocks and
   // comparing it to the original.
   if (Indices.size() == Includes.size() && llvm::is_sorted(Indices) &&
   Style.IncludeStyle.IncludeBlocks == tooling::IncludeStyle::IBS_Preserve)
@@ -2706,6 +2706,9 @@
 CurrentCategory = Includes[Index].Category;
   }
 
+  if (Cursor && *Cursor >= IncludesEndOffset)
+*Cursor += result.size() - IncludesBlockSize;
+
   // If the #includes are out of order, we generate a single replacement fixing
   // the entire range of blocks. Otherwise, no replacement is generated.
   if (replaceCRLF(result) == replaceCRLF(std::string(Code.substr(


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -890,6 +890,21 @@
   EXPECT_EQ(10u, newCursor(Code, 43));
 }
 
+TEST_F(SortIncludesTest, CalculatesCorrectCursorPositionWithRegrouping) {
+  Style.IncludeBlocks = Style.IBS_Regroup;
+  std::string Code = "#include \"b\"\n"  // Start of line: 0
+ "\n"// Start of line: 13
+ "#include \"aa\"\n" // Start of line: 14
+ "int i;";   // Start of line: 28
+  std::string Expected = "#include \"aa\"\n" // Start of line: 0
+ "#include \"b\"\n"  // Start of line: 14
+ "int i;";   // Start of line: 27
+  EXPECT_EQ(Expected, sort(Code));
+  EXPECT_EQ(12u, newCursor(Code, 26)); // Closing quote of "aa"
+  EXPECT_EQ(26u, newCursor(Code, 27)); // Newline after "aa"
+  EXPECT_EQ(27u, newCursor(Code, 28)); // Start of last line
+}
+
 TEST_F(SortIncludesTest, DeduplicateIncludes) {
   EXPECT_EQ("#include \n"
 "#include \n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2638,10 +2638,10 @@
 StringRef 

[PATCH] D124452: [clang-format] Adjust editor cursor position past #include blocks

[Owen Pan via Phabricator via cfe-commits]

owenpan marked an inline comment as done.
owenpan added inline comments.



Comment at: clang/unittests/Format/SortIncludesTest.cpp:895
+  Style.IncludeBlocks = Style.IBS_Regroup;
+  std::string Code = "#include \"b\"\n"  // Start of line: 0
+ "\n"// Start of line: 13

HazardyKnusperkeks wrote:
> Should we add a check for the formatted text? So that one could easily see 
> where the cursor is moved.
Yep!


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

https://reviews.llvm.org/D124452

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

[PATCH] D124452: [clang-format] Adjust editor cursor position past #include blocks

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 425376.
owenpan added a comment.

Updated the test case.


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

https://reviews.llvm.org/D124452

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -890,6 +890,21 @@
   EXPECT_EQ(10u, newCursor(Code, 43));
 }
 
+TEST_F(SortIncludesTest, CalculatesCorrectCursorPositionWithRegrouping) {
+  Style.IncludeBlocks = Style.IBS_Regroup;
+  std::string Code = "#include \"b\"\n"  // Start of line: 0
+ "\n"// Start of line: 13
+ "#include \"aa\"\n" // Start of line: 14
+ "int i;";   // Start of line: 28
+  std::string Expected = "#include \"aa\"\n" // Start of line: 0
+ "#include \"b\"\n"  // Start of line: 14
+ "int i;";   // Start of line: 27
+  EXPECT_EQ(Expected, sort(Code));
+  EXPECT_EQ(12u, newCursor(Code, 26)); // Closing quote of "aa"
+  EXPECT_EQ(26u, newCursor(Code, 27)); // Newline after "aa"
+  EXPECT_EQ(27u, newCursor(Code, 28)); // Start of last line
+}
+
 TEST_F(SortIncludesTest, DeduplicateIncludes) {
   EXPECT_EQ("#include \n"
 "#include \n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2638,10 +2638,10 @@
 StringRef Code, tooling::Replacements ,
 unsigned *Cursor) {
   tooling::IncludeCategoryManager Categories(Style.IncludeStyle, FileName);
-  unsigned IncludesBeginOffset = Includes.front().Offset;
-  unsigned IncludesEndOffset =
+  const unsigned IncludesBeginOffset = Includes.front().Offset;
+  const unsigned IncludesEndOffset =
   Includes.back().Offset + Includes.back().Text.size();
-  unsigned IncludesBlockSize = IncludesEndOffset - IncludesBeginOffset;
+  const unsigned IncludesBlockSize = IncludesEndOffset - IncludesBeginOffset;
   if (!affectsRange(Ranges, IncludesBeginOffset, IncludesEndOffset))
 return;
   SmallVector Indices =
@@ -2685,7 +2685,7 @@
   // the entire block. Otherwise, no replacement is generated.
   // In case Style.IncldueStyle.IncludeBlocks != IBS_Preserve, this check is 
not
   // enough as additional newlines might be added or removed across #include
-  // blocks. This we handle below by generating the updated #imclude blocks and
+  // blocks. This we handle below by generating the updated #include blocks and
   // comparing it to the original.
   if (Indices.size() == Includes.size() && llvm::is_sorted(Indices) &&
   Style.IncludeStyle.IncludeBlocks == tooling::IncludeStyle::IBS_Preserve)
@@ -2706,6 +2706,9 @@
 CurrentCategory = Includes[Index].Category;
   }
 
+  if (Cursor && *Cursor >= IncludesEndOffset)
+*Cursor += result.size() - IncludesBlockSize;
+
   // If the #includes are out of order, we generate a single replacement fixing
   // the entire range of blocks. Otherwise, no replacement is generated.
   if (replaceCRLF(result) == replaceCRLF(std::string(Code.substr(


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -890,6 +890,21 @@
   EXPECT_EQ(10u, newCursor(Code, 43));
 }
 
+TEST_F(SortIncludesTest, CalculatesCorrectCursorPositionWithRegrouping) {
+  Style.IncludeBlocks = Style.IBS_Regroup;
+  std::string Code = "#include \"b\"\n"  // Start of line: 0
+ "\n"// Start of line: 13
+ "#include \"aa\"\n" // Start of line: 14
+ "int i;";   // Start of line: 28
+  std::string Expected = "#include \"aa\"\n" // Start of line: 0
+ "#include \"b\"\n"  // Start of line: 14
+ "int i;";   // Start of line: 27
+  EXPECT_EQ(Expected, sort(Code));
+  EXPECT_EQ(12u, newCursor(Code, 26)); // Closing quote of "aa"
+  EXPECT_EQ(26u, newCursor(Code, 27)); // Newline after "aa"
+  EXPECT_EQ(27u, newCursor(Code, 28)); // Start of last line
+}
+
 TEST_F(SortIncludesTest, DeduplicateIncludes) {
   EXPECT_EQ("#include \n"
 "#include \n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2638,10 +2638,10 @@
 StringRef Code, tooling::Replacements ,
 unsigned *Cursor) {
   tooling::IncludeCategoryManager Categories(Style.IncludeStyle, FileName);
-  unsigned 

[PATCH] D124452: [clang-format] Adjust editor cursor position past #include blocks

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added inline comments.



Comment at: clang/unittests/Format/SortIncludesTest.cpp:895
+  Style.IncludeBlocks = Style.IBS_Regroup;
+  std::string Code = "#include \"b\"\n"  // Start of line: 0
+ "\n"// Start of line: 13

Should we add a check for the formatted text? So that one could easily see 
where the cursor is moved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124452

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

[PATCH] D124452: [clang-format] Adjust editor cursor position past #include blocks

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

Looks ok, but I'm not very familiar with this part of clang-format, so please 
leave some time for others to chime in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124452

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

[PATCH] D124452: [clang-format] Adjust editor cursor position past #include blocks

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: curdeius, HazardyKnusperkeks, MyDeveloperDay.
owenpan added a project: clang-format.
Herald added a project: All.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/55027.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124452

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -890,6 +890,17 @@
   EXPECT_EQ(10u, newCursor(Code, 43));
 }
 
+TEST_F(SortIncludesTest, CalculatesCorrectCursorPositionWithRegrouping) {
+  Style.IncludeBlocks = Style.IBS_Regroup;
+  std::string Code = "#include \"b\"\n"  // Start of line: 0
+ "\n"// Start of line: 13
+ "#include \"aa\"\n" // Start of line: 14
+ "f();\n";   // Start of line: 28
+  EXPECT_EQ(12u, newCursor(Code, 26));
+  EXPECT_EQ(26u, newCursor(Code, 27));
+  EXPECT_EQ(27u, newCursor(Code, 28));
+}
+
 TEST_F(SortIncludesTest, DeduplicateIncludes) {
   EXPECT_EQ("#include \n"
 "#include \n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2638,10 +2638,10 @@
 StringRef Code, tooling::Replacements ,
 unsigned *Cursor) {
   tooling::IncludeCategoryManager Categories(Style.IncludeStyle, FileName);
-  unsigned IncludesBeginOffset = Includes.front().Offset;
-  unsigned IncludesEndOffset =
+  const unsigned IncludesBeginOffset = Includes.front().Offset;
+  const unsigned IncludesEndOffset =
   Includes.back().Offset + Includes.back().Text.size();
-  unsigned IncludesBlockSize = IncludesEndOffset - IncludesBeginOffset;
+  const unsigned IncludesBlockSize = IncludesEndOffset - IncludesBeginOffset;
   if (!affectsRange(Ranges, IncludesBeginOffset, IncludesEndOffset))
 return;
   SmallVector Indices =
@@ -2685,7 +2685,7 @@
   // the entire block. Otherwise, no replacement is generated.
   // In case Style.IncldueStyle.IncludeBlocks != IBS_Preserve, this check is 
not
   // enough as additional newlines might be added or removed across #include
-  // blocks. This we handle below by generating the updated #imclude blocks and
+  // blocks. This we handle below by generating the updated #include blocks and
   // comparing it to the original.
   if (Indices.size() == Includes.size() && llvm::is_sorted(Indices) &&
   Style.IncludeStyle.IncludeBlocks == tooling::IncludeStyle::IBS_Preserve)
@@ -2706,6 +2706,9 @@
 CurrentCategory = Includes[Index].Category;
   }
 
+  if (Cursor && *Cursor >= IncludesEndOffset)
+*Cursor += result.size() - IncludesBlockSize;
+
   // If the #includes are out of order, we generate a single replacement fixing
   // the entire range of blocks. Otherwise, no replacement is generated.
   if (replaceCRLF(result) == replaceCRLF(std::string(Code.substr(


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -890,6 +890,17 @@
   EXPECT_EQ(10u, newCursor(Code, 43));
 }
 
+TEST_F(SortIncludesTest, CalculatesCorrectCursorPositionWithRegrouping) {
+  Style.IncludeBlocks = Style.IBS_Regroup;
+  std::string Code = "#include \"b\"\n"  // Start of line: 0
+ "\n"// Start of line: 13
+ "#include \"aa\"\n" // Start of line: 14
+ "f();\n";   // Start of line: 28
+  EXPECT_EQ(12u, newCursor(Code, 26));
+  EXPECT_EQ(26u, newCursor(Code, 27));
+  EXPECT_EQ(27u, newCursor(Code, 28));
+}
+
 TEST_F(SortIncludesTest, DeduplicateIncludes) {
   EXPECT_EQ("#include \n"
 "#include \n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2638,10 +2638,10 @@
 StringRef Code, tooling::Replacements ,
 unsigned *Cursor) {
   tooling::IncludeCategoryManager Categories(Style.IncludeStyle, FileName);
-  unsigned IncludesBeginOffset = Includes.front().Offset;
-  unsigned IncludesEndOffset =
+  const unsigned IncludesBeginOffset = Includes.front().Offset;
+  const unsigned IncludesEndOffset =
   Includes.back().Offset + Includes.back().Text.size();
-  unsigned IncludesBlockSize = IncludesEndOffset - IncludesBeginOffset;
+  const unsigned IncludesBlockSize = IncludesEndOffset - IncludesBeginOffset;
   if (!affectsRange(Ranges,