[PATCH] D93938: [clang-format] Fixed AfterEnum handling

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:1346 verifyFormat("enum { A, B, C } ShortEnum1, ShortEnum2;", Style); Style.AllowShortEnumsOnASingleLine = false; + verifyFormat("enum {\n" EXPECT_FALSE(Style.BraceWrap

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

MyDeveloperDay added a comment. by and large, this looks pretty good to me.. Comment at: clang/docs/ClangFormatStyleOptions.rst:198 -**AlignConsecutiveAssignments** (``bool``) +**AlignConsecutiveAssignments** (``AlignConsecutiveAssignmentsStyle``) If ``true``, aligns conse

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:12581 +"\n" +"/* block comment */\n" +"\n" can you add an example with a block comment that spans multiple lines e.g. ``` int a = 5 /* *

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

MyDeveloperDay added a comment. we don't commit with failing tests so lets understand why it fails. Can you add the tests without multiple names for the enum Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93938/new/ https://reviews.llvm.org/D93938

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:130 +/// \endcode +ACA_AcrossComments + }; Is there a case for aligning over empty lines and comments? ``` int a = 5; /* comment */ int oneTwoThree = 123; `

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. I'm sorry, but now you are missing the files from the review, I think you diffing against your own commits and not commit that are in the repo. This makes the review

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:1347 Style.AllowShortEnumsOnASingleLine = false; + verifyFormat("enum {\n" + " A,\n" + " B,\n" + " C\n" + "} ShortEnum1, ShortEnu

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:130 +/// \endcode +ACA_AcrossComments + }; tinloaf wrote: > MyDeveloperDay wrote: > > Is there a case for aligning over empty lines and comments? > > > > ``` > > int a

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. This LGTM, I'm not sure if others have any further comments Ideally we should add a comment to the release notes, (which you could do via a separate NFC commit if you wanted)

[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1222-1223 +PreviousLine->Last->isOneOf(tok::semi, tok::r_brace) && +RootToken.NewlinesBefore == 1) + ++Newlines; +else if (!Style.EmptyLineBeforeAccessModifier &

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

MyDeveloperDay added a comment. Thank you for submitting this patch? Comment at: clang/include/clang/Format/Format.h:117 +/// int b= 23; +/// +/// int ccc = 23; tinloaf wrote: > HazardyKnusperkeks wrote: > > You are adding a Tab here, or do

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:198 -**AlignConsecutiveAssignments** (``bool``) - If ``true``, aligns consecutive assignments. +**AlignConsecutiveAssignments** (``AlignConsecutiveAssignmentsStyle``) + Style of aligning

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:198 -**AlignConsecutiveAssignments** (``bool``) - If ``true``, aligns consecutive assignments. +**AlignConsecutiveAssignments** (``AlignConsecutiveAssignmentsStyle``) + Style of aligning

[PATCH] D94201: [clang-format] Skip UTF8 Byte Order Mark while sorting includes

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

[PATCH] D94201: [clang-format] Skip UTF8 Byte Order Mark while sorting includes

MyDeveloperDay added a comment. In D94201#2483824 , @HazardyKnusperkeks wrote: > Should the BOM not be ignored in ANY case, not just while sorting includes? > And thus a different code should be touched? > > How about aligned assignments at the beginning

[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

MyDeveloperDay added subscribers: krasimir, klimek. MyDeveloperDay added a comment. @rjelonek please don't set the editable to "clang" but leave it as all users F14974099: image.png otherwise only members of the clang project can actually edit it the review

[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

MyDeveloperDay added a comment. Could you add user case where you see this issue to the description or add a link to the related bug in the bug tracker if there is one, I'm just struggling a little to see what the original problem was. Generally it looks fine, just for understanding I'd like to

[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

MyDeveloperDay added a comment. I'm not seeing this // clang-format off #include "d.h" #include "b.h" // clang-format on #include "a.h" #include "c.h" #include "e.h" $ unix2dos test1.cpp unix2dos: converting file test1.cpp to DOS format... $ clang-format test1.cpp // clang-form

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

MyDeveloperDay added a comment. I've not used this before, but you seem to be suggesting that `#pragma hdrstop` should be used to determine when the files main header should now be placed? Could you explain a little more as to why you think that should be the case (i.e. is there a technical rea

[PATCH] D94217: [clang-format] Find main include after block ended with #pragma hdrstop

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:2308 IncludesInBlock.clear(); -FirstIncludeBlock = false; +if (Trimmed.startswith("#pragma hdrstop")) // precompiled headers + FirstIncludeBlock = true; ---

[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. This LGTM, thank you for the patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94206/new/ https://reviews.llvm.org/D94206 __

[PATCH] D94206: [clang-format] turn on formatting after "clang-format on" while sorting includes

MyDeveloperDay added a comment. In D94206#2485397 , @rjelonek wrote: > I do not have commit access. we are going need to know your name and email address so we can commit on your behalf git commit --amend --author="John Doe " https://llvm.org/docs/P

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

MyDeveloperDay added a comment. I think I would remove the code examples from the "AlignConsecutive style" to avoid confusion (that would be the first change) then perhaps regenerate and update the documentation so we can then see, its seems most have a "code block", can the specific examples n

[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:235 +IO.enumCase(Value, "Never", FormatStyle::ELBAMS_Never); +IO.enumCase(Value, "DontModify", FormatStyle::ELBAMS_DontModify); +IO.enumCase(Value, "LogicalBlock", FormatStyle::ELBAMS_Logica

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:13707 WhitesmithsBraceStyle); verifyFormat("enum X\n" any reason why this is being removed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D94500: Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:819 +Expanded.IndentCaseLabels = true; +Expanded.IndentCaseBlocks = false; break; I guess its fine to define these defaults here but what if people turn them off in their

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

MyDeveloperDay added a comment. My assumption is that you want to stick with the minimum and maximum is that correct? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92257/new/ https://reviews.llvm.org/D92257 ___

[PATCH] D94661: [clang-format] [PR19056] Add support for access modifiers indentation

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:17874 + verifyFormat("class C {\n" + "int i;\n" + "};\n", So this is indented 4 because the Access modifier is 4? when the IndentWidth is 2?

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

MyDeveloperDay added a comment. Thie LGTM, just to check a few nits /usr/bin/sphinx-build -n ./docs ./html /clang/llvm-project/clang/docs/ClangFormatStyleOptions.rst:330: WARNING: Bullet list ends without a blank line; unexpected unindent. /clang/llvm-project/clang/docs/ClangFormatStyleOpt

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

MyDeveloperDay added a comment. Thank you for changing the dump_style to make this clearer F15070790: image.png F15070797: image.png F15070803: image.png F15070810: image.png

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

MyDeveloperDay accepted this revision. MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:103 /// - /// This will align C/C++ preprocessor macros of consecutive lines. - /// Will result in formattings like + /// ``Consecutive`` will result

[PATCH] D92753: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

MyDeveloperDay added a comment. Sorry but due to the following issue raised by @RatTac , I'm reverting this prior to the LLVM 12 branch out so I can work on it further. I also think this would impact #pragma's in any ifdef code which I envisage would be significant. #if defined(WIN32) #pra

[PATCH] D93839: [clang-format] PR48594 BraceWrapping: SplitEmptyRecord ignored for templates

This revision was automatically updated to reflect the committed changes. Closed by commit rG00dc97f16708: [clang-format] PR48594 BraceWrapping: SplitEmptyRecord ignored for templates (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D93776: [clang-format] Add StatementAttributeLikeMacros option

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Please address the "not done" comment (regarding the sorting), but other than that its LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93776/new/ https://reviews.llvm.org/D9

[PATCH] D91949: [clang-format] Add BeforeStructInitialization option in BraceWrapping configuration

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. I think the revision whilst it does what is needed to structs doesn't address the many other times this forms appear. I think we need something a little more extensiv

[PATCH] D93844: [clang-format] Add possibility to be based on parent directory

MyDeveloperDay added a comment. What I can't easily tell from the tests is if you are overriding any styles defined in the parent with a local style. Comment at: clang/include/clang/Format/Format.h:57 + // the parent directories. It is not part of the actual style for formatt

[PATCH] D94906: [clang-format] Apply Allman style to lambdas

MyDeveloperDay added a comment. I think this is LGTM, however.. I've always felt that we shouldn't require `BreakBeforeBraces` to be `Custom` in order to be able to use the BraceWrapping. Users in my view should be able to say: BreakBeforeBraces: Allman BraceWrapping: BeforeLambda

[PATCH] D94906: [clang-format] Apply Allman style to lambdas

MyDeveloperDay added a comment. Similarly I see users having to define everything just to turn one thing off {F15089845, size=full} Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94906/new/ https://reviews.llvm.org/D94906 ___

[PATCH] D109557: Adds a BreakBeforeClosingParen option

MyDeveloperDay added a comment. I stuggle to see that if ( quitelongarg != (alsolongarg - 1) ) is correct, I mean 3 lines for a 1 line if seems like this is something different, its like auto string = std::string( ); This just doesn't seem correct for empty functions

[PATCH] D111273: [clang-format-diff] Fix missing formatting for zero length git diff lines

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. So this isn't just clang-format-diff.py but also all of `git clang-format` git add Analysis.cpp $ git clang-format clang-format did not modify any files This is bad because this is a way in which those using "git cla

[PATCH] D95168: [clang-format] Add InsertBraces option

MyDeveloperDay added a comment. ping @tiagoma are you still around to pursue this feature request or can this be commandeered? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95168/new/ https://reviews.llvm.org/D95168 _

[PATCH] D110803: [clang-format][docs][NFC] correct the "first supported versions" of some of the clang-format options

This revision was automatically updated to reflect the committed changes. Closed by commit rGa2a826d8b66c: [clang-format][docs][NFC] correct the "first supported versions" of some of the… (authored by MyDeveloperDay). Changed prior to commit: https://reviews.llvm.org/D110803?vs=376121&id=37842

[PATCH] D105191: [Clang][OpenMP] Add partial support for Static Device Libraries

MyDeveloperDay added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:120 + /* bitcode SDL?*/ true, + /* PostClang Link? */ false); // Add an intermediate output file. This file now fails clang

[PATCH] D111545: [Clang][NFC] Fix multiline comment prefixes in function headers

MyDeveloperDay added a comment. what @thakis said, this isn't normally my area but this is one of the files that has reverted its "clang-format clean" status with recent commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111545/new/ https://re

[PATCH] D95168: [clang-format] Add InsertBraces option

MyDeveloperDay added a comment. In D95168#3060296 , @tiagoma wrote: > go for it @tiagoma to be honest this actually works pretty well, I've been using it in a private build on the large project I work on and I've been ripping through some of the legacy

[PATCH] D95168: [clang-format] Add InsertBraces option

MyDeveloperDay added a comment. To confirm if (x) for(int i=0;i<10;i++) foo(); will be transformed to if (x) for (int i = 0; i < 10; i++) { foo(); } but this itself will NOT be transformed to if (x) { for (int i = 0; i < 10; i++) { foo(); }

[PATCH] D111545: [Clang][NFC] Fix multiline comment prefixes in function headers

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. To me you should be doing /*bitcode SDL=*/true It looks like its still failing clang-format might I suggest making the changes to the comment, git adding the file

[PATCH] D84375: [git-clang-format] Add --diffstat parameter

MyDeveloperDay added a comment. @roligugus I can land this for you, please rebase the review then please add your name and email here so we can ensure you get the credit. https://llvm.org/docs/DeveloperPolicy.html#commit-messages Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D109557: Adds a BreakBeforeClosingParen option

MyDeveloperDay added a comment. > The you quoted would, in my mind, be formatted like this: > > void foo() { > if ( > quitelongarg != (alsolongarg - 1) > ) { // ABC is a very long comment > return; > } > } > > This is because I don't allow br

[PATCH] D109557: Adds a BreakBeforeClosingParen option

MyDeveloperDay added a comment. Do you think this is going to need some other capability to put the break after the opening paren? e.g. `BreakAfterOpeningParen` if ( ^ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109557/new/ https://rev

[PATCH] D109557: Adds a BreakBeforeClosingParen option

MyDeveloperDay added a comment. Personally I'm not convinced there wouldn't be people who want this for function declarations and definitions Function( param1, param2, param3 ); but don't want this if ( foo() ) Repository: rG LLVM Github Monorepo CHAN

[PATCH] D33029: [clang-format] add option for dangling parenthesis

MyDeveloperDay added a subscriber: csmulhern. MyDeveloperDay added a comment. I understand the frustration (I'm not convinced that its Phabricators fault to be honest, that's our process and plenty of people follow it without issues) , Our incredible original code owners have moved on to do I as

[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

MyDeveloperDay added a comment. @thakis , At the back of my mind is those people who always say "Measure, Measure, Measure"... This issue still plagues us, we constantly get bugs reported that come back to this. (https://bugs.llvm.org/show_bug.cgi?id=52021,https://bugs.llvm.org/show_bug.cgi?i

[PATCH] D111815: [clang-format] [PR42014,PR52021] don't let clang-format assert/crash when file being formatted is read-only/locked

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: thakis, curdeius, HazardyKnusperkeks. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. This is a bug which gets reported from time to time and we've had multiple attempts to

[PATCH] D109557: Adds a BreakBeforeClosingParen option

MyDeveloperDay added a comment. Just as a general pattern what we see is that options start out as `bool`, shortly become `enums`, then as they get more complex become `structs` e.g. `BraceWrapping` bool BreakBeforeClosingParen trying to think ahead a little can make future backwards compati

[PATCH] D111815: [clang-format] [PR42014,PR52021] don't let clang-format assert/crash when file being formatted is read-only/locked

This revision was automatically updated to reflect the committed changes. Closed by commit rGa92cf5a5a0cd: [clang-format] [PR42014,PR52021] don't let clang-format assert/crash when file… (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D84375: [git-clang-format] Add --diffstat parameter

MyDeveloperDay added a comment. In D84375#3065785 , @roligugus wrote: > @MyDeveloperDay Thanks for the follow-up! I've rebased on latest main. > > Out of curiosity, as I am trying to wrap my head around the llvm workflow: > Could I `arc land ...` myself o

[PATCH] D84375: [git-clang-format] Add --diffstat parameter

MyDeveloperDay added a comment. I just tested this an it seems to work $ git clang-format --diffstat clang/lib/Format/Format.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84375/ne

[PATCH] D84375: [git-clang-format] Add --diffstat parameter

This revision was automatically updated to reflect the committed changes. Closed by commit rG191a395343b9: [git-clang-format] Add --diffstat parameter (authored by roligugus, committed by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D111975: [clang-format] [PR52015] clang-format should put __attribute__((foo)) on its own line before @interface / @implementation / @protocol

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: benhamilton, krasimir, HazardyKnusperkeks, curdeius. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=52015 A newline should be place b

[PATCH] D112019: [clang-format] [PR51412] AlignConsecutiveMacros fights with Visual Studio and resource.h

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, krasimir. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. clang-format `AlignConsecutiveMacros` feature causes real problems when using Win32 r

[PATCH] D111273: [clang-format-diff] Fix missing formatting for zero length git diff lines

MyDeveloperDay added a comment. Just guessing but is this caused by the text at the bottom? There doesn’t seem to be a zero line change in the patch file —- 2.30.1 (Apple Git-130) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111273/new/ https://

[PATCH] D111273: [clang-format-diff] Fix missing formatting for zero length git diff lines

MyDeveloperDay added a comment. The following fix should resolve this $ git diff git-clang-format diff --git a/clang/tools/clang-format/git-clang-format b/clang/tools/clang-format/git-clang-format index 40e848d55a8c..c7e15eb7b483 100755 --- a/clang/tools/clang-format/git-clang-format +

[PATCH] D112056: [clang-format] git-clang-format throws an assertion when removing files as part of the commit

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, krasimir, HazardyKnusperkeks. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. Following a change D111273: [clang-format-diff] Fix missing formatting for zero lengt

[PATCH] D112056: [clang-format] git-clang-format throws an assertion when removing files as part of the commit

MyDeveloperDay added a comment. In D112056#3073142 , @lhames wrote: > I think that this should fix the issue I saw, but `clang-format` will still > crash when passed a `-lines=0:X` option. Would it make sense to either error > out in that tool, or teach

[PATCH] D112019: [clang-format] [PR51412] AlignConsecutiveMacros fights with Visual Studio and resource.h

MyDeveloperDay added a comment. In D112019#3073628 , @HazardyKnusperkeks wrote: > Otherwise it looks good. Thanks @HazardyKnusperkeks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112019/new/ https://rev

[PATCH] D112056: [clang-format] git-clang-format throws an assertion when removing files as part of the commit

This revision was automatically updated to reflect the committed changes. Closed by commit rG5e969125c609: [clang-format] git-clang-format throws an assertion when removing files as part… (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D111975: [clang-format] [PR52015] clang-format should put __attribute__((foo)) on its own line before @interface / @implementation / @protocol

This revision was automatically updated to reflect the committed changes. Closed by commit rG57e00810edd7: [clang-format] [PR52015] clang-format should put __attribute__((foo)) on its… (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

MyDeveloperDay added a comment. FYI I doubt you'll get a response from the original clang-format code owners as they have moved on. But I can take a look Before giving this the ok, I'd like to understand a little more. if I make a (bad)formatting change in a file which is not already staged and

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Much, thank you for seeing this over the line, This LGTM, lets wait a bit in case any of the others have a comment. I assume you need help committing this? if so we need your n

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

MyDeveloperDay added a subscriber: lodato. MyDeveloperDay added a comment. @lodato if you are still around (I see no changes since 2017), we may be able to add you as a "Co-author" I'm not sure of the policy, (again I would need your name and email address) Co-authored-by: name Repository:

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

MyDeveloperDay added a comment. No problem, I hope your ok if @lodato gets back to us, I think we should probably try and credit them a least a little. Like I said let us wait a bit for people to have a chance to take a look (I'm not in the US hence why I've seen it already), I doubt there wi

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add flags

MyDeveloperDay added a comment. Fundamentally this looks ok to me, the biggest concern is fathoming out the change in TokenAnnotator.cpp to mean the same thing, but I think that is what the tests should be for I think. One think I do is use the output of this https://clang.llvm.org/docs/ClangF

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add flags

MyDeveloperDay added a comment. Fundamentally this looks ok to me, the biggest concern is fathoming out the change in TokenAnnotator.cpp to mean the same thing, but I think that is what the tests should be for I think. One think I do is use the output of this https://clang.llvm.org/docs/ClangF

[PATCH] D108538: [clang-format] break after the closing paren of a TypeScript decoration

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM, Thanks for picking these up Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108538/new/ https://reviews.llvm.org/D108538 __

[PATCH] D108620: [clang-format] keep TypeScript argument decorators in line

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

[PATCH] D108620: [clang-format] keep TypeScript argument decorators in line

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:497 if (Style.Language == FormatStyle::LK_JavaScript && - Previous.is(tok::r_paren) && Previous.is(TT_JavaAnnotation)) { + Current.TokenText == "get" && Previous.is(tok::r_par

[PATCH] D108692: [clang-format] Support TypeScript override keyword

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

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. This looks good, I like to move towards one style, it was getting too confusing. Did you test this on a large code base at all? and maybe wait for one of the others to take a l

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Thank you for your submission, but... 1. This is not the way to change this file, its autogenerated from clang/include/Format/Format.h using clang/docs/tools/dump_fo

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

MyDeveloperDay added subscribers: HazardyKnusperkeks, owenpan, krasimir, sammccall, curdeius, klimek. MyDeveloperDay added a comment. In D108765#2967363 , @FederAndInk wrote: > Thank you for your explanations, I understand now. > > But as I look into `c

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

MyDeveloperDay added a comment. I'm good with words like `List of Strings` but I don't think we need `Enum` `unsigned` I think Integer, I'm not sure what the code is even going to do if you supply a -ve, Warn I hope! (there's contribution idea number 2 for you right there!) ;-) Repository:

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

MyDeveloperDay added a comment. Possible bug https://bugs.llvm.org/show_bug.cgi?id=51640 Comment at: clang/include/clang/Format/Format.h:547 /// false: - /// enum - /// { + /// enum { /// A, Post commit Nit: @lunasorcery Rule of thumb in

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

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: krasimir, curdeius, HazardyKnusperkeks. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. LLVM 13.0.0-rc2 shows change of behaviour in enum and interface BraceWrapping (likely

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

MyDeveloperDay updated this revision to Diff 369046. MyDeveloperDay added a comment. Remove the unnecessary comment changes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108810/new/ https://reviews.llvm.org/D108810 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/F

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

MyDeveloperDay updated this revision to Diff 369052. MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. Address Nit to reduce if nesting [--] Global test environment tear-down [==] 814 tests from 25 test suites ran. (41295 ms total) [ PASSED ]

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3803 + if (Line.startsWith(Keywords.kw_interface) && + Style.BraceWrapping.AfterClass) +return true; krasimir wrote: > nit: the `&& Style.BraceWrapping.Aft

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM, thanks for adding that and fixing the spelling mistake, let the others have time to chip in. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108765/new/ https://reviews.llv

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

MyDeveloperDay updated this revision to Diff 369082. MyDeveloperDay added a comment. Handle comments before the enum CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108810/new/ https://reviews.llvm.org/D108810 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTe

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

MyDeveloperDay added a comment. In D108765#2969036 , @FederAndInk wrote: > For now, I handle plural manually, but it can be changed, I also kept > Unsigned, what are your thoughts? I think thats ok for now, did you try building the file with sphinx

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGed367b9dff10: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C#… (authored by MyDeveloperDay). Repository: rG LLVM Git

[PATCH] D108882: Add backward compatibility tests for PackConstructorInitializers

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. It seems a little convoluted, but OK CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108882/new/ https://reviews.llvm.org/D108882 __

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

MyDeveloperDay added inline comments. Comment at: clang/docs/tools/dump_format_style.py:23 + plurals = { +'IncludeCategory': 'IncludeCategories' + } HazardyKnusperkeks wrote: > FederAndInk wrote: > > HazardyKnusperkeks wrote: > > > Could you not just check

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

MyDeveloperDay added inline comments. Comment at: clang/docs/tools/dump_format_style.py:26 +def register_plural(singular: str, plural: str): + if plural not in plurals: This failed for me with invalid syntax Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

MyDeveloperDay added inline comments. Comment at: clang/docs/tools/dump_format_style.py:9 import re +import inspect +import subprocess FederAndInk wrote: > HazardyKnusperkeks wrote: > > I think these should be sorted. > ok, it will be done are these standard imp

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

MyDeveloperDay added a comment. error: pathspec './plurals.txt' did not match any file(s) known to git Traceback (most recent call last): File "./dump_format_style.py", line 18, in subprocess.check_call(['git', 'checkout', '--', PLURAL_FILE]) File "/usr/lib/python3.8/subprocess.

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. I think interacting with git or even blocking for input doesn’t feel right I have an issue out on llvm preserve checks that would run this tool and that couldn’t bloc

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

MyDeveloperDay added a comment. For me it feels enough to output to stderr that we are missing plurals and what they are for, it’s not like you can say what they should be is it? This will happen so infrequently that it’s not worth the environment issues that checking git will cause. Reposit

<    19   20   21   22   23   24   25   >