[PATCH] D139029: [clang-format] Don't move comments if AlignTrailingComments: Kind: Leave

HazardyKnusperkeks added a comment. In D139029#3961438 , @mairacanal wrote: > In D139029#3961372 , > @HazardyKnusperkeks wrote: > >> Can you please add a test with more than one newline? > > Hi @HazardyKnusperke

[PATCH] D139029: [clang-format] Don't move comments if AlignTrailingComments: Kind: Leave

HazardyKnusperkeks added a comment. In D139029#3961444 , @HazardyKnusperkeks wrote: > In D139029#3961438 , @mairacanal > wrote: > >> In D139029#3961372 , >> @HazardyKnu

[PATCH] D138402: [clang-format] Correctly count a tab's width in a comment

HazardyKnusperkeks updated this revision to Diff 479054. HazardyKnusperkeks marked an inline comment as done. HazardyKnusperkeks added a reviewer: klimek. HazardyKnusperkeks added a comment. Pulled the `verifyFormat` into this change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138402/

[PATCH] D139029: [clang-format] Don't move comments if AlignTrailingComments: Kind: Leave

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. Thanks, nice work. Do you need someone to push it for you? In that case we need a name and an email address for the commit. CHANGES SINCE LAST ACTION https://review

[PATCH] D138378: [clang-format][NFC] Skip unneeded calculations

HazardyKnusperkeks updated this revision to Diff 479189. HazardyKnusperkeks added a comment. Reverted the changes in the switch. We now only have a fast path. On my machine 4 consecutive runs of the clang format unittests, the last three times (as reported by gtest) are: | | Before patch

[PATCH] D139211: [WIP][clang-format] Properly handle the C11 _Generic keyword.

HazardyKnusperkeks added a comment. Looks good to me. And adding a token type is absolutely nothing negative. The more tokens get a type, the better. Comment at: clang/lib/Format/TokenAnnotator.cpp:1630 TemplateArgument, + // C11 _Generic selection + C11Generi

[PATCH] D139257: [clang-format][NFC] Link braces of a block in UnwrappedLineParser

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2714 + +static void resetOptional(FormatToken *LBrace) { + if (!LBrace) This should be named differently. For me reset means assigning false. Repository: rG LLVM G

[PATCH] D138358: [clang-format] Match all block braces

HazardyKnusperkeks abandoned this revision. HazardyKnusperkeks added a comment. In D138358#3968610 , @owenpan wrote: > Please see D139257 for a different > approach. All my hard work... *sniff*. Repository: rG LLVM

[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. I think this is okay. A followup could handle single line lambdas. But I have a personal struggle with them, especially the name `AllowShortLambdasOnASingleLine` no it is not //allow// if turned on it is //force//. A

[PATCH] D144709: [clang-format] Improve QualifierAlignment

HazardyKnusperkeks added a comment. In D144709#4196738 , @AlexanderHederstaf wrote: > In D144709#4193603 , > @MyDeveloperDay wrote: > >> In D144709#4188921 , >> @Alexan

[PATCH] D146042: [clang-format] Fix numerous issues with "LambdaBodyIndentation: OuterScope" option

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:21916 // Lambdas with different indentation styles. + Style = getLLVMStyleWithColumns(60); jp4a50 wrote: > MyDeveloperDay wrote: > > jp4a50 wrote: > > > MyDeveloperD

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:4742-4748 + if (Left.is(tok::l_paren) && Style.AlwaysBreakBeforeFunctionParameters && + !Right.is(tok::r_paren) && Left.Previous) { + const FormatToken &TwoPrevious = *Left.Previo

[PATCH] D146401: [clang-format] Don't squash Verilog escaped identifiers

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Format/FormatTestVerilog.cpp:19 protected: - static std::string format(llvm::StringRef Code, unsigned Offset, -

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:4343 + WhitespaceSensitiveMacros == R.WhitespaceSensitiveMacros && + Macros == R.Macros; } I put a lot of effort into bringing the stuff sorted. And n

[PATCH] D146926: [clang-format] Add option to decorate reflowed block comments

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/BreakableToken.cpp:406 Decoration = "* "; - if (Lines.size() == 1 && !FirstInLine) { + if ((Lines.size() == 1 && !FirstInLine) || !Style.DecorateReflowedComments) { // Comments for which FirstInLine

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

HazardyKnusperkeks added a comment. Looks good to me, not giving my formal approval since the ongoing discussion about the style guide. Comment at: clang/include/clang/Format/Format.h:823 + /// \endcode + /// \version 16.0 + bool AlwaysBreakBeforeFunctionParameters; ---

[PATCH] D146101: [clang-format] Add DesignatedInitializerIndentWidth option.

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:1665-1669 + const auto DesignatedInitializerIndentWidth = + Style.DesignatedInitializerIndentWidth < 0 + ? Style.ContinuationIndentWidth + : Sty

[PATCH] D146995: [clang-format][NFC] Refactor unit tests for "LambdaBodyIndentation: OuterScope" option.

HazardyKnusperkeks added a comment. In D146995#4227532 , @jp4a50 wrote: > If you guys are happy with this, could you please merge it for me? You need to state a name and email for the commit. > Edit: Also could you please advise about the failing CI tes

[PATCH] D146101: [clang-format] Add DesignatedInitializerIndentWidth option.

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:1665-1669 + const auto DesignatedInitializerIndentWidth = + Style.DesignatedInitializerIndentWidth < 0 + ? Style.ContinuationIndentWidth + : Sty

[PATCH] D147111: [clang-format] Add MinDigits suboptions to IntegerLiteralSeparator

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Format/Format.h:4299 InsertNewlineAtEOF == R.InsertNewlineAtEOF && IntegerLiteralSeparator

[PATCH] D143560: clang-format.el: fix warnings

HazardyKnusperkeks added a comment. In D143560#4149733 , @phst wrote: > I'm not super familiar with the process either, but IIRC @sammccall or > @djasper have to merge this. Anyone with commit access can do that. In D143560#4147918

[PATCH] D144537: [clang-format] Don't move qualifiers past pointers-to-member

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. In D144537#4145545 , @MyDeveloperDay wrote: > maybe we should cherry pick into 16? +1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D144709: [clang-format] Improve west to east const

HazardyKnusperkeks added a comment. In D144709#4149737 , @AlexanderHederstaf wrote: > I tried to refactor QualifierAlignmentFixer to handle more types, notably > removing the *,&,&& requirement as e.g. const Foo did not work. To avoid > moving qualifie

[PATCH] D144709: [clang-format] Improve left to right const

HazardyKnusperkeks added a comment. In D144709#4151546 , @AlexanderHederstaf wrote: >> Highlight by me. That is not acceptable, running the output of >> `clang-format` through `clang-format` shall not result in any change. In >> that case it's better t

[PATCH] D144884: [clang-format] Only add pragma continuation indentation for 'omp' clauses

HazardyKnusperkeks added a comment. In D144884#4155739 , @jhuber6 wrote: > In D144884#4155730 , @jdoerfert > wrote: > >> I'm assuming they have tests? > > I could add a test for the comment case in the bug report

[PATCH] D144884: [clang-format] Only add pragma continuation indentation for 'omp' clauses

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:1279 + if (State.Line->InPragmaDirective) { +FormatToken *PragmaType = State.Line->First->Next->Next; +if (PragmaType && PragmaType->TokenText.equals("omp"))

[PATCH] D144709: [clang-format] Improve QualifierAlignment

HazardyKnusperkeks added a comment. In D144709#4160940 , @AlexanderHederstaf wrote: > In D144709#4160820 , > @MyDeveloperDay wrote: > >> As tests currently fail can you set it to "planned changes" until they pas

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

HazardyKnusperkeks added a comment. Please reupload with the complete context. Please add tests. Comment at: clang/include/clang/Format/Format.h:827 + /// \code + /// someFunction( + /// int argument1, That's not a valid declaration (missing return

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

HazardyKnusperkeks added reviewers: rymiel, owenpan, MyDeveloperDay. HazardyKnusperkeks added a comment. Our tests reside in `clang/unittests/Format`, you want to look into the `TokenAnnotatorTests`. In fact **I** can't read the tests you wrote there. And could you please provide an example what

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

HazardyKnusperkeks added a comment. And please add an entry to the `ReleaseNotes.rst`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125171/new/ https://reviews.llvm.org/D125171 ___ cfe-commits mailing l

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTestObjC.cpp:1528 + // This is added to acknowledge the behavior, but it should be improved. + verifyFormat("ATTRIBUTE_MACRO ATTRIBUTE_MACRO(X)\n" + "@interface Foo\n" -

[PATCH] D145344: [clang-format] Don't annotate left brace of class as FunctionLBrace

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. In D145344#4171087 , @MyDeveloperDay wrote: > Thanks for this, LGTM (maybe we can cherry pick this to 16.0) +1 because of the invalid code. Repository: rG LLVM Github Mon

[PATCH] D145435: Choose style (file) from within code for use in IDEs

HazardyKnusperkeks added a comment. I think this would be a great feature. But I also think that it needs more documentation. You also need to make sure, that we don't change that comment, that is `SpacesInLineCommentPrefix` and `ColumnLimit` should have no effect on the comment. ==

[PATCH] D145435: Choose style (file) from within code for use in IDEs

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/Format.cpp:3542 + +// Prefix found, and at start of file or preceded by white space? +if (PrefixPos != StringRef::npos && I understand the reasoning, but the check down below I think d

[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:3393 +break; + } else { +return; don't need `else` afte

[PATCH] D145435: Choose style (file) from within code for use in IDEs

HazardyKnusperkeks added a comment. In D145435#4179662 , @owenpan wrote: > In D145435#4179308 , @bersbersbers > wrote: > >> In D145435#4173875 , @owenpan >> wrote: >> >>

[PATCH] D152975: [clang-format] Allow break after return keyword

HazardyKnusperkeks added a comment. In D152975#4430504 , @gedare wrote: > In D152975#4425932 , > @HazardyKnusperkeks wrote: > >> And please add a remark in the changelog. > > Is there a separate changelog file?

[PATCH] D153205: [clang-format] Add new block type ListInit

HazardyKnusperkeks added a comment. In D153205#4430528 , @owenpan wrote: > It seems to me that there has been a proliferation of new options being > proposed and/or accepted recently. I'd like to remind everyone of the > long-standing policy >

[PATCH] D153208: [clang-format] Add InsertNewlineAtEOF to .clang-format files

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. We also could add a clang-format style, the you wouldn't have to to touch all .clang-format files. ;) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153208/new/

[PATCH] D153228: [clang-format] Fixed bad performance with enabled qualifier fixer.

HazardyKnusperkeks added a comment. I'm shocked, astonished and in awe how small this patch is. Great work and explanation! Comment at: clang/lib/Format/Format.cpp:3475 AnalyzerPass; SmallVector Passes; Just increase here, and add a comment that th

[PATCH] D153208: [clang-format] Add InsertNewlineAtEOF to .clang-format files

HazardyKnusperkeks added a comment. In D153208#4431697 , @owenpan wrote: > In D153208#4431239 , > @HazardyKnusperkeks wrote: > >> We also could add a clang-format style, the you wouldn't have to to touch >> all

[PATCH] D153228: [clang-format] Fixed bad performance with enabled qualifier fixer.

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. Please wait for at least @MyDeveloperDay to say something, as he is the original author of the work. Comment at: clang/lib/Format/Format.cpp:3475

[PATCH] D153205: [clang-format] Add new block type ListInit

HazardyKnusperkeks added a comment. In D153205#4437966 , @MyDeveloperDay wrote: > My only concern is that this changes behaviour without someone making the > conscious decision to make that change, now as much as I can't understand why > someone would

[PATCH] D153228: [clang-format] Fixed bad performance with enabled qualifier fixer.

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/Format.cpp:3476-3477 + + // Regarding the 16: Note that multiple passes are added in + // addQualifierAlignmentFixerPasses(). + SmallVector Passes; owenpan wrote: > This comment is unnecess

[PATCH] D153579: [clang-format] Align consecutive function pointers and references

HazardyKnusperkeks added a comment. Could you please split this into two reviews? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153579/new/ https://reviews.llvm.org/D153579 ___ cfe-commits mailing list c

[PATCH] D153641: [clang-format] Preserve AmpAmpTokenType in nested parentheses

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. In D153641#465 , @rymiel wrote: > @HazardyKnusperkeks I'm not sure why it didn't recurse already, given that > you even do

[PATCH] D153798: [clang-format] Correctly annotate operator free function call

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:316 + // line can't be a declaration. bool OperatorCalledAsMemberFunction = + (Prev->Previous && Nuu wrot

[PATCH] D89918: Fix issue: clang-format result is not consistent if "// clang-format off" is followed by macro definition.

HazardyKnusperkeks added a comment. Is this still an issue? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89918/new/ https://reviews.llvm.org/D89918 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D146101: [clang-format] Add DesignatedInitializerIndentWidth option.

HazardyKnusperkeks added a comment. In D146101#4233535 , @jp4a50 wrote: > So at the risk of adding to the number of decisions we need to come to a > consensus on, I was about to update the KJ style guide to explicitly call out > the difference in indent

[PATCH] D147176: [clang-format] NFC ensure Style operator== remains sorted for ease of editing

HazardyKnusperkeks added a comment. In D147176#4232633 , @MyDeveloperDay wrote: > Honestly these developers are pretty much 'OGs' , I'm happy to tidy up after > them if they don't quite follow our current conventions. Obviously at any > point you can

[PATCH] D121584: [clang-format] Correctly recognize arrays in template parameter list.

HazardyKnusperkeks added a comment. In D121584#4233165 , @carlosgalvezp wrote: > In D121584#3404704 , @curdeius > wrote: > >> Yes, let's revert this. I should have a fix soon though. > > @curdeius Did you manage

[PATCH] D137327: [clang-format] Handle object instansiation in if-statements

HazardyKnusperkeks added a comment. In D137327#4233652 , @MyDeveloperDay wrote: > In D137327#4233551 , @thieta wrote: > >> In D137327#4233290 , >> @MyDeveloperDay wrote:

[PATCH] D146101: [clang-format] Add BracedInitializerIndentWidth option.

HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:949 + /// If unset, ``ContinuationIndentWidth`` is used. + /// \code + /// AlignAfterOpenBracket: AlwaysBreak jp4a50 wrote: > MyDeveloperDay wrote: > > did you check ge

[PATCH] D147327: [clang-format] Add option for having one port per line in Verilog

HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Format/Format.h:4185 + /// For Verilog, put each port on its own line in module instantiations. + ///

[PATCH] D147556: [clang-format] Add BracedInitializerIndentWidth option.

HazardyKnusperkeks added a comment. Um, was this a mistake? What is with D146101 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147556/new/ https://reviews.llvm.org/D147556 ___

[PATCH] D93240: [clang-format] Add SpaceBeforeCaseColon option

HazardyKnusperkeks added a comment. In D93240#4254439 , @sstwcw wrote: > A goto label isn't affected by this option. Is it intentional? Of course, because it's only for `case`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D147895: [clang-format] Handle Verilog assertions and loops

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:844 "};"); - ASSERT_EQ(Tokens.size(), 44u); + ASSERT_EQ(Tokens.si

[PATCH] D147894: [clang-format] SortIncludes documentation: remove contradiction in its description

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. In D147894#4254941 , @MyDeveloperDay wrote: > Sorry I don’t get how this change helps. Removing the option values does make > it clearer IMHO What options? CHANGES SINCE L

[PATCH] D147969: Add InsertBraces to ChromiumStyle

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/Format.cpp:1699 ChromiumStyle.DerivePointerAlignment = false; +ChromiumStyle.InsertBraces = true; if (Language == FormatStyle::LK_ObjC) MyDeveloperDay wrote: > This is an code mo

[PATCH] D147895: [clang-format] Handle Verilog assertions and loops

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:844 "};"); - ASSERT_EQ(Tokens.size(), 44u); + ASSERT_EQ(Tokens.size(), 44u) << Tokens; EXPECT_TOKEN(Tokens[13], tok::kw_requires, TT_RequiresClause);

[PATCH] D146101: [clang-format] Add BracedInitializerIndentWidth option.

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:1665-1669 + const auto DesignatedInitializerIndentWidth = + Style.DesignatedInitializerIndentWidth < 0 + ? Style.C

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

HazardyKnusperkeks added a comment. In D148467#4272173 , @MyDeveloperDay wrote: > I'm not convinced `AfterCSharpProperty` is a good name, open for suggestions I've not enough domain knowledge to name it. Comment at: clang/include/cl

[PATCH] D148473: [clang-format] C# short ternary operator misinterpreted as a CSharpNullable

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/TokenAnnotator.cpp:1338-1342 +if (!Contexts.back().IsExpression && Line.MustBeDeclaration && +!(T

[PATCH] D148484: [clang-format] Correctly format goto labels followed by blocks

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:714 // are in a control flow statements as well as several style flags. -if (Line.First->is(tok::kw_case) || +if (Line.First->is(tok::kw_case) || Line.Last->is(TT_GotoLa

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:1220 +/// \endcode +bool AfterCSharpProperty; }; MyDeveloperDay wrote: > HazardyKnusperkeks wrote: > > Please sort. :) > Are we sure we want THIS to be alphabetic

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. Just a side note, please link to commits, or even better the reviews here when talking about old commits. I found it through the github issue, but I think the other way around is better. I was interested in the commit

[PATCH] D151145: Add disabled unittest reproducing TextProto formatting issue.

HazardyKnusperkeks added reviewers: MyDeveloperDay, owenpan, rymiel, HazardyKnusperkeks. HazardyKnusperkeks added a comment. As stated on Github I don't think we have a bug. Comment at: clang/unittests/Format/FormatTestRawStrings.cpp:79 +)format"; +llvm::MemoryBufferRef Co

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

HazardyKnusperkeks added a comment. In D151047#4363127 , @owenpan wrote: > In D151047#4361205 , @Sedeniono > wrote: > >> Also, should it be merged into the LLVM 16.x branch? > > +1. @MyDeveloperDay and @HazardyKn

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

HazardyKnusperkeks added a comment. In D151047#4369503 , @MyDeveloperDay wrote: > I think we need to extract the context of the test from RenameTests to ensure > we have it covered here. I don't personally normally run the entire LLVM > suite. Dito, a

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

HazardyKnusperkeks added a comment. In D151047#4369742 , @Sedeniono wrote: > To create a new fix, do I understand the guide > correctly that > I basically execute `arc diff --verbatim` and m

[PATCH] D151145: Add disabled unittest reproducing TextProto formatting issue.

HazardyKnusperkeks added a comment. > In D151145#4365580 , > @HazardyKnusperkeks wrote: > >> As stated on Github I don't think we have a bug. > > Are you sure, I'm pretty sure this is a bug? Can you elaborate on the > discussion on github. I wouldn't wan

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseLabels

HazardyKnusperkeks added a comment. When I read the title I thought "Yay, some one is doing what I want and don't find the time.", but (for me) sadly you're not. I'd like to align the colon (and thus the statement behind that). Do you think you can add that option too? Commen

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseLabels

HazardyKnusperkeks added a comment. In D151761#4385571 , @galenelias wrote: > In D151761#4385163 , > @HazardyKnusperkeks wrote: > >> When I read the title I thought "Yay, some one is doing what I want and >> do

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:19217 + + // Verify comments and empty lines break the alignment + verifyFormat("switch (level) {\n" Comment at: clang/unittests/Format/FormatTest.cp

[PATCH] D152051: libclang-cpp: Add external visibility attribute to all classes

HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:4532 /// Returns ``true`` if the Style has been set. -bool getPredefinedStyle(StringRef Name, FormatStyle::LanguageKind Language, +LLVM_EXTERNAL_VISIBILITY bool getPredefinedStyle(StringRef

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. Please wait for some other approval (or comments) for a few days. Comment at: clang/unittests/Format/FormatTest.cpp:19218 + // Verify comments and em

[PATCH] D143546: [clang-format] Insert a space between a numeric UDL and a dot

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3884 if (Style.isCpp()) { +if (Right.is(tok::period) && Left.is(tok::numeric_constant)) + return true; Add a comment w

[PATCH] D143546: [clang-format] Insert a space between a numeric UDL and a dot

HazardyKnusperkeks added a comment. In D143546#4114341 , @owenpan wrote: > In D143546#4113721 , @rymiel wrote: > >> In D143546#4112077 , @owenpan >> wrote: >> >>> As this

[PATCH] D143691: Fix clang-formats IncludeCategory to match the documentation

HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/Format.cpp:1379 LLVMStyle.IncludeStyle.IncludeCategories = { - {"^\"(llvm|llvm-c|clang|clang-c)/",

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added a comment. This revision now requires changes to proceed. I can see that this is maybe useful, but that have to be behind a new option, which is turned off by default. And a big no to changing the existing tests, you

[PATCH] D143691: Fix clang-formats IncludeCategory to match the documentation

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/Format.cpp:1379 LLVMStyle.IncludeStyle.IncludeCategories = { - {"^\"(llvm|llvm-c|clang|clang-c)/", 2, 0, false}, - {"^(<|\"(gtest|gmock|isl|json)/)", 3, 0, false}, - {".*", 1, 0, false}}; +

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

HazardyKnusperkeks added a comment. In D143870#4124057 , @Febbe wrote: > @HazardyKnusperkeks thank you for the review, I would add another option, but > I don't know a good name. I would propose a > > `boolean` `IncludeDeduplicateInAllBlocks` which defau

[PATCH] D143691: Fix clang-formats IncludeCategory to match the documentation

HazardyKnusperkeks added a comment. Looks good to me, apart from that one issue. Comment at: clang/lib/Tooling/Inclusions/IncludeStyle.cpp:23 + +constexpr int DefaultPriority = std::numeric_limits::max(); + Please move this into the `mapping` function. ==

[PATCH] D143691: Fix clang-formats IncludeCategory to match the documentation

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/SortIncludesTest.cpp:548 + EXPECT_EQ("#include \"a.h\"\n" +"#include \"llvm/a.h\"\n" +"#include \"b.h\"\n" Febbe wrote: > HazardyKnusperkeks wrote: > > While I m

[PATCH] D144296: [clang-format] Rewrite how indent is reduced for compacted namespaces

HazardyKnusperkeks added a comment. There is no PP stuff in the new tests. And I see the style was bogus before, but shouldn't all variables start with a capital letter? Please fix that in the code you touch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D143825: [clang-format] Put ports on separate lines in Verilog module headers

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2665 +if (Style.isVerilog() && Precedence == prec::Comma && +VerilogFirstOfType != nullptr) { + addFakeParenthesis(VerilogFirstOfType, prec::Comma); owenpan

[PATCH] D150057: [clang-format] Fix consecutive alignments in #else blocks

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. I have to say, I don't understand it, but I believe you. Why is continuing, when the token is finalized the right thing? Comment at: clang/unittests/Format/FormatTestComments.cpp:4319 /\ -/ +/

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:4514-4515 +// Apply this logic for parens that are not function attribute macros. +if ((Left.is(tok::r_paren) && Left.isNot(TT_AttributeParen)) && +canBeObjCSelectorComponent

[PATCH] D150057: [clang-format] Fix consecutive alignments in #else blocks

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:6371-6380 + verifyFormat("#if FOO\n" + "int a = 1;\n" + "#else\n" + "int ab = 2;\n" + "#endif\n" + "#ifdef BAR\n" +

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:5473 + if (Right.isOneOf(tok::kw___attribute, TT_AttributeMacro)) +return true; + jaredgrubb wrote: > HazardyKnusperkeks wrote: > > Does changing this return value make

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

HazardyKnusperkeks added a comment. LGTM, but someone other has to approve. Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:43-44 +struct ObjCPropertyEntry { + StringRef attribute; // eg, "readwrite" + StringRef value; // eg, the "foo" of the attribute "g

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

HazardyKnusperkeks added a comment. Shouldn't it have token annotator tests? Comment at: clang/lib/Format/UnwrappedLineParser.cpp:495-496 SmallVector LBraceStack; + // Track the previous token type corresponding to our lbraces // to help + // detect brace types + SmallVec

[PATCH] D150452: [clang-format] Recognize nested blocks

HazardyKnusperkeks added a comment. On this change I like the token annotator tests, and reusing the same stack instead of adding a second one which always has the same size. But the code changes in D150403 seem to be better, at least I couldn't think directly

[PATCH] D149562: [clang-format] Stop comment disrupting indentation of Verilog ports

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. In D149562#4337955 , @sstwcw wrote: >> I'll go along with other reviewers on this one. > > So what do the other reviewers think

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

HazardyKnusperkeks added a comment. Please add the full stop to the code comment(s), and then mark review comments as done. You should also give credit to @sstwcw in the commit message/differential description for the token annotator tests. In D150403#4339530

[PATCH] D150539: [clang-format] Handle ud suffixes in IntegerLiteralSeparator

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/IntegerLiteralSeparatorFixer.cpp:116-117 if (Style.isCpp()) { - if (const auto Pos = Text.find_first_of("_i

[PATCH] D150629: [clang-format] Don't allow template to be preceded by closing brace

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:10682 verifyFormat("a < 0 ? b : a > 0 ? c : d;"); + verifyFormat("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;"); verifyFormat("void f() {\n" owenpan wrote: > Do

[PATCH] D150614: [clang-format] Ignore first token when finding MustBreak

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:20136 "#ifdef _DEBUG\n" - "void bar()\n" - " {\n" - " }\n" + "void bar() {}\n" "#else\n" -

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

HazardyKnusperkeks added a comment. In D150403#4343708 , @galenelias wrote: > Thanks @HazardyKnusperkeks! I don't have commit access, so will need someone > to land this for me. We'll wait a bit, if someone might have a comment. And (at least I) need

[PATCH] D150614: [clang-format] Ignore first token when finding MustBreak

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:20136 "#ifdef _DEBUG\n" - "void bar()\n" - " {\n" - " }\n" + "void bar(

<    3   4   5   6   7   8   9   10   11   >