MyDeveloperDay updated this revision to Diff 459408.
MyDeveloperDay added a comment.
- address review comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133589/new/
https://reviews.llvm.org/D133589
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTestJson.cpp
Index: clang/unittests/Format/FormatTestJson.cpp
===================================================================
--- clang/unittests/Format/FormatTestJson.cpp
+++ clang/unittests/Format/FormatTestJson.cpp
@@ -159,6 +159,27 @@
"]");
}
+TEST_F(FormatTestJson, JsonArrayOneLine) {
+ FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+ Style.BreakArrays = false;
+ Style.SpacesInContainerLiterals = false;
+ verifyFormat("[]", Style);
+ verifyFormat("[1]", Style);
+ verifyFormat("[1, 2]", Style);
+ verifyFormat("[1, 2, 3]", Style);
+ verifyFormat("[1, 2, 3, 4]", Style);
+ verifyFormat("[1, 2, 3, 4, 5]", Style);
+
+ verifyFormat("[\n"
+ " 1,\n"
+ " 2,\n"
+ " {\n"
+ " A: 1\n"
+ " }\n"
+ "]",
+ Style);
+}
+
TEST_F(FormatTestJson, JsonNoStringSplit) {
FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
Style.IndentWidth = 4;
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4399,18 +4399,22 @@
// }
if (Left.is(TT_DictLiteral) && Left.is(tok::l_brace))
return true;
- // Always break after a JSON array opener.
- // [
- // ]
+ // Always break after a JSON array opener based on BreakArrays.
if (Left.is(TT_ArrayInitializerLSquare) && Left.is(tok::l_square) &&
- !Right.is(tok::r_square)) {
- return true;
+ Right.isNot(tok::r_square) ||
+ Left.is(tok::comma)) {
+ if (Right.is(tok::l_brace))
+ return true;
+ // scan to the right if an we see an object or an array inside
+ // then break.
+ for (const auto *Tok = &Right; Tok; Tok = Tok->Next) {
+ if (Tok->isOneOf(tok::l_brace, tok::l_square))
+ return true;
+ if (Tok->isOneOf(tok::r_brace, tok::r_square))
+ break;
+ }
+ return Style.BreakArrays;
}
- // Always break after successive entries.
- // 1,
- // 2
- if (Left.is(tok::comma))
- return true;
}
// If the last token before a '}', ']', or ')' is a comma or a trailing
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -706,9 +706,8 @@
// BreakInheritance was not, initialize the latter from the
// former for backwards compatibility.
if (BreakBeforeInheritanceComma &&
- Style.BreakInheritanceList == FormatStyle::BILS_BeforeColon) {
+ Style.BreakInheritanceList == FormatStyle::BILS_BeforeColon)
Style.BreakInheritanceList = FormatStyle::BILS_BeforeComma;
- }
IO.mapOptional("BreakBeforeTernaryOperators",
Style.BreakBeforeTernaryOperators);
@@ -722,12 +721,12 @@
// BreakConstructorInitializers was not, initialize the latter from the
// former for backwards compatibility.
if (BreakConstructorInitializersBeforeComma &&
- Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeColon) {
+ Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeColon)
Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma;
- }
IO.mapOptional("BreakAfterJavaFieldAnnotations",
Style.BreakAfterJavaFieldAnnotations);
+ IO.mapOptional("BreakArrays", Style.BreakArrays);
IO.mapOptional("BreakStringLiterals", Style.BreakStringLiterals);
IO.mapOptional("ColumnLimit", Style.ColumnLimit);
IO.mapOptional("CommentPragmas", Style.CommentPragmas);
@@ -755,7 +754,6 @@
Style.EmptyLineBeforeAccessModifier);
IO.mapOptional("ExperimentalAutoDetectBinPacking",
Style.ExperimentalAutoDetectBinPacking);
-
IO.mapOptional("PackConstructorInitializers",
Style.PackConstructorInitializers);
// For backward compatibility:
@@ -1249,6 +1247,7 @@
LLVMStyle.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never;
LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
LLVMStyle.ExperimentalAutoDetectBinPacking = false;
+ LLVMStyle.BreakArrays = true;
LLVMStyle.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
LLVMStyle.FixNamespaceComments = true;
LLVMStyle.ForEachMacros.push_back("foreach");
@@ -1963,9 +1962,8 @@
(Style.JavaScriptQuotes == FormatStyle::JSQS_Single &&
!Input.startswith("\"")) ||
(Style.JavaScriptQuotes == FormatStyle::JSQS_Double &&
- !Input.startswith("\'"))) {
+ !Input.startswith("\'")))
continue;
- }
// Change start and end quote.
bool IsSingle = Style.JavaScriptQuotes == FormatStyle::JSQS_Single;
@@ -2072,9 +2070,8 @@
if (Tok->is(tok::coloncolon) && Tok->Previous->is(TT_TemplateOpener))
return true;
if (Tok->is(TT_TemplateCloser) &&
- Tok->Previous->is(TT_TemplateCloser)) {
+ Tok->Previous->is(TT_TemplateCloser))
return true;
- }
}
}
}
@@ -2094,9 +2091,8 @@
if (const auto *Func =
Prev->MatchingParen->getPreviousNonComment()) {
if (Func->isOneOf(TT_FunctionDeclarationName, TT_StartOfName,
- TT_OverloadedOperator)) {
+ TT_OverloadedOperator))
continue;
- }
}
}
}
@@ -2191,9 +2187,8 @@
continue;
if (!(FormatTok->is(tok::r_square) &&
Matching->is(TT_ArrayInitializerLSquare)) &&
- !(FormatTok->is(tok::r_brace) && Matching->is(TT_DictLiteral))) {
+ !(FormatTok->is(tok::r_brace) && Matching->is(TT_DictLiteral)))
continue;
- }
FormatToken *Prev = FormatTok->getPreviousNonComment();
if (Prev->is(tok::comma) || Prev->is(tok::semi))
continue;
@@ -2313,9 +2308,8 @@
if (AnnotatedLines[CurrentLine]->startsWithNamespace()) {
if (!checkEmptyNamespace(AnnotatedLines, CurrentLine, NewLine,
- DeletedLines)) {
+ DeletedLines))
return false;
- }
CurrentLine = NewLine;
continue;
}
@@ -2336,9 +2330,8 @@
// Check if the empty namespace is actually affected by changed ranges.
if (!AffectedRangeMgr.affectsCharSourceRange(CharSourceRange::getCharRange(
AnnotatedLines[InitLine]->First->Tok.getLocation(),
- AnnotatedLines[CurrentLine]->Last->Tok.getEndLoc()))) {
+ AnnotatedLines[CurrentLine]->Last->Tok.getEndLoc())))
return false;
- }
for (unsigned i = InitLine; i <= CurrentLine; ++i)
DeletedLines.insert(i);
@@ -2354,12 +2347,10 @@
void cleanupPair(FormatToken *Start, LeftKind LK, RightKind RK,
bool DeleteLeft) {
auto NextNotDeleted = [this](const FormatToken &Tok) -> FormatToken * {
- for (auto *Res = Tok.Next; Res; Res = Res->Next) {
+ for (auto *Res = Tok.Next; Res; Res = Res->Next)
if (!Res->is(tok::comment) &&
- DeletedTokens.find(Res) == DeletedTokens.end()) {
+ DeletedTokens.find(Res) == DeletedTokens.end())
return Res;
- }
- }
return nullptr;
};
for (auto *Left = Start; Left;) {
@@ -2542,9 +2533,8 @@
for (auto *Line : AnnotatedLines) {
if (Line->First && (Line->First->TokenText.startswith("#") ||
Line->First->TokenText == "__pragma" ||
- Line->First->TokenText == "_Pragma")) {
+ Line->First->TokenText == "_Pragma"))
continue;
- }
for (const FormatToken *FormatTok = Line->First; FormatTok;
FormatTok = FormatTok->Next) {
if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
@@ -2600,12 +2590,10 @@
// Determines whether 'Ranges' intersects with ('Start', 'End').
static bool affectsRange(ArrayRef<tooling::Range> Ranges, unsigned Start,
unsigned End) {
- for (auto Range : Ranges) {
+ for (auto Range : Ranges)
if (Range.getOffset() < End &&
- Range.getOffset() + Range.getLength() > Start) {
+ Range.getOffset() + Range.getLength() > Start)
return true;
- }
- }
return false;
}
@@ -2724,9 +2712,8 @@
// 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) {
+ Style.IncludeStyle.IncludeBlocks == tooling::IncludeStyle::IBS_Preserve)
return;
- }
std::string result;
for (unsigned Index : Indices) {
@@ -2734,9 +2721,8 @@
result += "\n";
if (Style.IncludeStyle.IncludeBlocks ==
tooling::IncludeStyle::IBS_Regroup &&
- CurrentCategory != Includes[Index].Category) {
+ CurrentCategory != Includes[Index].Category)
result += "\n";
- }
}
result += Includes[Index].Text;
if (Cursor && CursorIndex == Index)
@@ -2750,9 +2736,8 @@
// 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(
- IncludesBeginOffset, IncludesBlockSize)))) {
+ IncludesBeginOffset, IncludesBlockSize))))
return;
- }
auto Err = Replaces.add(tooling::Replacement(
FileName, Includes.front().Offset, IncludesBlockSize, result));
@@ -2821,13 +2806,11 @@
if (Trimmed.contains(RawStringTermination))
FormattingOff = false;
- if (Trimmed == "// clang-format off" ||
- Trimmed == "/* clang-format off */") {
+ if (Trimmed == "// clang-format off" || Trimmed == "/* clang-format off */")
FormattingOff = true;
- } else if (Trimmed == "// clang-format on" ||
- Trimmed == "/* clang-format on */") {
+ else if (Trimmed == "// clang-format on" ||
+ Trimmed == "/* clang-format on */")
FormattingOff = false;
- }
const bool EmptyLineSkipped =
Trimmed.empty() &&
@@ -2948,9 +2931,8 @@
if (!result.empty()) {
result += "\n";
if (CurrentIsStatic != Imports[Index].IsStatic ||
- CurrentImportGroup != JavaImportGroups[Index]) {
+ CurrentImportGroup != JavaImportGroups[Index])
result += "\n";
- }
}
for (StringRef CommentLine : Imports[Index].AssociatedCommentLines) {
result += CommentLine;
@@ -2964,9 +2946,8 @@
// If the imports are out of order, we generate a single replacement fixing
// the entire block. Otherwise, no replacement is generated.
if (replaceCRLF(result) == replaceCRLF(std::string(Code.substr(
- Imports.front().Offset, ImportsBlockSize)))) {
+ Imports.front().Offset, ImportsBlockSize))))
return;
- }
auto Err = Replaces.add(tooling::Replacement(FileName, Imports.front().Offset,
ImportsBlockSize, result));
@@ -3055,9 +3036,8 @@
if (isLikelyXml(Code))
return Replaces;
if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript &&
- isMpegTS(Code)) {
+ isMpegTS(Code))
return Replaces;
- }
if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript)
return sortJavaScriptImports(Style, Code, Ranges, FileName);
if (Style.Language == FormatStyle::LanguageKind::LK_Java)
@@ -3463,21 +3443,18 @@
return FormatStyle::LK_Java;
if (FileName.endswith_insensitive(".js") ||
FileName.endswith_insensitive(".mjs") ||
- FileName.endswith_insensitive(".ts")) {
+ FileName.endswith_insensitive(".ts"))
return FormatStyle::LK_JavaScript; // (module) JavaScript or TypeScript.
- }
if (FileName.endswith(".m") || FileName.endswith(".mm"))
return FormatStyle::LK_ObjC;
if (FileName.endswith_insensitive(".proto") ||
- FileName.endswith_insensitive(".protodevel")) {
+ FileName.endswith_insensitive(".protodevel"))
return FormatStyle::LK_Proto;
- }
if (FileName.endswith_insensitive(".textpb") ||
FileName.endswith_insensitive(".pb.txt") ||
FileName.endswith_insensitive(".textproto") ||
- FileName.endswith_insensitive(".asciipb")) {
+ FileName.endswith_insensitive(".asciipb"))
return FormatStyle::LK_TextProto;
- }
if (FileName.endswith_insensitive(".td"))
return FormatStyle::LK_TableGen;
if (FileName.endswith_insensitive(".cs"))
@@ -3487,9 +3464,8 @@
if (FileName.endswith_insensitive(".sv") ||
FileName.endswith_insensitive(".svh") ||
FileName.endswith_insensitive(".v") ||
- FileName.endswith_insensitive(".vh")) {
+ FileName.endswith_insensitive(".vh"))
return FormatStyle::LK_Verilog;
- }
return FormatStyle::LK_Cpp;
}
@@ -3548,9 +3524,8 @@
StringRef Source = "<command-line>";
if (std::error_code ec =
parseConfiguration(llvm::MemoryBufferRef(StyleName, Source), &Style,
- AllowUnknownOptions)) {
+ AllowUnknownOptions))
return make_string_error("Error parsing -style: " + ec.message());
- }
if (Style.InheritsParentConfig) {
ChildFormatTextToApply.emplace_back(
llvm::MemoryBuffer::getMemBuffer(StyleName, Source, false));
@@ -3622,9 +3597,8 @@
auto Status = FS->status(Directory);
if (!Status ||
- Status->getType() != llvm::sys::fs::file_type::directory_file) {
+ Status->getType() != llvm::sys::fs::file_type::directory_file)
continue;
- }
for (const auto &F : FilesToLookFor) {
SmallString<128> ConfigFile(Directory);
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -872,6 +872,23 @@
/// \version 3.7
bool BinPackParameters;
+ /// If ``true``, clang-format will always break after a Json array `[`
+ /// otherwise it will scan until the closing `]` to determine if it should add
+ /// newlines between elements (prettier compatible).
+ ///
+ /// NOTE: This is currently only for formatting JSON.
+ /// \code
+ /// true: false:
+ /// [ vs. [1, 2, 3, 4]
+ /// 1,
+ /// 2,
+ /// 3,
+ /// 4
+ /// ]
+ /// \endcode
+ /// \version 16
+ bool BreakArrays;
+
/// The style of wrapping parameters on the same line (bin-packed) or
/// on one line each.
enum BinPackStyle : int8_t {
@@ -3878,6 +3895,7 @@
AttributeMacros == R.AttributeMacros &&
BinPackArguments == R.BinPackArguments &&
BinPackParameters == R.BinPackParameters &&
+ BreakArrays == R.BreakArrays &&
BreakBeforeBinaryOperators == R.BreakBeforeBinaryOperators &&
BreakBeforeBraces == R.BreakBeforeBraces &&
BreakBeforeConceptDeclarations == R.BreakBeforeConceptDeclarations &&
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1698,6 +1698,23 @@
@Mock
DataLoad loader;
+**BreakArrays** (``Boolean``) :versionbadge:`clang-format 16`
+ If ``true``, clang-format will always break after a Json array `[`
+ otherwise it will scan until the closing `]` to determine if it should add
+ newlines between elements (prettier compatible).
+
+ NOTE: This is currently only for formatting JSON.
+
+ .. code-block:: c++
+
+ true: false:
+ [ vs. [1, 2, 3, 4]
+ 1,
+ 2,
+ 3,
+ 4
+ ]
+
**BreakBeforeBinaryOperators** (``BinaryOperatorStyle``) :versionbadge:`clang-format 3.6`
The way to wrap binary operators.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
