[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