[clang] [clang-format] Do not update cursor pos if no includes replacement (PR #77456)

Björn Schäpers via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 20 12:03:09 PST 2024


HazardyKnusperkeks wrote:

> Oh, sorry @HazardyKnusperkeks I missed this in your previous comment.
> 
> I've updated the PR.
> 
> As a side note, I tried adding these two tests but they fail, meaning the cursor is still incorrectly computed with CRLF when replacing lines.
> 
> This is due to adding lines between the include groups (with Regroup option), as this new line is added with only the character `\n` (while the line break between include lines is right because the include line itself contains a trailing `\r`). I am not sure how to fix this correctly because I do not want to break things by trimming the include line and I do not think `sortCppIncludes` should manage whitespaces (`WhitespaceManager` seems to be the correct class to handle this?). Maybe the cursor should be updated when replacing these whitespaces?
> 
> ```c++
> TEST_F(
>        SortIncludesTest,
>        CalculatesCorrectCursorPositionWhenNewLineReplacementsWithRegroupingAndCRLF) {
>   Style.IncludeBlocks	  = Style.IBS_Regroup;
>   FmtStyle.LineEnding	  = FormatStyle::LE_CRLF;
>   Style.IncludeCategories = {
>       {"^\"a\"", 0, 0, false}, {"^\"b\"", 1, 1, false}, {".*", 2, 2, false}};
>   std::string Code = "#include \"a\"\r\n" // Start of line: 0
>                      "#include \"b\"\r\n" // Start of line: 14
>                      "#include \"c\"\r\n" // Start of line: 28
>                      "\r\n"               // Start of line: 42
>                      "int i;";            // Start of line: 44
>   std::string Expected = "#include \"a\"\r\n" // Start of line: 0
>                          "\r\n"               // Start of line: 14
>                          "#include \"b\"\r\n" // Start of line: 16
>                          "\r\n"               // Start of line: 30
>                          "#include \"c\"\r\n" // Start of line: 32
>                          "\r\n"               // Start of line: 46
>                          "int i;";            // Start of line: 48
>   EXPECT_EQ(Expected, sort(Code));
>   EXPECT_EQ(0u, newCursor(Code, 0));
>   EXPECT_EQ(15u, newCursor(Code, 14)); // FIXME: should expect 16, caused by \r
>   EXPECT_EQ(30u, newCursor(Code, 28)); // FIXME: should expect 32, caused by \r
>   EXPECT_EQ(44u, newCursor(Code, 42)); // FIXME: should expect 46, caused by \r
>   EXPECT_EQ(46u, newCursor(Code, 44)); // FIXME: should expect 48, caused by \r
> }
> 
> TEST_F(
>        SortIncludesTest,
>        CalculatesCorrectCursorPositionWhenNoNewLineReplacementsWithRegroupingAndCRLF) {
>   Style.IncludeBlocks = Style.IBS_Regroup;
>   FmtStyle.LineEnding = FormatStyle::LE_CRLF;
>   Style.IncludeCategories = {
>       {"^\"a\"", 0, 0, false}, {"^\"b\"", 1, 1, false}, {".*", 2, 2, false}};
>   std::string Code = "#include \"a\"\r\n" // Start of line: 0
>                      "\r\n"               // Start of line: 14
>                      "#include \"c\"\r\n" // Start of line: 16
>                      "\r\n"               // Start of line: 30
>                      "#include \"b\"\r\n" // Start of line: 32
>                      "\r\n"               // Start of line: 46
>                      "int i;";            // Start of line: 48
>   std::string Expected = "#include \"a\"\r\n" // Start of line: 0
>                          "\r\n"               // Start of line: 14
>                          "#include \"b\"\r\n" // Start of line: 16
>                          "\r\n"               // Start of line: 30
>                          "#include \"c\"\r\n" // Start of line: 32
>                          "\r\n"               // Start of line: 46
>                          "int i;";            // Start of line: 48
>   EXPECT_EQ(Expected, sort(Code));
>   EXPECT_EQ(0u, newCursor(Code, 0));
>   EXPECT_EQ(14u, newCursor(Code, 14));
>   EXPECT_EQ(30u, newCursor(Code, 16)); // FIXME: should expect 32, caused by \r
>   EXPECT_EQ(30u, newCursor(Code, 30));
>   EXPECT_EQ(15u, newCursor(Code, 32)); // FIXME: should expect 15, caused by \r
>   EXPECT_EQ(44u, newCursor(Code, 46)); // FIXME: should expect 46, caused by \r
>   EXPECT_EQ(46u, newCursor(Code, 48)); // FIXME: should expect 48, caused by \r
> }
> ```

That's outside my comfortable zone and I'll refer to @owenca or @mydeveloperday.

https://github.com/llvm/llvm-project/pull/77456


More information about the cfe-commits mailing list