[PATCH] D121758: [clang-format] Add support for formatting Verilog code

MyDeveloperDay added a comment. Firstly thank you for taking the time to submit the patch, I've only taken a cursory view at first and I definitely have some comments, I've added the main clang-format reviewers This is a very invasive patch so there are a number of things we might want to do t

[PATCH] D121758: [clang-format] Add support for formatting Verilog code

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2241 +**BreakBetweenInstancePorts** (``Boolean``) :versionbadge:`clang-format 15` + For Verilog, put each port on its own line in module instantiations. I don't feel "Inst

[PATCH] D121758: [clang-format] Add support for formatting Verilog code

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:1373 + else if (!Current.isOneOf(tok::comment, tok::identifier) && + !Keywords.isPPHash(Current, Style) && !Current.isStringLiteral()) State.StartOfStringLiteral = 0; -

[PATCH] D121758: [clang-format] Add support for formatting Verilog code

MyDeveloperDay added a comment. I'm very much a "clang-format all the things" kind of person which is why I added C# and JSON support, as someone who moves between languages all day every day I want one tool to rule them all.. I'd love it if it would format YAML (but thats another story).. For

[PATCH] D121758: [clang-format] Add support for formatting Verilog code

MyDeveloperDay added a comment. You need to rebase the patch, it doesn't apply. Comment at: clang/lib/Format/Format.cpp:3451 +for (auto Suffix : Lang.second) + if (FileName.endswith(Suffix)) +return Lang.first; krasimir wrote: > I kinda prefer

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

MyDeveloperDay added a comment. Can you add a summary as to why you are making this change? the code doesn't look the same before/after so whats changing and why? is there no unit tests that perhaps need to be added? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/Format.cpp:2762 +// This addresses https://github.com/llvm/llvm-project/issues/38995 +int WithSemic

[PATCH] D121890: [clang-format] Copy help options to the doc directory.

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. 1. Please log a github issue showing us what you are fixing. 2. Please add the main clang-formatter developers as reviewers and not the clang-format project (I think w

[PATCH] D121758: [clang-format] Add support for formatting Verilog code

MyDeveloperDay added a comment. Abandoned? huh? are you breaking it apart? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121758/new/ https://reviews.llvm.org/D121758 ___ cfe-commits mailing list cfe-comm

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

MyDeveloperDay added a comment. Just an observation, I have this in my Day Job all the time.. 1. Developer A develops a piece of code 2. Over the years the developers in our group learn that code and become familiar with it 3. Developer B arrives in our group, telling us we are doing it all wron

[PATCH] D121749: [clang-format][docs] Regenerate ClangFormatStyleOptions.rst

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:361 - * ``bool AlignCompound`` Only for ``AlignConsecutiveAssignments``. Whether compound -assignments like ``+=`` are aligned along with ``=``. + * ``bool AlignCompound`` Only for

[PATCH] D121757: [clang-format] Take out common code for parsing blocks

MyDeveloperDay added a comment. In D121757#3389218 , @sstwcw wrote: > For the new stuff I have the option of still adding the function > `parseIndentedBlock` but only using it in new code. Please be more blunt > about whether I should close this revisi

[PATCH] D121890: [clang-format] Copy help options to the doc directory.

MyDeveloperDay added a comment. In D121890#3389171 , @sstwcw wrote: > The issue is #54418. > > Actually I don't know who I should add as reviewers. The patches for > clang-format don't always seem to have the same reviewers. Who are the main > develop

[PATCH] D121907: [clang-format] Use an enum for context types.

MyDeveloperDay added a comment. So out of interest, what is the reason? my assumption is that you wanted to add more for Verilog and you felt adding the extra bools was the wrong design and its better an an enum bool InCpp11AttributeSpecifier = false; bool InCSharpAttributeSpecifier = false

[PATCH] D121749: [clang-format][docs] Regenerate ClangFormatStyleOptions.rst

MyDeveloperDay added a subscriber: Eugene.Zelenko. MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:361 - * ``bool AlignCompound`` Only for ``AlignConsecutiveAssignments``. Whether compound -assignments like ``+=`` are aligned alon

[PATCH] D121749: [clang-format][docs] Regenerate ClangFormatStyleOptions.rst

MyDeveloperDay added a comment. I’m not blocking it just using the observation to highlight that the rst generated isn’t conformant with best practices. The solution being that a volunteer offers to fix the dump_format_style.py or enduring Format.h isn’t making those mistakes Repository: rG

[PATCH] D121916: [clang-format] [doc] Add script to automatically update help output in ClangFormat.rst.

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Wow! Now that’s what I’m talking about!! Awesome! Thank you so much! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121916/new/ h

[PATCH] D121906: [clang-format] Indent import statements in JavaScript.

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Can you please supply full context diff "Context not available." Comment at: clang/lib/Format/ContinuationIndenter.cpp:629 int PPColumnCorrection

[PATCH] D121753: [clang-format] Use a macro for non-C keywords

MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/FormatToken.h:884 struct AdditionalKeywords { +#define LIST_ADDITIONAL_KEYWORDS

[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. This just made a 300 lines function and a 500 line function with minimal comments into a 800 line function.. For no real benefit? Because from what I can tell you hav

[PATCH] D121756: [clang-format] Clean up code looking for if statements

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Sorry I don't believe we've covered the changes with unit tests. Comment at: clang/lib/Format/FormatToken.h:528 + /// to be handled separately. +

[PATCH] D121754: [clang-format] Refactor determineStarAmpUsage

MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/TokenAnnotator.cpp:2146-2147 +// know how they can be followed by a star or amp. +// co_await, delete - I

[PATCH] D121758: [clang-format] Add support for formatting Verilog code

MyDeveloperDay added a comment. In D121758#3389181 , @sstwcw wrote: > Yes. I am surprised that you asked since everyone asked me to break it apart. Well I was thinking you move the unrelated parts out and then reduce the side of this patch, (but you'll

[PATCH] D121906: [clang-format] Indent import statements in JavaScript.

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:631 + Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash && Previous.is(tok::hash) && State.FirstIndent > 0 && (State.Line->Type == LT_PreprocessorDirective || ---

[PATCH] D121907: [clang-format] Use an enum for context types. NFC

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. I think fundamentally this look ok? @curdeius and @owenpan I feel I want to bow to your better judgement. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D121907: [clang-format] Use an enum for context types. NFC

MyDeveloperDay added a comment. Please wait for the other reviewers if you have commit access. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121907/new/ https://reviews.llvm.org/D121907 ___ cfe-commits m

[PATCH] D121906: [clang-format] Indent import statements in JavaScript.

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:633 (State.Line->Type == LT_PreprocessorDirective || State.Line->Type == LT_ImportStatement)) { Spaces += State.FirstIndent; AfterHash - Could be true

[PATCH] D121907: [clang-format] Use an enum for context types. NFC

MyDeveloperDay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121907/new/ https://reviews.llvm.org/D121907 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D121755: [clang-format] Join spaceRequiredBefore and spaceRequiredBetween

MyDeveloperDay added a comment. I think you can spin it any number of ways, but bottom line its a massive truth table and I sort of feel however you change it, it will just become a different equally incomprehensible pattern. For me I'd prefer to stick with the devil I know, unless its broken,

[PATCH] D121754: [clang-format] Refactor determineStarAmpUsage

MyDeveloperDay added a comment. > But we can: https://gcc.godbolt.org/z/8Mb1xo7oP This is what I love about you all, you push me to learn more about C++, its a slippery br Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121754/new/ https://rev

[PATCH] D121756: [clang-format] Clean up code looking for if statements

MyDeveloperDay added a comment. > It turned out this patch does change behavior. i.e. now your function is considering more things, the formatted code is different, do you think the new formatting is more correct? (I mean it feels like it should be right? assuming we want to consider whiles in

[PATCH] D121756: [clang-format] Clean up code looking for if statements

MyDeveloperDay added a comment. In D121756#3398165 , @sstwcw wrote: > It turned out this patch does change behavior. > > - while ( > - FormatTok->isOneOf(tok::identifier, tok::kw_requires, > tok::coloncolon)) { > + while (FormatTok->isOneOf(

[PATCH] D121756: [clang-format] Clean up code looking for if statements

MyDeveloperDay added a comment. FWIW I'm not a fan of the `while( \n` case, so assuming this fix, fixes that then that would be good I think. - while ( - FormatTok->isOneOf(tok::identifier, tok::kw_requires, tok::coloncolon)) { + while (FormatTok->isOneOf(tok::identifier, tok::kw_r

[PATCH] D122230: [clang-format] don't break up #-style comment sections

MyDeveloperDay added a comment. I hit something like this today with some code which was //#undef I'm not sure I think that // #undef or //# undef is what I want either Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122230/new/ https://r

[PATCH] D121916: [clang-format] [doc] Add script to automatically update help output in ClangFormat.rst.

MyDeveloperDay added inline comments. Comment at: clang/docs/tools/dump_format_help.py:29 +def get_help_output(): +args = ["clang-format", "--help"] +cmd = subprocess.Popen(args, stdout=subprocess.PIPE, curdeius wrote: > sstwcw wrote: > > You intentionall

[PATCH] D121451: [clang-format] Add space to comments starting with '#'.

MyDeveloperDay added a comment. I'm a little uncomfortable with //# becoming // # Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121451/new/ https://reviews.llvm.org/D121451 ___ cfe-commits mailing

[PATCH] D123299: [clang-format][docs] Fix incorrect 'clang-format 9' option marker

MyDeveloperDay accepted this revision. MyDeveloperDay 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/D123299/new/ https://reviews.llvm.org/D123299 ___

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

MyDeveloperDay added a comment. The hard part about doing 2 separate cases is where we miss-interpret a .hpp or .h file as being the wrong language (C/C++/ObjC/ObjC++), I don't think I have a problem with a single regex (its not like we change it that often). Is there a use case for having them

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Accepting but please wait for @owenpan Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/new/ https://reviews.llvm.org/D12137

[PATCH] D123571: [clang-format] Clean up unit tests for AlignArrayOfStructures

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Thanks for taking this on. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123571/new/ https://reviews.llvm.org/D123571 ___ cfe-commit

[PATCH] D123450: [clang-format] Parse Verilog if statements

MyDeveloperDay added a comment. As first passes for adding a new language I think this looks pretty good. ``if (Keywords.isBlockBegin(*FormatTok, Style)) {`` I'm not a massive fan of handling l_brace and begin and end as the same thing, I might be tempted to handle them separately, as then its

[PATCH] D118911: [clang-format] regression from clang-format v13

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, owenpan. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://github.com/llvm/llvm-project/issues/53567 The following source namespace A

[PATCH] D118911: [clang-format] regression from clang-format v13

MyDeveloperDay updated this revision to Diff 405705. MyDeveloperDay added a comment. Simply logic - review comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118911/new/ https://reviews.llvm.org/D118911 Files: clang/lib/Format/UnwrappedLineParser.cpp clang/unittests/Format/Forma

[PATCH] D118911: [clang-format] regression from clang-format v13

MyDeveloperDay updated this revision to Diff 405708. MyDeveloperDay added a comment. De'Morganed!! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118911/new/ https://reviews.llvm.org/D118911 Files: clang/lib/Format/UnwrappedLineParser.cpp clang/unittests/Format/FormatTest.cpp Inde

[PATCH] D118911: [clang-format] regression from clang-format v13

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG23fc20e06c08: [clang-format] regression from clang-format v13 (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D113319: [clang-format] Improve require and concept handling

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. @HazardyKnusperkeks I think this is tremendous, I think this looks and feels like a much more thorough approach to formatting concepts. Thank you so much for taking this on and

[PATCH] D118991: [clang-format][docs] Fix incorrect 'clang-format 14' configuration options markers

MyDeveloperDay added a comment. Thank you for looking into this, I'm more than happy for us to change the values to their more accurate first releases, but can I ask how you went about making those assumptions? For me initially I simply looks at the commit in the blame then tracked that to what

[PATCH] D118991: [clang-format][docs] Fix incorrect 'clang-format 14' configuration options markers

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thank you for this. LGTM (I double-checked a few and they all look good to me!) Comment at: clang/docs/ClangFormatStyleOptions.rst:2212 -**EmptyLineAfterAc

[PATCH] D118991: [clang-format][docs] Fix incorrect 'clang-format 14' configuration options markers

MyDeveloperDay added a comment. Do you need someone to land this for you? if so we need your "name " Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118991/new/ https://reviews.llvm.org/D118991 ___ cfe-co

[PATCH] D119218: [clang-format] Honour "// clang-format off" when using QualifierOrder.

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Perfect thank you Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119218/new/ https://reviews.llvm.org/D119218 __

[PATCH] D119597: [clang-format][NFC] Give State.Stack.back() a meaningful name

MyDeveloperDay added a comment. I like this change for clarity reasons, my only concern and it's not based on evidence is what if any of these functions get passed in State, and then they themselves alter the State.Stack? In most cases I'd expect CurrentState to always be correct, but doesn't i

[PATCH] D119599: Add option to align compound assignments like `+=`

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:163 + /// \endcode + bool AlignCompoundAssignments; + This option is not independent of `AlignConsecutiveAssignments` is it? this will cause confusion when people turn it on

[PATCH] D119599: Add option to align compound assignments like `+=`

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:468 + // Widths of the aligned text. + // The part to the left of the anchor. For right-justified assignment + // operators, this includes the initial space before the sign but not ---

[PATCH] D119599: Add option to align compound assignments like `+=`

MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/WhitespaceManager.cpp:273 AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End, -

[PATCH] D119597: [clang-format][NFC] Give State.Stack.back() a meaningful name

MyDeveloperDay added a comment. @HazardyKnusperkeks agreed, I'm fine with this approach (especially on what is one of the more confusing parts of clang-format) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119597/new/ https://reviews.llvm.org/D119

[PATCH] D119785: [clang-format] Fix formatting of struct-like records followed by variable declaration.

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM, I really like the approach of us annotating more like this, it makes the token much easier to reason about when there is ambiguity. nice one! Comment a

[PATCH] D119682: [clang-format][docs] Fix incorrect 'clang-format 13' configuration options markers

MyDeveloperDay accepted this revision. MyDeveloperDay added a subscriber: HazardyKnusperkeks. MyDeveloperDay added a comment. This revision is now accepted and ready to land. @HazardyKnusperkeks could you validate the `IndentRequiresClause` I know I added `IndentRequires` in 13 but is this the sa

[PATCH] D119682: [clang-format][docs] Fix incorrect 'clang-format 13' configuration options markers

MyDeveloperDay added a comment. I understand what you are saying re 'IndentRequiresClause' but this leaves us with people with "IndentRequires" in their .clang-format without any understanding of what it means? i.e. what about the 14.0 people? if we've renamed an option then the documentation s

[PATCH] D120034: [clang-format] PROPOSAL - WIP: Added ability to parse template arguments

MyDeveloperDay added a comment. A noble effort. It needs someone like yourself to do this as you already have a deep understanding. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120034/new/ https://reviews.llvm.org/D120034 ___

[PATCH] D124748: [clang-format] Fix whitespace counting stuff

MyDeveloperDay added a comment. Is there any way you could add a unit test for this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124748/new/ https://reviews.llvm.org/D124748 ___ cfe-commits mailing lis

[PATCH] D124749: [clang-format] Handle Verilog preprocessor directives

MyDeveloperDay added a comment. You add significant number of keywords here but I don't see any of them being tested? can you add a unit tests to cover what you are adding Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124749/new/ https://reviews.l

[PATCH] D126096: [clang-format] Fix QualifierAlignment with global namespace qualified types.

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Brilliant thank you, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126096/new/ https://reviews.llvm.org/D126096 __

[PATCH] D125848: [clang-format] Handle attributes in enum declaration.

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. This looks good to me, but let the others chime in.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125848/new/ https://reviews.l

[PATCH] D126157: [clang-format][NFC] Insert/remove braces in clang/lib/Format/

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM too.. @owenpan this is one of my favourite features!! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126157/new/ https://reviews.llvm.org/D126157 __

[PATCH] D126132: [clang-format] Fix a crash on lambda trailing return type

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM (sorry I've been slow on the reviews, my day job keeps getting in the way ;-)) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126132/new/ https://reviews.llvm.org/D126132

[PATCH] D125961: [clang-format] Don't break lines after pragma region

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:19604 + + EXPECT_EQ("#pragma region TEST(FOO : BAR)", format("#pragma region TEST(FOO : BAR)", Style)); +} do we need to consider endregion at all? it would be nice to have

[PATCH] D64695: [clang-format] Modified SortIncludes and IncludeCategories to priority for sorting #includes within the Group Category.

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64695/new/ https://reviews.llvm.org/D64695 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D66662: [clang-format] [PR43100] clang-format C# support does not add a space between "using" and paren

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: djasper, klimek, owenpan. MyDeveloperDay added a project: clang. Addresses https://bugs.llvm.org/show_bug.cgi?id=43100 Formatting using statement in C# with clang-format removes the space between using and paren even when Spac

[PATCH] D66662: [clang-format] [PR43100] clang-format C# support does not add a space between "using" and paren

MyDeveloperDay updated this revision to Diff 217167. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D2/new/ https://reviews.llvm.org/D2 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTestCSharp.cpp Index: clang/unittests/Format/FormatTestCSharp.cpp ==

[PATCH] D50147: clang-format: support external styles

MyDeveloperDay added a comment. I am not opposed to this idea, I actually think this has some mileage based on a use case I've encountered: our team tends to use windows builds of clang-format from (https://llvm.org/builds/) because its easy to distribute the installer, this is ok but sometime

[PATCH] D50147: clang-format: support external styles

MyDeveloperDay added a comment. but perhaps I'm really just agreeing to @sammccall 's suggestion of "-style=" which kind of feels sufficient to me.. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50147/new/ https://reviews.llvm.org/D50147

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. This LGTM, just some minor Nits Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1354 Line->Tokens.begin()->Tok->MustBreakBefore = true; -

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

MyDeveloperDay added a comment. I think generally speaking this looks ok, I'm ok with the extra clause, as long as it's clear what it's doing Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:329 : 0; +} else if (I[1]->First->is(tok::l_brace) && +

[PATCH] D72150: Allow space after C-style cast in C# code

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thanks for the patch, really good to have others working on C# support. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72150/

[PATCH] D72144: Treat C# `using` as a control statement

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72144/new/ https://reviews.llvm.org/D72144 ___ cfe-commits mailing li

[PATCH] D71659: [clang-format] Added new option to allow setting spaces before and after the operator

MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3359 + // nuanced as aggressive line breaks are placed when the lambda is not the + // last arg. if ((Style.Language == FormatStyle::LK_Cp

[PATCH] D72276: [clang-format] Add IndentCaseBlocks option

MyDeveloperDay added a comment. Could you add an entry into the clang-format section of the clang release notes Comment at: clang/unittests/Format/FormatTest.cpp:1235 + "}", + Style)); } Could you add a test with indented ca

[PATCH] D72276: [clang-format] Add IndentCaseBlocks option

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:1235 + "}", + Style)); } MyDeveloperDay wrote: > Could you add a test with indented case labels and indented case blocks? what happens if

[PATCH] D72276: [clang-format] Add IndentCaseBlocks option

MyDeveloperDay added a comment. This LGTM, thanks for the patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72276/new/ https://reviews.llvm.org/D72276 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D72401: Fixes for spaces around C# object initializers

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM, thanks once again for the C# patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72401/new/ https://reviews.llvm.org/D724

[PATCH] D72401: Fixes for spaces around C# object initializers

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2870 +if (Left.is(TT_Unknown) && Right.is(tok::l_brace) && Left.Previous && +Left.Previous->is(tok::kw_new)) + return true; krasimir wrote: > The `TT_Unknown` i

[PATCH] D87291: [clang-format][regression][PR47461] ifdef causes catch to be seen as a function

MyDeveloperDay updated this revision to Diff 291452. MyDeveloperDay added a comment. Minimize other places where try could be incorrectly seen as an identifier try // foo { or try /* foo */ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87291/new/ https://reviews.llvm.org/D8729

[PATCH] D87587: [clang-format][PR47290] Make one-line namespaces resistant to FixNamespaceComments, update documentation

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. I have a hard time when people change tests! just because one person wants them this way doesn't mean everyone will. I am not sure about others but I'd be ok about seeing this as an option to override the value b

[PATCH] D87352: Fix typo in doc ClangFormatStyleOptions.rst

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. I have realized this is the wrong change The actual error is in Format.h clang/docs/ClangFormatStyleOption.rst is generated using the clang/docs/tools/dump_format_st

[PATCH] D87352: Fix typo in Format.h

MyDeveloperDay added a comment. Ok almost... so you change the Format.h, then run the docs/tools/clang_format_stype.py and attach both the rst and the .h to the review CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87352/new/ https://reviews.llvm.org/D87352 ___

[PATCH] D87587: [clang-format][PR47290] Make one-line namespaces resistant to FixNamespaceComments, update documentation

MyDeveloperDay added a comment. Firstly let me say thank you for the patch, and taking the time to listen to the reviewers. Please don't be discouraged by my reply/opinon. > a) adding a new option means increasing our maintenance cost by possibly > adding a rarely-used switch (which I also disl

[PATCH] D87352: Fix typo

MyDeveloperDay added a subscriber: arichardson. MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:778 - For example: __capability. - So this is an example of where someone has changed the .rst and not the Format.h! @aric

[PATCH] D87291: [clang-format][regression][PR47461] ifdef causes catch to be seen as a function

MyDeveloperDay added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87291/new/ https://reviews.llvm.org/D87291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87352: Fix typo

This revision was automatically updated to reflect the committed changes. Closed by commit rG4dd9c709ef1b: [clang-format] [NFC] Fix spelling mistake in the documentation (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D8735

[PATCH] D87587: [clang-format][PR47290] Make one-line namespaces resistant to FixNamespaceComments, update documentation

MyDeveloperDay added a comment. In D87587#2276462 , @kuzkry wrote: > Thanks @MyDeveloperDay, I appreciate your input in this review. You're a good > reviewer. :) > Also, I think you've made some good points there and I'm going to turn this > review into

[PATCH] D87291: [clang-format][regression][PR47461] ifdef causes catch to be seen as a function

MyDeveloperDay added a comment. In D87291#2278743 , @curdeius wrote: > LGTM. > Don't we risk any other side effects? There could be, but maybe we can address them as we see them. The original reason for including this was so that the Linux kernel could u

[PATCH] D87291: [clang-format][regression][PR47461] ifdef causes catch to be seen as a function

This revision was automatically updated to reflect the committed changes. Closed by commit rG40e771c1c0d3: [clang-format][regression][PR47461] ifdef causes catch to be seen as a function (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D86137: Add -Wno-error=unknown flag to clang-format.

MyDeveloperDay added a comment. IMHO I feel that given that these changes are more wide reaching than just clang-format that it might be more correct for your to request commit access yourself. You need to own these changes, because if the llvm/Support changes fail the build or unit tests the

[PATCH] D87201: [clang-format] Add a option for the position of Java static import

This revision was automatically updated to reflect the committed changes. Closed by commit rG2e7add812eb7: [clang-format] Add a option for the position of Java static import (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D86137: Add -Wno-error=unknown flag to clang-format.

MyDeveloperDay added a comment. it certainly LGTM, like I said I've had met this issue before and I think this does get around that situation where perhaps your not even using the option it's complaining about (especially if its a C++ option and you are using clang-format for C#) So yes I thin

[PATCH] D87931: [clang-format] Recognize "hxx" as a C++ header in clang-format-diff.py

MyDeveloperDay accepted this revision. MyDeveloperDay 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/D87931/new/ https://reviews.llvm.org/D87931 _

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

MyDeveloperDay added a comment. @JakeMerdichAMD this has caused a regression https://bugs.llvm.org/show_bug.cgi?id=47589 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79388/new/ https://reviews.llvm.org/D79388 ___

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2996 Line->Tokens.back().Tok->MustBreakBefore = true; +Line->Tokens.back().Tok->MustBreakAlignBefore = true; MustBreakBeforeNextToken = false; If the line end

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2996 Line->Tokens.back().Tok->MustBreakBefore = true; +Line->Tokens.back().Tok->MustBreakAlignBefore = true; MustBreakBeforeNextToken = false; MyDeveloperDay

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:746 + Expanded.BraceWrapping.SplitEmptyRecord = true; + Expanded.BraceWrapping.SplitEmptyNamespace = true; + Expanded.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock; I didn't

<    4   5   6   7   8   9   10   11   12   13   >