[PATCH] D115738: [clang-format] Code following C# Lambda Expressions has wrong formatting

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2007 +if (FormatTok->is(tok::l_brace)) { + if (Style.isCSharp() && Style.BraceWrapping.AfterFunction == true) { +FormatTok->MustBreakBefore = true; Yo

[PATCH] D115738: [clang-format] Code following C# Lambda Expressions has wrong formatting

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2007 +if (FormatTok->is(tok::l_brace)) { + if (Style.isCSharp() && Style.BraceWrapping.AfterFunction == true) { +FormatTok->MustBreakBefore = true; ow

[PATCH] D115769: [clang-format] Remove spurious JSON binding when DisableFormat = true

owenpan added a reviewer: owenpan. owenpan added a comment. Can you add a test case? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115769/new/ https://reviews.llvm.org/D115769 ___ cfe-commits mailing lis

[PATCH] D115738: [clang-format] Fix formatting of the code that follows C# Lambda Expressions

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1870 // C# may break after => if the next character is a newline. if (Style.isCSharp() && Style.BraceWrapping.AfterFunction == true) { // calling `addUnwrappedLi

[PATCH] D115738: [clang-format] Fix formatting of the code that follows C# Lambda Expressions

owenpan added a comment. Thanks for factoring the code. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1872 // calling `addUnwrappedLine()` here causes odd parsing errors. FormatTok->MustBreakBefore = true; } MyDevelop

[PATCH] D115738: [clang-format] Fix formatting of the code that follows C# Lambda Expressions

owenpan accepted this revision. owenpan added a comment. Thanks! LGTM other than the extraneous braces on lines 1859-1861. (See https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements.) Comment at: clang/lib/Form

[PATCH] D115879: [clang-format] extern with new line brace without indentation

owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:3819 + Style.IndentExternBlock = FormatStyle::IEBS_NoIndent; + verifyFormat("extern \"C\"\n{ /*13*/\n}", Style); + verifyFormat("extern \"C\"\n{\n" curdeius wrote: > I'd prefer th

[PATCH] D115879: [clang-format] extern with new line brace without indentation

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1282-1300 if (FormatTok->Tok.is(tok::string_literal)) { nextToken(); if (FormatTok->Tok.is(tok::l_brace)) { +if (Style.BraceWrapping.AfterExternBlock) { + addUn

[PATCH] D115879: [clang-format] extern with new line brace without indentation

owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:3816 + Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock; + Style.BreakBeforeBraces = FormatStyle::BS_Custom; + Style.BraceWrapping.AfterExternBlock = true; This line i

[PATCH] D115879: [clang-format] extern with new line brace without indentation

owenpan accepted this revision. owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1291-1293 +((Style.IndentExternBlock == FormatStyle::IEBS_Indent) || + (Style.BraceWrapping.AfterExternBlock && + Style.IndentE

[PATCH] D115738: [clang-format] Fix formatting of the code that follows C# Lambda Expressions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG163c13fed9f6: [clang-format] Fix formatting of the code that follows C# Lambda Expressions (authored by peterstys, committed by owenpan). Repository

[PATCH] D115879: [clang-format] extern with new line brace without indentation

owenpan accepted this revision. owenpan added a comment. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115879/new/ https://reviews.llvm.org/D115879 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, curdeius, HazardyKnusperkeks, peterstys. owenpan added a project: clang-format. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Move the handling of brace wrap

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1641 case tok::equal: - // Fat arrows (=>) have tok::TokenKind tok::equal but TokenType - // TT_FatArrow. They always start an expression or a child block if - // followed by a

[PATCH] D115910: [doc] Fix regex in ClangFormatStyleOptions for IncludeCategories

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG212e6c99776f: [doc] Fix regex in ClangFormatStyleOptions for IncludeCategories (authored by jhuels, committed by owenpan). Herald added a subscriber:

[PATCH] D14484: [clang-format] Formatting constructor initializer lists by putting them always on different lines

owenpan added a comment. See D108752 . Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14484/new/ https://reviews.llvm.org/D14484 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D90232: [clang-format] Formatting constructor initializer lists by putting them always on different lines (update to D14484)

owenpan added a comment. See D108752 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90232/new/ https://reviews.llvm.org/D90232 ___ cfe-commits mailing list cfe-commits@

[PATCH] D115990: AlignConsecutiveDeclarations not working for 'const' keyword in JavsScript

owenpan added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1839 +PreviousNotConst = PreviousNotConst->getPreviousNonComment(); +} Remove braces. Comment at: clang/lib/Format/TokenAnnotator.cpp:1875 +// cons

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

owenpan updated this revision to Diff 395380. owenpan added a comment. Additional cleanups: - Restrict parsing `=>` to JavaScript and C#. - Rename `tryToParseCSharpLambda` to `tryToParseChildBlock`. - Remove redundant caret `case`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115967/ne

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

owenpan marked an inline comment as done. owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1641 case tok::equal: - // Fat arrows (=>) have tok::TokenKind tok::equal but TokenType - // TT_FatArrow. They always start an expression o

[PATCH] D116007: [clang-format] Fix BreakBeforeBraces: Attach ignored with trailing requires-expression of function.

owenpan accepted this revision. owenpan added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116007/new/ https://reviews.llvm.org/D116007 _

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2005 - } - break; case tok::l_square: MyDeveloperDay wrote: > Is this case not covered by this? I presume not as you didn't remove any unit > tests > > https://gi

[PATCH] D115990: AlignConsecutiveDeclarations not working for 'const' keyword in JavsScript

owenpan added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1875 +// const a = in JavaScript. +if (Style.isJavaScript() && PreviousNotConst->is(tok::kw_const)) + return true; MyDeveloperDay wrote: > owenpan wrote: > > curdeius wrot

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1955-1957 +if (Style.isCSharp() && FormatTok->is(TT_FatArrow) && +tryToParseChildBlock()) + continue; peterstys wrote: > It seems that this block could be removed

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1964-1965 } - if (FormatTok->is(TT_FatArrow)) { -nextToken(); -// Fat arrows can be followed by simple expressions or by child blocks -// in curly braces. -

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

owenpan updated this revision to Diff 395544. owenpan added a comment. Removed more redundant code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115967/new/ https://reviews.llvm.org/D115967 Files: clang/lib/Format/TokenAnnotator.cpp clang/lib/Format/UnwrappedLineParser.cpp clang

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1955-1957 +if (Style.isCSharp() && FormatTok->is(TT_FatArrow) && +tryToParseChildBlock()) + continue; peterstys wrote: > owenpan wrote: > > peterstys wrote: > > >

[PATCH] D115967: [clang-format][NFC] Handle wrapping after => in mustBreakBefore()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb9f6e09b850a: [clang-format][NFC] Handle wrapping after => in mustBreakBefore() (authored by owenpan). Changed prior to commit: https://reviews.ll

[PATCH] D116188: [clang-format] Fix short enums getting wrapped even when denied

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:398-400 + if (Tok && Tok->is(tok::kw_typedef)) +Tok = Tok->getNextNonComment(); + if (Tok && Tok->isOneOf(tok::kw_class, tok::kw_struct)) { Nits: - It's unnec

[PATCH] D116188: [clang-format] Fix short enums getting wrapped even when denied

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8ea64d5585ec: [clang-format] Fix short enums getting wrapped even when denied (authored by yodaldevoid, committed by owenpan). Repository: rG LLVM

[PATCH] D116229: [clang-format] Add option to align pointers in C style casts differently

owenpan added a comment. In D116229#3208507 , @curdeius wrote: > Why this option is useful? Why would someone want to have a different > alignment in casts than in other places? > As such I'm opposed to introducing one more option. Is there any well > e

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks, curdeius, sammccall, aaron.ballman, krasimir. owenpan added a project: clang-format. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. See th

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks, curdeius. owenpan added a project: clang-format. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo http

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

owenpan added a comment. In D116318#3211258 , @curdeius wrote: > Wasn't assert enough? Could you add a test with code that provokes the > problem? This fix is an NFC, so there will be no (user) test case for it. :) The current behavior does not match t

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

owenpan added a comment. In D116318#3211689 , @HazardyKnusperkeks wrote: > I'm against that patch. > >> Don't pay for what you don't use. > > We should not put the size check into every call. That's what you must do now **without **this patch, which wil

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

owenpan added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3419 + if (isa(D)) { vs. if (isa(D)) { + for (auto *A : D.attrs()) { for (auto *A : D.attrs()) + if (shouldProcessAttr(A)) { if (shouldPr

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

owenpan updated this revision to Diff 396493. owenpan added a comment. Addresses review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

owenpan marked 4 inline comments as done. owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2173 +void UnwrappedLineParser::KeepAncestorBraces() { + const int MaxNestingLevels = 2; + const int Size = NestedTooDeep.size(); Hazar

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

owenpan marked 2 inline comments as done. owenpan added a comment. In D116316#3211269 , @curdeius wrote: > Could you have a look at preceding reviews and see if there wasn't a similar > patch before? I conversed

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

owenpan updated this revision to Diff 396573. owenpan added a comment. Fixed a bug and added a test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h c

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

owenpan added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3398 +**RemoveBracesLLVM** (``Boolean``) :versionbadge:`clang-format 14` + Remove optional braces of control statements (``if``, ``else``, ``for``, MyDeveloperDay wrote: > Can we

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2303 + assert(IfRightBrace->MatchingParen == IfLeftBrace); + IfLeftBrace->MatchingParen = nullptr; + IfRightBrace->MatchingParen = nullptr; HazardyKnusperkeks wrote

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:23224 + + EXPECT_EQ("if (a)\n" +" if (b)\n" owenpan wrote: > MyDeveloperDay wrote: > > any reason these can't be verifyFormats? > Did you mean to add the expected part

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

owenpan created this revision. owenpan added reviewers: djasper, klimek, krasimir, MyDeveloperDay, curdeius, HazardyKnusperkeks. owenpan added a project: clang-format. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add a new optio

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:18472 + FormatStyle::PCIS_Never); + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock; HazardyKnusperkeks wrote: > Should there be a test for the

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

owenpan added inline comments. Comment at: clang/lib/Format/Format.cpp:666-677 +if (Style.PackConstructorInitializers == FormatStyle::PCIS_BinPack) { + bool OnCurrentLine = false; + IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine", +

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

owenpan added inline comments. Comment at: clang/lib/Format/Format.cpp:1189 GoogleStyle.DerivePointerAlignment = true; + GoogleStyle.PackConstructorInitializers = FormatStyle::PCIS_NextLine; GoogleStyle.IncludeStyle.IncludeCategories = {{"^", 2, 0, false},

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

owenpan updated this revision to Diff 369061. owenpan added a comment. Added backward compatibility for the default value `PCIS_NextLine` in Google and Chromium styles and aforementioned cleanups. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108752/new/ https://reviews.llvm.org/D10875

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

owenpan added a comment. In D108752#2967040 , @MyDeveloperDay wrote: > This looks good, I like to move towards one style, it was getting too > confusing. I agree! > Did you test this on a large code base at all? No. I don't think it's necessary (as t

[PATCH] D108810: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

owenpan added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3788-3790 +if (Line.First && Line.First->Next && +Line.First->isOneOf(Keywords.kw_internal, tok::kw_public, +tok::kw_private, tok::kw_protected) && -

[PATCH] D108810: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

owenpan added a comment. LGTM. Comment at: clang/lib/Format/TokenAnnotator.cpp:3799 +if (Style.BraceWrapping.AfterEnum) { + if (Line.startsWith(tok::kw_enum) || + Line.startsWith(tok::kw_typedef, tok::kw_enum)) Nit: I think it would be a littl

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

This revision was automatically updated to reflect the committed changes. Closed by commit rG8a780a2f18c5: [clang-format] Group options that pack constructor initializers (authored by owenpan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108752/new

[PATCH] D105099: [clang-format] Add an option to put one constructor initializer per line

owenpan abandoned this revision. owenpan added a comment. In favor of https://reviews.llvm.org/D108752. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105099/new/ https://reviews.llvm.org/D105099 ___ cfe-

[PATCH] D108882: Add backward compatibility tests for PackConstructorInitializers

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks. owenpan added a project: clang-format. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://review

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

owenpan marked an inline comment as done. owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:18472 + FormatStyle::PCIS_Never); + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock; owenpan wrote: >

[PATCH] D108882: Add backward compatibility tests for PackConstructorInitializers

owenpan updated this revision to Diff 369308. owenpan added a comment. Reworked the logic for handling the backward compatibility. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108882/new/ https://reviews.llvm.org/D108882 Files: clang/lib/Format/Format.cpp clang/unittests/Format/Fo

[PATCH] D108882: Add backward compatibility tests for PackConstructorInitializers

owenpan added a comment. In D108882#2971192 , @MyDeveloperDay wrote: > It seems a little convoluted, but OK Will add comments to explain the logic. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108882/new/ https://reviews.llvm.org/D108882 __

[PATCH] D108882: Add backward compatibility tests for PackConstructorInitializers

This revision was automatically updated to reflect the committed changes. Closed by commit rG4b1fde8a2b68: [clang-format] Add PackConstructorInitializers backward compat test (authored by owenpan). Changed prior to commit: https://reviews.llvm.org/D108882?vs=369308&id=369339#toc Repository:

[PATCH] D109582: [clang-format] Restrict the special handling for K&R C to C/C++

owenpan created this revision. owenpan added reviewers: krasimir, MyDeveloperDay, HazardyKnusperkeks. owenpan added a project: clang-format. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Commits 58494c856a15

[PATCH] D109582: [clang-format] Restrict the special handling for K&R C to C/C++

This revision was automatically updated to reflect the committed changes. Closed by commit rG3205dd3d59b3: [clang-format] Restrict the special handling for K&R C to C/C++ (authored by owenpan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109582/new

[PATCH] D109752: [clang-format] Top-level unwrapped lines don't follow a left brace

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks. owenpan added a project: clang-format. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch makes characterizing top-level unwrapped l

[PATCH] D109752: [clang-format] Top-level unwrapped lines don't follow a left brace

owenpan added a comment. Yes, the said invalid code may get reformatted. However, the main purpose of this patch is to fix the efficiency issue as nested function declarators would no longer be checked for possible K&R C function definitions. Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D109752: [clang-format] Top-level unwrapped lines don't follow a left brace

owenpan added a comment. It will prevent them from even being checked in `parseStructuralElement` as `IsTopLevel` is now `false`: case tok::l_paren: { parseParens(); // Break the unwrapped line if a K&R C function definition has a parameter // declaration. if (!IsTopLevel || !S

[PATCH] D109752: [clang-format] Top-level unwrapped lines don't follow a left brace

This revision was automatically updated to reflect the committed changes. Closed by commit rG8025c03f70ee: [clang-format] Top-level unwrapped lines don't follow a left brace (authored by owenpan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109752/

<    8   9   10   11   12   13