[clang-format] Add option to remove leading blank lines #91221
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sstwcw
requested review from
owenca,
mydeveloperday,
HazardyKnusperkeks and
rymiel
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-format @llvm/pr-subscribers-clang Author: None (sstwcw) ChangesFull diff: https://github.com/llvm/llvm-project/pull/91221.diff 7 Files Affected:
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index ce9035a2770eec..c81de131f050c1 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -4406,6 +4406,11 @@ the configuration (without a prefix: ``Auto``).
**KeepEmptyLinesAtEOF** (``Boolean``) :versionbadge:`clang-format 17` :ref:`¶ <KeepEmptyLinesAtEOF>`
Keep empty lines (up to ``MaxEmptyLinesToKeep``) at end of file.
+.. _KeepEmptyLinesAtStart:
+
+**KeepEmptyLinesAtStart** (``Boolean``) :versionbadge:`clang-format 19` :ref:`¶ <KeepEmptyLinesAtStart>`
+ Keep empty lines (up to ``MaxEmptyLinesToKeep``) at start of file.
+
.. _KeepEmptyLinesAtTheStartOfBlocks:
**KeepEmptyLinesAtTheStartOfBlocks** (``Boolean``) :versionbadge:`clang-format 3.7` :ref:`¶ <KeepEmptyLinesAtTheStartOfBlocks>`
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 8ebdc86b98329c..9a7837b1bac2d2 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -3061,6 +3061,10 @@ struct FormatStyle {
/// \version 17
bool KeepEmptyLinesAtEOF;
+ /// Keep empty lines (up to ``MaxEmptyLinesToKeep``) at start of file.
+ /// \version 19
+ bool KeepEmptyLinesAtStart;
+
/// If true, the empty line at the start of blocks is kept.
/// \code
/// true: false:
@@ -4994,6 +4998,7 @@ struct FormatStyle {
JavaScriptQuotes == R.JavaScriptQuotes &&
JavaScriptWrapImports == R.JavaScriptWrapImports &&
KeepEmptyLinesAtEOF == R.KeepEmptyLinesAtEOF &&
+ KeepEmptyLinesAtStart == R.KeepEmptyLinesAtStart &&
KeepEmptyLinesAtTheStartOfBlocks ==
R.KeepEmptyLinesAtTheStartOfBlocks &&
Language == R.Language &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index ad0e2c3c620c32..33dca7b08f998d 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -208,6 +208,9 @@ RawStringFormatStyleManager::RawStringFormatStyleManager(
LanguageStyle = PredefinedStyle;
}
LanguageStyle->ColumnLimit = CodeStyle.ColumnLimit;
+ // This way the first line of the string does not have to follow the code
+ // before the string.
+ LanguageStyle->KeepEmptyLinesAtStart = true;
for (StringRef Delimiter : RawStringFormat.Delimiters)
DelimiterStyle.insert({Delimiter, *LanguageStyle});
for (StringRef EnclosingFunction : RawStringFormat.EnclosingFunctions)
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index c8d8ec3afbd990..31ffbf46cdd085 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1003,6 +1003,7 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
IO.mapOptional("KeepEmptyLinesAtEOF", Style.KeepEmptyLinesAtEOF);
+ IO.mapOptional("KeepEmptyLinesAtStart", Style.KeepEmptyLinesAtStart);
IO.mapOptional("LambdaBodyIndentation", Style.LambdaBodyIndentation);
IO.mapOptional("LineEnding", Style.LineEnding);
IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
@@ -1513,6 +1514,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
LLVMStyle.JavaScriptWrapImports = true;
LLVMStyle.KeepEmptyLinesAtEOF = false;
+ LLVMStyle.KeepEmptyLinesAtStart = true;
LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
LLVMStyle.LambdaBodyIndentation = FormatStyle::LBI_Signature;
LLVMStyle.Language = Language;
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 4ae54e56331bdc..b8485ae2b91975 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1473,8 +1473,10 @@ static auto computeNewlines(const AnnotatedLine &Line,
Newlines = std::min(Newlines, 1u);
if (Newlines == 0 && !RootToken.IsFirst)
Newlines = 1;
- if (RootToken.IsFirst && !RootToken.HasUnescapedNewline)
+ if (RootToken.IsFirst &&
+ (!Style.KeepEmptyLinesAtStart || !RootToken.HasUnescapedNewline)) {
Newlines = 0;
+ }
// Remove empty lines after "{".
if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine &&
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 8c74ed2d119a3f..004fd750f3e467 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -177,6 +177,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) {
CHECK_PARSE_BOOL(InsertBraces);
CHECK_PARSE_BOOL(InsertNewlineAtEOF);
CHECK_PARSE_BOOL(KeepEmptyLinesAtEOF);
+ CHECK_PARSE_BOOL(KeepEmptyLinesAtStart);
CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index e6f8e4a06515ea..d76d4b7a7858c2 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -45,6 +45,9 @@ TEST_F(FormatTest, FormatsGlobalStatementsAt0) {
verifyFormat("\nint i;", " \n\t \v \f int i;");
verifyFormat("int i;\nint j;", " int i; int j;");
verifyFormat("int i;\nint j;", " int i;\n int j;");
+ auto Style = getLLVMStyle();
+ Style.KeepEmptyLinesAtStart = false;
+ verifyFormat("int i;", " \n\t \v \f int i;", Style);
}
TEST_F(FormatTest, FormatsUnwrappedLinesAtFirstFormat) {
@@ -21898,6 +21901,10 @@ TEST_F(FormatTest, HandlesUTF8BOM) {
verifyFormat("\xef\xbb\xbf");
verifyFormat("\xef\xbb\xbf#include <iostream>");
verifyFormat("\xef\xbb\xbf\n#include <iostream>");
+ auto Style = getLLVMStyle();
+ Style.KeepEmptyLinesAtStart = false;
+ verifyFormat("\xef\xbb\xbf#include <iostream>",
+ "\xef\xbb\xbf\n#include <iostream>", Style);
}
// FIXME: Encode Cyrillic and CJK characters below to appease MS compilers.
|
|
Should I make dropping leading blank lines the default? There are some tests verifying that files that start with newlines should still start with newlines after being formatted. However, they seem to be there to prevent the newlines from moving to wrong places. There is 303baf5 "skip empty lines and comments in the top of the code when inserting new headers". std::string Code = "\nint x;";
std::string Expected = "\n#include \"fix.h\"\n"
"#include \"a.h\"\n"
"#include \"b.h\"\n"
"#include \"c.h\"\n"
"#include <list>\n"
"#include <vector>\n"
"int x;";
tooling::Replacements Replaces = toReplacements(
{createInsertion("#include \"a.h\""), createInsertion("#include \"c.h\""),
createInsertion("#include \"b.h\""),
createInsertion("#include <vector>"), createInsertion("#include <list>"),
createInsertion("#include \"fix.h\"")});
EXPECT_EQ(Expected, formatAndApply(Code, Replaces));There is also a test for preserving the byte order mark. verifyFormat("\xef\xbb\xbf\n#include <iostream>"); |
HazardyKnusperkeks
approved these changes
May 9, 2024
mydeveloperday
approved these changes
May 9, 2024
|
I think this is a good time to combine the |
We should keep leading blank lines (up to |
|
Please refer to e3eca33 for grouping/deprecating existing options with a new one. |
|
I was initially going to make removing leading blank lines the default.
Now the line is not needed anymore. But I forgot to remove it.
|
owenca
approved these changes
Jun 18, 2024
| @@ -45,6 +45,9 @@ TEST_F(FormatTest, FormatsGlobalStatementsAt0) { | |||
| verifyFormat("\nint i;", " \n\t \v \f int i;"); | |||
| verifyFormat("int i;\nint j;", " int i; int j;"); | |||
| verifyFormat("int i;\nint j;", " int i;\n int j;"); | |||
| auto Style = getLLVMStyle(); | |||
There was a problem hiding this comment.
An empty line before auto Style = ...?
| @@ -21947,6 +21950,10 @@ TEST_F(FormatTest, HandlesUTF8BOM) { | |||
| verifyFormat("\xef\xbb\xbf"); | |||
| verifyFormat("\xef\xbb\xbf#include <iostream>"); | |||
| verifyFormat("\xef\xbb\xbf\n#include <iostream>"); | |||
| auto Style = getLLVMStyle(); | |||
The options regarding which blank lines are kept are also aggregated. The new option is `KeepEmptyLines`.
sstwcw
force-pushed
the
format-leading-blank
branch
from
dfa556a to
9267f8f
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/1025 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/556 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/7/builds/497 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/837 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/76/builds/356 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/799 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/63/builds/184 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/260 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/428 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/623 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/39/builds/195 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/23/builds/331 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/77/builds/352 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/108/builds/408 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/13/builds/272 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/42/builds/132 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/136/builds/115 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/41/builds/269 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/566 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/81/builds/253 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/383 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/540 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/135/builds/203 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/79/builds/203 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/25/builds/316 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/164/builds/394 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/317 Here is the relevant piece of the build log for the reference: |
|
@sstwcw Please don't ignore the presubmit, it detected issue before merging: Some checks were not successful |
sstwcw
added a commit
that referenced
this pull request
Jun 25, 2024
…)" This reverts commit 9267f8f. I changed a formatter option. I forgot to update other components that depend on the formatter when the option name changed.
sstwcw
added a commit
that referenced
this pull request
Jun 30, 2024
The options regarding which blank lines are kept are also aggregated. The new option is `KeepEmptyLines`. This patch was initially part of 9267f8f. I neglected to check the server builds before I added it. It broke clangd. Jie Fu fixed the problem in 4c91b49. I was unaware of it. I thought the main branch was still broken. I reverted the first patch in 70cfece. It broke his fix. He reverted it in c69ea04. Now the feature is added again including the fix.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/676 Here is the relevant piece of the build log for the reference: |
lravenclaw
pushed a commit
to lravenclaw/llvm-project
that referenced
this pull request
Jul 3, 2024
The options regarding which blank lines are kept are also aggregated. The new option is `KeepEmptyLines`. This patch was initially part of 9267f8f. I neglected to check the server builds before I added it. It broke clangd. Jie Fu fixed the problem in 4c91b49. I was unaware of it. I thought the main branch was still broken. I reverted the first patch in 70cfece. It broke his fix. He reverted it in c69ea04. Now the feature is added again including the fix.
AlexisPerry
pushed a commit
to llvm-project-tlp/llvm-project
that referenced
this pull request
Jul 9, 2024
The options regarding which blank lines are kept are also aggregated. The new option is `KeepEmptyLines`.
AlexisPerry
pushed a commit
to llvm-project-tlp/llvm-project
that referenced
this pull request
Jul 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.