[clang-tools-extra] 9325e97 - [clangd] Handle tabs in getIncrementalChangesAfterNewline()
Nathan Ridge via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 28 22:43:23 PDT 2022
Author: Nathan Ridge
Date: 2022-03-29T01:43:09-04:00
New Revision: 9325e97a3599929a52bef3e5c8e403c98f9428ba
URL: https://github.com/llvm/llvm-project/commit/9325e97a3599929a52bef3e5c8e403c98f9428ba
DIFF: https://github.com/llvm/llvm-project/commit/9325e97a3599929a52bef3e5c8e403c98f9428ba.diff
LOG: [clangd] Handle tabs in getIncrementalChangesAfterNewline()
Tabs are not handled by columnWidthUTF8() (they are considered
non-printable) so require additional logic to handle.
Fixes https://github.com/clangd/clangd/issues/1074
Differential Revision: https://reviews.llvm.org/D122554
Added:
Modified:
clang-tools-extra/clangd/Format.cpp
clang-tools-extra/clangd/unittests/FormatTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Format.cpp b/clang-tools-extra/clangd/Format.cpp
index bb492dbdbd3bd..c3e92636d1957 100644
--- a/clang-tools-extra/clangd/Format.cpp
+++ b/clang-tools-extra/clangd/Format.cpp
@@ -116,12 +116,41 @@ struct IncrementalChanges {
std::string CursorPlaceholder;
};
+// The two functions below, columnWidth() and columnWidthWithTabs(), were
+// adapted from similar functions in clang/lib/Format/Encoding.h.
+// FIXME: Move those functions to clang/include/clang/Format.h and reuse them?
+
+// Helper function for columnWidthWithTabs().
+inline unsigned columnWidth(StringRef Text) {
+ int ContentWidth = llvm::sys::unicode::columnWidthUTF8(Text);
+ if (ContentWidth < 0)
+ return Text.size(); // fallback for unprintable characters
+ return ContentWidth;
+}
+
+// Returns the number of columns required to display the \p Text on a terminal
+// with the \p TabWidth.
+inline unsigned columnWidthWithTabs(StringRef Text, unsigned TabWidth) {
+ unsigned TotalWidth = 0;
+ StringRef Tail = Text;
+ for (;;) {
+ StringRef::size_type TabPos = Tail.find('\t');
+ if (TabPos == StringRef::npos)
+ return TotalWidth + columnWidth(Tail);
+ TotalWidth += columnWidth(Tail.substr(0, TabPos));
+ if (TabWidth)
+ TotalWidth += TabWidth - TotalWidth % TabWidth;
+ Tail = Tail.substr(TabPos + 1);
+ }
+}
+
// After a newline:
// - we continue any line-comment that was split
// - we format the old line in addition to the cursor
// - we represent the cursor with a line comment to preserve the newline
IncrementalChanges getIncrementalChangesAfterNewline(llvm::StringRef Code,
- unsigned Cursor) {
+ unsigned Cursor,
+ unsigned TabWidth) {
IncrementalChanges Result;
// Before newline, code looked like:
// leading^trailing
@@ -152,12 +181,12 @@ IncrementalChanges getIncrementalChangesAfterNewline(llvm::StringRef Code,
if (!CommentMarker.empty() &&
(NewLineIsComment || !commentMarker(NextLine).empty() ||
(!TrailingTrim.empty() && !TrailingTrim.startswith("//")))) {
- using llvm::sys::unicode::columnWidthUTF8;
// We indent the new comment to match the previous one.
StringRef PreComment =
Leading.take_front(CommentMarker.data() - Leading.data());
std::string IndentAndComment =
- (std::string(columnWidthUTF8(PreComment), ' ') + CommentMarker + " ")
+ (std::string(columnWidthWithTabs(PreComment, TabWidth), ' ') +
+ CommentMarker + " ")
.str();
cantFail(
Result.Changes.add(replacement(Code, Indentation, IndentAndComment)));
@@ -191,10 +220,11 @@ IncrementalChanges getIncrementalChangesAfterNewline(llvm::StringRef Code,
}
IncrementalChanges getIncrementalChanges(llvm::StringRef Code, unsigned Cursor,
- llvm::StringRef InsertedText) {
+ llvm::StringRef InsertedText,
+ unsigned TabWidth) {
IncrementalChanges Result;
if (InsertedText == "\n")
- return getIncrementalChangesAfterNewline(Code, Cursor);
+ return getIncrementalChangesAfterNewline(Code, Cursor, TabWidth);
Result.CursorPlaceholder = " /**/";
return Result;
@@ -246,8 +276,8 @@ split(const tooling::Replacements &Replacements, unsigned OldCursor,
std::vector<tooling::Replacement>
formatIncremental(llvm::StringRef OriginalCode, unsigned OriginalCursor,
llvm::StringRef InsertedText, format::FormatStyle Style) {
- IncrementalChanges Incremental =
- getIncrementalChanges(OriginalCode, OriginalCursor, InsertedText);
+ IncrementalChanges Incremental = getIncrementalChanges(
+ OriginalCode, OriginalCursor, InsertedText, Style.TabWidth);
// Never *remove* lines in response to pressing enter! This annoys users.
if (InsertedText == "\n") {
Style.MaxEmptyLinesToKeep = 1000;
diff --git a/clang-tools-extra/clangd/unittests/FormatTests.cpp b/clang-tools-extra/clangd/unittests/FormatTests.cpp
index 0eb217eeb5e37..f7384a1bc63c9 100644
--- a/clang-tools-extra/clangd/unittests/FormatTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FormatTests.cpp
@@ -19,13 +19,11 @@ namespace clang {
namespace clangd {
namespace {
-std::string afterTyped(llvm::StringRef CodeWithCursor,
- llvm::StringRef Typed) {
+std::string afterTyped(llvm::StringRef CodeWithCursor, llvm::StringRef Typed,
+ clang::format::FormatStyle Style) {
Annotations Code(CodeWithCursor);
unsigned Cursor = llvm::cantFail(positionToOffset(Code.code(), Code.point()));
- auto Changes =
- formatIncremental(Code.code(), Cursor, Typed,
- format::getGoogleStyle(format::FormatStyle::LK_Cpp));
+ auto Changes = formatIncremental(Code.code(), Cursor, Typed, Style);
tooling::Replacements Merged;
for (const auto& R : Changes)
if (llvm::Error E = Merged.add(R))
@@ -38,11 +36,15 @@ std::string afterTyped(llvm::StringRef CodeWithCursor,
}
// We can't pass raw strings directly to EXPECT_EQ because of gcc bugs.
-void expectAfterNewline(const char *Before, const char *After) {
- EXPECT_EQ(After, afterTyped(Before, "\n")) << Before;
+void expectAfterNewline(const char *Before, const char *After,
+ format::FormatStyle Style = format::getGoogleStyle(
+ format::FormatStyle::LK_Cpp)) {
+ EXPECT_EQ(After, afterTyped(Before, "\n", Style)) << Before;
}
-void expectAfter(const char *Typed, const char *Before, const char *After) {
- EXPECT_EQ(After, afterTyped(Before, Typed)) << Before;
+void expectAfter(const char *Typed, const char *Before, const char *After,
+ format::FormatStyle Style =
+ format::getGoogleStyle(format::FormatStyle::LK_Cpp)) {
+ EXPECT_EQ(After, afterTyped(Before, Typed, Style)) << Before;
}
TEST(FormatIncremental, SplitComment) {
@@ -131,7 +133,7 @@ void foo() {
^
}
)cpp",
- R"cpp(
+ R"cpp(
void foo() {
if (x)
return; // All spelled tokens are accounted for.
@@ -139,6 +141,17 @@ void foo() {
^
}
)cpp");
+
+ // Handle tab character in leading indentation
+ format::FormatStyle TabStyle =
+ format::getGoogleStyle(format::FormatStyle::LK_Cpp);
+ TabStyle.UseTab = format::FormatStyle::UT_Always;
+ TabStyle.TabWidth = 4;
+ TabStyle.IndentWidth = 4;
+ // Do not use raw strings, otherwise '\t' will be interpreted literally.
+ expectAfterNewline("void foo() {\n\t// this comment was\n^split\n}\n",
+ "void foo() {\n\t// this comment was\n\t// ^split\n}\n",
+ TabStyle);
}
TEST(FormatIncremental, Indentation) {
More information about the cfe-commits
mailing list