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

MyDeveloperDay added subscribers: sammccall, klimek. MyDeveloperDay added a comment. In D147176#4231952 , @rymiel wrote: >> didn't go via the normal clang-format reviewers > > Is there a reason for this? This isn't the only case either, for example: > htt

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

MyDeveloperDay added a comment. I can auto add us as all as reviewers if you'd like? This means if the title contains `[clang-format]` or it impacts a file path of /clang/lib/Format it should in theory run this rule F26945275: image.png Repository: rG L

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

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

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

MyDeveloperDay added a comment. Also I've added a couple of other rules F26945535: image.png This should in theory, automatically add a comment to a clang-format review that contains a Format.h change but not a ClangFormatStyleOption.rst change and vice ve

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

MyDeveloperDay added a comment. ...test comment which should fire an automated, you don't have any unit tests...comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147176/new/ https://reviews.llvm.org/D147176 __

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

MyDeveloperDay added a comment. In D147176#4232808 , @klimek wrote: > I apologize for not tagging appropriately - is that process documented > somewhere? I've implemented it via Phabricator Herald rules now, so we should be good going forward, @klimek

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

MyDeveloperDay added a comment. I don't like using additional variables in unit tests, if someone changes 25400 multiple tests could break, each test should be stand alone in my view Herald added a comment. NOTE: Clang-Format Team Automated Review Comment Your review contains a change to Clan

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

MyDeveloperDay added a comment. Can you show your example from a implementation case and not just a declaration i.e. what happens with `int function1(int param1) {}` Comment at: clang/lib/Format/TokenAnnotator.cpp:4789 + if (Left.is(tok::l_paren) && Style.AlwaysBreakBeforeFu

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

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:1374 +**AlwaysBreakBeforeFunctionParameters** (``Boolean``) :versionbadge:`clang-format 16.0` :ref:`¶ ` + If ``true``, always break before function parameters in a declaration. + --

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

MyDeveloperDay added a comment. because of https://github.com/llvm/llvm-project/issues/61785 should this really be reverted? is basically saying `X * Y {` must be `X *Y{` but that obviously not the case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

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

MyDeveloperDay added a comment. In D137327#4233551 , @thieta wrote: > In D137327#4233290 , > @MyDeveloperDay wrote: > >> because of https://github.com/llvm/llvm-project/issues/61785 should this >> really be reve

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

MyDeveloperDay added a comment. I like both @owenpan suggestions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137327/new/ https://reviews.llvm.org/D137327 ___ cfe-commits mailing list cfe-commits@lists.

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

MyDeveloperDay added a comment. In D137327#4234463 , @thieta wrote: > This was released in LLVM 16.0.0. The prior behaviour was there before, it’s marked in GitHub as a regression, can you please revert, we’ll mark the issue to be cherry picked, then le

[PATCH] D147295: [clang-format] Don't misannotate left squares as lambda introducers

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147295/new/ https://reviews.llvm.org/D147295 ___ cfe-commits mailing list cfe-com

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

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:949 + /// If unset, ``ContinuationIndentWidth`` is used. + /// \code + /// AlignAfterOpenBracket: AlwaysBreak did you check generating the html from the rst? I can never re

[PATCH] D147422: [clang-format] NFC Document the other space before colon option

MyDeveloperDay added a comment. In D147422#4240021 , @owenpan wrote: > Should we extend `SpacesInContainerLiterals` so that it controls JSON colons > too? If yes, then we don't need `SpaceBeforeJsonColon`. Otherwise, IMO we > should leave the doc alone.

[PATCH] D147422: [clang-format] NFC Document the other space before colon option

MyDeveloperDay added a comment. In D147422#4239701 , @Herald wrote: > NOTE: Clang-Format Team Automated Review Comment > > It looks like your clang-format review does not contain any unit tests, > please try to ensure all code changes have a unit test (u

[PATCH] D147577: [Format/ObjC] Support NS_ERROR_ENUM in ObjC language guesser

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/D147577/new/ https://reviews.llvm.org/D147577 ___

[PATCH] D128440: [WebAssembly] Initial support for reference type funcref in clang

MyDeveloperDay added inline comments. Herald added a project: clang-format. Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. Comment at: clang/lib/Format/FormatToken.h:615 tok::kw__Null_unspecified, tok::kw___ptr32, tok::kw___ptr64,

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

MyDeveloperDay added a comment. Can you mark your comments as done Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146101/new/ https://reviews.llvm.org/D146101 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D147577: [Format/ObjC] Support NS_ERROR_ENUM in ObjC language guesser

MyDeveloperDay added a comment. The build machines aren’t using a latest enough version of clang-format for our local .clang-format file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147577/new/ https://reviews.llvm.org/D147577 __

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

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Sorry I don’t get how this change helps. Removing the option values does make it clearer IMHO CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147894/new/ http

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

MyDeveloperDay added a comment. In D93240#4254439 , @sstwcw wrote: > A goto label isn't affected by this option. Is it intentional? why would it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93240/new/ h

[PATCH] D147969: Add InsertBraces to ChromiumStyle

MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/Format.cpp:1699 ChromiumStyle.DerivePointerAlignment = false; +ChromiumStyle.InsertBraces = true; if

[PATCH] D147969: Add InsertBraces to ChromiumStyle

MyDeveloperDay added a comment. This was the agreement reached when we pushed for clang format to start being allowed to add or rearrange tokens, I.E. in this case we add {} as such we could, as you saw yourselves make mistakes if our assumptions are not 100% correct and break code, we prefer p

[PATCH] D147969: Add InsertBraces to ChromiumStyle

MyDeveloperDay added a comment. These features tend to have an additional performance penalty that not all may want to pay, even if they did base their style on your style. I.E. chromium isn’t likely the only ones using chromium style Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Yes I didn’t see them repeated below CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147894/new/ https://reviews.llvm.org/D147894 __

[PATCH] D147969: Add InsertBraces to ChromiumStyle

MyDeveloperDay added a comment. RFC https://groups.google.com/g/llvm-dev/c/wka1Bnrd-aU?pli=1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147969/new/ https://reviews.llvm.org/D147969 ___ cfe-commits mai

[PATCH] D147969: Add InsertBraces to ChromiumStyle

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:1699 ChromiumStyle.DerivePointerAlignment = false; +ChromiumStyle.InsertBraces = true; if (Language == FormatStyle::LK_ObjC) owenpan wrote: > aaron.ballman wrote: > > Haza

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. If all comments and concerns are done, then I'm inclined to accept, but I'd like @owenpan and @HazardyKnusperkeks to give their opinion before we land this. Repository: rG

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

MyDeveloperDay added a comment. is it possible to point to a github issue that this related to in the review summary, if not please can you make one so we can track the issue its trying to solve Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125171

[PATCH] D148200: [clang-format] Correctly indent comment above PP Directive

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/D148200/new/ https://reviews.llvm.org/D148200 ___

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

MyDeveloperDay added a comment. In D147176#4263621 , @owenpan wrote: > @MyDeveloperDay We keep getting "spammed" by [libc++][format] patches. Can > you delete the rule in H987 : > > "field": "differential.revision.title",

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

MyDeveloperDay added a comment. we should be good now F27132578: image.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147176/new/ https://reviews.llvm.org/D147176 ___

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:876 Style.AlwaysBreakBeforeMultilineStrings); +IO.mapOptional("AlwaysBreakBeforeFunctionParameters", + Style.AlwaysBreakBeforeFunctionParameters); -

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

MyDeveloperDay added a comment. In D137327#4267239 , @thieta wrote: > In D137327#4235255 , > @MyDeveloperDay wrote: > >> In D137327#4234463 , @thieta wrote: >> >>> This w

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

MyDeveloperDay added a comment. In D147176#4266496 , @owenpan wrote: > In D147176#4265130 , > @MyDeveloperDay wrote: > >> we should be good now > > Thanks! I really like these new rules! Yeah sorry for the Spam,

[PATCH] D148447: [clang-format] Fix regression with AlignTrailingComments set to true

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. If this was the functionality pre 16 then LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148447/new/ https://reviews.llvm.or

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

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: owenpan, HazardyKnusperkeks, rymiel. MyDeveloperDay added a project: clang-format. Herald added projects: All, clang. MyDeveloperDay requested review of this revision. This change allows always breaking before braces on C# sette

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

MyDeveloperDay updated this revision to Diff 514017. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D148467 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h clang/lib/Format/Format.cp

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

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTestCSharp.cpp:1633 + "{\n" + "string Foo { set; get }\n" + "}\n", Hmm.. maybe this should be ``` string Foo { set;get } ``` CHAN

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

MyDeveloperDay added a comment. I'm not convinced `AfterCSharpProperty` is a good name, open for suggestions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D148467 ___ cfe-commits mailing list cfe-commits@

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

MyDeveloperDay updated this revision to Diff 514029. MyDeveloperDay added a comment. for default `set;get` or `get;set` for when `AfterCSharpProperty` is true, public Foo { set; get; } CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D1

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

MyDeveloperDay updated this revision to Diff 514030. MyDeveloperDay added a comment. re clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D148467 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/

[PATCH] D148472: [clang-format] CSharp don't allow there not to be a space between `is` and `[`

MyDeveloperDay created this revision. MyDeveloperDay added a reviewer: owenpan. MyDeveloperDay added a project: clang-format. Herald added projects: All, clang. Herald added reviewers: rymiel, HazardyKnusperkeks. MyDeveloperDay requested review of this revision. as `is` is a keyword in C# ensure t

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

MyDeveloperDay created this revision. MyDeveloperDay added a reviewer: owenpan. MyDeveloperDay added a project: clang-format. Herald added projects: All, clang. Herald added reviewers: rymiel, HazardyKnusperkeks. MyDeveloperDay requested review of this revision. Refactor the CSharpNullable assignm

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

MyDeveloperDay added a comment. The exclusions are not complete for example `cond? foo() : "B";` would still fail. but this moves us a little closer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148473/new/ https://reviews.llvm.org/D148473 __

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

MyDeveloperDay added a comment. @exv any thoughts on this one? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148473/new/ https://reviews.llvm.org/D148473 ___ cfe-commits mailing list cfe-commits@lists.ll

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

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

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:4525 -const FormatToken *Next = Right.getNextNonComment(); -if (!Next || Next->is(tok::semi)) return false; how is the semi case handled or is it not needed Rep

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

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. I know it might not seem an obvious use case but there really isn't a requirement to not include header files more than once.. imaging if I have #define ARCH "win32" #include "MyDataStructThatContainsPlaformSp

[PATCH] D132256: [clang-format] Add DefinitionBlockSpacing option

MyDeveloperDay added a comment. Herald added a reviewer: rymiel. Cloud you include a test that contains multiple levels of nested scope, I'm assuming we won't add an additonal line at every {} level (or will we?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

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

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

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

MyDeveloperDay added a comment. > Regarding the comment that I must not change existing tests: I think this > rule is too strict, because those tests are mostly regression tests. > But a regression tests does not test for correctness. So if a test had > already a wrong assumption, it must be ch

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thank you for the indepth explaination in https://github.com/llvm/llvm-project/issues/59178, that was really helpful for me trying to understand what the problem was. I thik y

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

MyDeveloperDay added a comment. We don't normally land broken tests, even if they are disabled, its better for us if we get a fix at the same time ;-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151145/new/ https://reviews.llvm.org/D151145

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

MyDeveloperDay added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151047/new/

[PATCH] D151578: [Format/ObjC] Support NS_ASSUME_NONNULL_BEGIN and FOUNDATION_EXPORT in ObjC language guesser

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/D151578/new/ https://reviews.llvm.org/D151578 ___

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseLabels

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:790 + +**AlignConsecutiveShortCaseLabels** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 17` :ref:`¶ ` + Style of aligning consecutive short case labels. Did you

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseLabels

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:790 + +**AlignConsecutiveShortCaseLabels** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 17` :ref:`¶ ` + Style of aligning consecutive short case labels. galeneli

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143546/new/ https://reviews.llvm.org/D143546 ___ cfe-commits mailing list cfe-com

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

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

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2836 + Tok = Tok->getNextNonComment(); + } else if (Tok->is(tok::hashhash)) { +// Concatenation. Skip. are we covering these cases in the unit tests

[PATCH] D143755: [clang-format] Add a space between an overloaded operator and '>'

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/D143755/new/ https://reviews.llvm.org/D143755 ___ cfe-commits mailing li

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thank you for adding the tests, as I don't know Verilog then I can't really comment on the correctness, as you are mostly in your own scoped verilog functions, I'm fine with yo

[PATCH] D144317: [clang-format] Fix handling of TypeScript tuples with optional last member

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Thank you for this LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144317/new/ https://reviews.llvm.org/D144317 ___ cfe-commits m

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Thanks for doing this @rymiel, LGTM.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144537/new/ https://reviews.llvm.org/D144537 ___

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

MyDeveloperDay added a comment. maybe we should cherry pick into 16? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144537/new/ https://reviews.llvm.org/D144537 ___ cfe-commits mailing list cfe-commits@li

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

MyDeveloperDay added a comment. Can you please add unit tests for the cases you added Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144709/new/ https://reviews.llvm.org/D144709 ___ cfe-commits mailing li

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

MyDeveloperDay added subscribers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. MyDeveloperDay added a comment. I'm one of the code owners for clang-format, but I'm not an emacs user (don't hate me..), honestly I don't think you'll get a response from anyone others than the clang-format

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

MyDeveloperDay added a comment. please mark the comments as done when you've addressed them CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143560/new/ https://reviews.llvm.org/D143560 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

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

MyDeveloperDay added a comment. Interesting Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:69 + // Ignore the comma separators. + continue; +} else if (Tok->isOneOf(tok::identifier, tok::kw_class)) { You need a unit tests that t

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

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp:77 + verifyFormat("@property(a, b, c) int p;", "@property(b, a, c) int p;", Style); + verifyFormat("@property(a, b, c) int p;", "@property(c, b, a) int p;", Style)

[PATCH] D150506: Migrate {starts,ends}with_insensitive to {starts,ends}_with_insensitive (NFC)

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. no concerns from the clang-format front. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150506/new/ https://reviews.llvm.org/D150

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

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp:156 + + for(auto const& Attribute: getAllObjCAttributes()) { +Style.ObjCPropertyAttributeOrder = { "FIRST", Attribute, "LAST" }; I'm not a mass

[PATCH] D148472: [clang-format] CSharp don't allow there not to be a space between `is` and `[`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. MyDeveloperDay marked an inline comment as done. Closed by commit rG63395cb0b69d: [clang-format] CSharp don't allow there not to be a space between `is` and `[` (author

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

MyDeveloperDay updated this revision to Diff 514343. MyDeveloperDay added a comment. simplify negative if CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148473/new/ https://reviews.llvm.org/D148473 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/TokenAnnotatorTest.

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG799b794d7699: [clang-format] C# short ternary operator misinterpreted as a CSharpNullable (authored by MyDeveloperDay). Changed prior to commit: h

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

MyDeveloperDay updated this revision to Diff 514586. MyDeveloperDay marked 8 inline comments as done. MyDeveloperDay added a comment. Address review comments, still would like to solve the indenting issue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.or

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

MyDeveloperDay planned changes to this revision. MyDeveloperDay added a comment. need to handle `init;` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D148467 ___ cfe-commits mailing list cfe-commits@lists.

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

MyDeveloperDay added a comment. >> public Foo { >> set; >> get; >> } > > At least from my experience, the getter is specified before the setter, > though I'm unsure how important this is in your eyes. Lets hold off the idea of swapping `set;get` around from this patch, but this

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

MyDeveloperDay updated this revision to Diff 514682. MyDeveloperDay added a comment. add init support and fix indentation issue when only one property is defined but the is an auto-property CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D148467 File

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

MyDeveloperDay updated this revision to Diff 514684. MyDeveloperDay added a comment. upload the correct patch file CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D148467 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst

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

MyDeveloperDay added a comment. There is more to this than meets the eye.. what we have so far, from existing `AfterFunction` use and the propsed here `AfterCSharpProperty` is... public Foo {<--- controlled by **AfterFunction **(rightly or wrongly) get {

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

MyDeveloperDay updated this revision to Diff 515060. MyDeveloperDay added a comment. Adds additional options to give us much greater control over how we format C# properties especially auto properties (Submitting this for safe keeping, I need to try and minimize the if expression, I'm sure it c

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

MyDeveloperDay updated this revision to Diff 515068. MyDeveloperDay added a comment. Sorry had a clang-format reflow comments moment, revert the unrelated comment changes simplify the if's a little CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D1484

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

MyDeveloperDay added a comment. In my C# project, these settings give me just what we tend to use. BreakBeforeBraces: Custom BraceWrapping: AfterFunction: true AfterCSharpProperty: true AllowShortCSharpPropertiesOnASingleLine: false AlwaysBreakBetweenShortCSharpProperties: false

[PATCH] D148777: [clang-format] Hanlde leading whitespaces for JSON files

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148777/new/ https://reviews.llvm.org/D148777 ___ cfe-commits mailing list cfe-com

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

MyDeveloperDay updated this revision to Diff 515298. MyDeveloperDay marked 10 inline comments as done. MyDeveloperDay added a comment. Address review comment, add convenience functions to simplify conditions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.

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

MyDeveloperDay planned changes to this revision. MyDeveloperDay added a comment. There is another case I need to cover Style.BraceWrapping.AfterCSharpProperty = false; Style.AllowShortCSharpPropertiesOnASingleLine = false; Style.AlwaysBreakBetweenShortCSharpProperties = false; Style.Brace

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

MyDeveloperDay updated this revision to Diff 515372. MyDeveloperDay added a comment. Handle the case string Foo { set;get; } CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D148467 Files: clang/docs/ClangFormatStyleOptions.rst clang

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

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:816 +/// \code +///original: +///int someFunction(int arg1, int arg2); Can you format this as we do the other documentation? Repository: rG LLVM Github

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.h:231 /// Used e.g. to break like: + /// ``` /// functionCall(Parameter, otherCall( If you want this to look like code you need to use \code \endcode CHANGES SINCE LA

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.h:231 /// Used e.g. to break like: + /// ``` /// functionCall(Parameter, otherCall( MyDeveloperDay wrote: > If you want this to look like code you need to use \code \en

[PATCH] D149088: [clang-format] Add run-clang-format.py script.

MyDeveloperDay added a comment. is it possible to pass other arguments like `-n` if not running in place is there any protection from prevent stdout/stderr from overlapping..? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149088/new/ https://revi

[PATCH] D149088: [clang-format] Add run-clang-format.py script.

MyDeveloperDay added inline comments. Comment at: clang/tools/run-clang-format.py:63 +default="c,cc,cpp,cxx,c++,h,hh,hpp,hxx,h++") +parser.add_argument('-style', +help='formatting style', what about ``` run-cl

[PATCH] D149561: [clang-format] Recognize Verilog edge identifiers

MyDeveloperDay added a comment. Could you add anotator tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149561/new/ https://reviews.llvm.org/D149561 ___ cfe-commits mailing list cfe-commits@lists.llv

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

MyDeveloperDay added a comment. if we don't plan on fixing the issues please abandon the review CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145435/new/ https://reviews.llvm.org/D145435 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:347 +IO.enumCase(Value, "false", FormatStyle::FPBS_Leave); +IO.enumCase(Value, "true", FormatStyle::FPBS_Always); + } yes remove these... this is why you need to mark the comme

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

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:4027 + /// is probably for the port on the following line instead of the parenthesis + /// it follows. /// \code This seems an odd corner case Repository: rG LLVM Githu

<    15   16   17   18   19   20   21   22   23   24   >