[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

2020-11-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Do you think we should show what it looks like with more complex asm examples in the tests? what would the following look like? ` asm volatile("rbit %[aVal], %[aVal]" : [aVal] "+r"(val) : "[aVal]"(val));` `__asm__ __volatile__("str x0, %0" : "=m"(regs->regs[0]));

[PATCH] D91949: [clang-format] Add BreakBeforeStructInitialization configuration

2020-11-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Ok, thank you for making the changes so its easier to review, Do you think this style should be part of the "BraceMapping" style? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91949/new/ https://reviews.llvm.org/D91949 __

[PATCH] D91949: [clang-format] Add BreakBeforeStructInitialization configuration

2020-11-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Firstly thank you for the patch, and believe me I don't want to discourage you (as I remember my first patches), I think the rational is ok, but I think you might be laser focused on your use case and not looking at the impact on the change of the wider clang-for

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

2020-11-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. 1. You need to default initialize BreakBeforeInlineASMColon in the LLVM style. FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { ... LLVMStyle.BreakBeforeInlineASMColon = false; ... 2. You need a "CHECK_PARSE_BOOL" unit

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

2020-11-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. My understanding is that the following current behavour ` asm("AA" : "DEF" : "GHI"); asm volatile( "AAA" : "DEF" : "

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

2020-11-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2727 + + * ``unsigned Maximum`` The maximum number of spaces at the start of the comment. + I'm personally not a massive fan of stuffing -1 into an unsigned but I understa

[PATCH] D91996: [clang-format] Remove double trim

2020-11-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. In D91996#2417892 , @klimek wrote: > Isn't the comment incorrect after this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91996/n

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

2020-11-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think fundamentally from my perspective this seem ok, out of interest can I ask what drove you to require it? My assumption is that some people write comments like // Free comment without space\n" and you want to be able to consistently format it to be (N sp

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

2020-12-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2710 +**SpacesInLineComments** (``SpacesInLineComment``) + How many spaces are allowed at the start of a line comment. To disable the Is this change generated? with clang/

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I don't think it was far off, I just I agreed with @klimek in trying to address the many different cases that came up I started to feel that `parseConstraintExpression` was becoming fragile Having said that I like to think because they are in concepts specific p

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

2020-12-03 Thread MyDeveloperDay via Phabricator via cfe-commits
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 comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92257/new/ https://reviews.llv

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 309335. MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. rebase before making any further changes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79773/new/ https://reviews.llvm.org/D79773 Files: clang/docs/Cl

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2314 + nextToken(); + if (FormatTok->Tok.is(tok::less)) { +while (!FormatTok->Tok.is(tok::greater)) { kli

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 309350. MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. Address the issue with still munching the semi mark as experimental in the release notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79773/new/ https:/

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Having reviewed the current status I'm going to leave this patch with just minor changes from where its been for some time, if others feel it has merits in landing as experimental support I'd be happy with that. I would rather build from a base than try and keep

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D79773#2432088 , @miscco wrote: > As someone who has extensively worked with concepts I cannot stress how much > this would improve my live and I'd like to think that this will help even in its current form, but it nee

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

2020-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Its close Comment at: clang/lib/Format/Format.cpp:751 /*AfterFunction=*/false, /*AfterName

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @JohelEGP can you let me know if you plan to remove the "requested changes" you asked for back in July? I cannot commit with the revision until that is removed CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79773/new/ https://reviews.llvm.org/D79773 ___

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG840e651dc6d7: [clang-format] Improve clang-formats handling of concepts (authored by MyDeveloperDay). Repository: rG LL

[PATCH] D92666: [clang-format] [NFC] keep clang-format tests clang-format clean

2020-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, sammccall. MyDeveloperDay added a project: clang-format. MyDeveloperDay requested review of this revision. Herald added a project: clang. I use several of the clang-format clean directories as a test suite, this one had

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D79773#2434126 , @JohelEGP wrote: > I was a bit late, but thanks for everything! No problem, sorry not for waiting, but let the games begin. P.S. I think I may have left a comment for you over in one of your bugs. not

[PATCH] D92666: [clang-format] [NFC] keep clang-format tests clang-format clean

2020-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 309602. MyDeveloperDay added a comment. change the mode back CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92666/new/ https://reviews.llvm.org/D92666 Files: clang/unittests/Format/MacroExpanderTest.cpp Index: clang/unittests/Format/Macro

[PATCH] D92666: [clang-format] [NFC] keep clang-format tests clang-format clean

2020-12-05 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0e226d00d21c: [clang-format] [NFC] keep clang-format tests clang-format clean (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D926

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

2020-12-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Maybe I'm confused if you the setting is to break before ASM Colon then I expect it to always break, is that not what is required? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 _

[PATCH] D91507: [clang-format] Add option for case sensitive regexes for sorted includes

2020-12-05 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8668eae2adf2: [clang-format] Add option for case sensitive regexes for sorted includes (authored by MyDeveloperDay). Changed prior to commit: http

[PATCH] D27377: clang-format: Support the Java 8 'default' method modifier

2020-12-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay commandeered this revision. MyDeveloperDay edited reviewers, added: lhchavez; removed: MyDeveloperDay. MyDeveloperDay added a comment. This revision is no needed, the test will pass without any additional code changes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D27377/ne

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

2020-12-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Can I assume you need someone to land this for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92257/new/ https://reviews.llvm.org/D92257 ___ cfe-commits mailing list c

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

2020-12-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D92257#2435899 , @HazardyKnusperkeks wrote: > In D92257#2435701 , @MyDeveloperDay > wrote: > >> Can I assume you need someone to land this for you? > > Yes I do. But I have a que

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

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, JakeMerdichAMD, krasimir. MyDeveloperDay added projects: clang-format, clang. MyDeveloperDay requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. A quick search of

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think my expectation would be that if you are going to have an option `BreakBeforeInlineASMColon` then it should do it for all case long and short, otherwise its not going to be clear. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://r

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

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 309869. MyDeveloperDay added a comment. Address review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92753/new/ https://reviews.llvm.org/D92753 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h

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

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 7 inline comments as done. MyDeveloperDay added a comment. > Should this be controlled on a case by case basis, maybe control indentation > using a regex over the pragma arguments, WDYT? Most uses of pragmas seem to be at the 0'th level of scrope `#pragma once` etc Most ot

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

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I tend to agree with @krasimir I don't see where you really use Maximum to mean anything, the nested configuration seems perhaps unnecessarily confusing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92257/new/ http

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

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:893 /*BeforeLambdaBody=*/false, + /*BeforeStructInitialization=*/false, /*BeforeWhile=*/false, an

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

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D92753#2436852 , @curdeius wrote: > You've added `messUp` parameter to `verifyFormat`, because, IIUC, pragmas > wouldn't be at the desired scope level indentation otherwise. messUp was pull the line below onto the pragm

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

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 309896. MyDeveloperDay added a comment. Add for(...) test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92753/new/ https://reviews.llvm.org/D92753 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h c

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

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 309926. MyDeveloperDay added a comment. Allow the tests to messUp() the code, turns out its some sort of interaction ONLY with the BeforeHash case (last test) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92753/new/ https://reviews.llvm.org/

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

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:979 +ContinuationIndent += Style.ContinuationIndentWidth; const FormatToken *PreviousNonComment = Current.getPreviousNonComment(); const FormatToken *NextNonComment = Previous.ge

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

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Also how does this relate to the `AfterStruct` setting? bool AfterStruct Wrap struct definitions. true: struct foo { int x; }; false: struct foo { int x; }; CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91949/new/ https://re

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. What does `the behavior goes wrong` mean? why can't it be left aligned? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91245/new/ https://reviews.llvm.org/D91245 ___ cfe-co

[PATCH] D90238: Update to D31635

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2832 (!Left.is(TT_PointerOrReference) || - (Style.PointerAlignment != FormatStyle::PAS_Right && + (getTokenPointerAlignment(Left) != FormatStyle::PAS_Right && !Line.I

[PATCH] D90238: [clang-format] Added ReferenceAlignmentStyle option - (Update to D31635)

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Looks almost there, just a few nits really Comment at: clang/docs/ClangFormatStyleOptions.rst:2316 +**ReferenceAlignment** (``ReferenceAlignmentStyle``) + Reference alignment style (overrides ``PointerAlignment`` for Did you g

[PATCH] D90238: [clang-format] Added ReferenceAlignmentStyle option - (Update to D31635)

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @catskul I'm really sorry this disappeared into the ether because it was missing the project and any real reviewers, I tend to go back now and again and do a search for "Lost" clang format reviews and stumbled on this one, hopefully this will bring it to the fore

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

2020-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'm a little confused between BestFit and Compact I'm not a massive fan of changing unit tests, just saying. Comment at: clang/docs/ClangFormatStyleOptions.rst:1473 -**ConstructorInitializerAllOnOneLineOrOnePerLine** (``bool``) - If the const

[PATCH] D89464: Revert "[clang-format] Fix AlignConsecutive on PP blocks"

2020-10-16 Thread MyDeveloperDay via Phabricator via cfe-commits
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/D89464/new/ https://reviews.llvm.org/D89464 _

[PATCH] D89709: [clang-format] Drop clangFrontend dependency for FormatTests

2020-10-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM, I'm for anything that speeds up the build Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89709/new/ https://reviews.llvm.or

[PATCH] D88239: [clang-format] Fix spaces around */& in multi-variable declarations

2020-10-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added subscribers: sammccall, djasper, klimek. MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:6636 - verifyFormat("a *a = aaa, *b = bbb,\n" - " *b = bb

[PATCH] D88239: [clang-format] Fix spaces around */& in multi-variable declarations

2020-10-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think this patch needs rebasing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88239/new/ https://reviews.llvm.org/D88239 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D88956: [clang-format] Fix misformatted macro definitions after D86959

2020-10-21 Thread MyDeveloperDay via Phabricator via cfe-commits
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/D88956/new/ https://reviews.llvm.org/D88956 _

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-10-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 300489. MyDeveloperDay added a comment. Rebase the patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79773/new/ https://reviews.llvm.org/D79773 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/c

[PATCH] D90246: [clang-format] Improve BAS_DontAlign+AllowAllArgumentsOnNextLine=false

2020-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This LGTM actually, and something that's been annoying me for ages that it breaks like this Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90246/new/ https://reviews.llvm.org/D90246 _

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

2020-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @thakis didn't you and I go through this process of removing frontEnd once before? do we really want to add this back in for all the reasons you said before? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90121/new/

[PATCH] D90534: [clang-format] Add new option PenaltyIndentedWhitespace

2020-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. 1. This requires unit tests 2. This requires a fill context diff 3. This requires changes to the ClangFormatStyleOptions.rst by running the dump_style tool in clang/do

[PATCH] D90533: [clang-format] Always consider option PenaltyBreakBeforeFirstCallParameter

2020-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. You need to supply full context diffs Comment at: clang/unittests/Format/FormatTest.cpp:4481 Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign; + Style.PenaltyBreakBeforeFirstCallParameter = 0; Style.AlignOperands = FormatStyle::OAS

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

2020-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @dmikis just for history sake I introduce the frontEnd when doing D68554: [clang-format] Proposal for clang-format to give compiler style warnings , D69854: [clang-format] [RELAND] Remove the dependency on frontend

[PATCH] D93626: [clang-format] PR48535 clang-format Incorrectly Removes Space After C Style Cast When Type Is Not a Pointer

2021-05-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This fix seems to cause a regression. (https://bugs.llvm.org/show_bug.cgi?id=50326) causing ColumLimit to not be observed in the following example size_t foo = (*(function))( F, Bar, F, Ba, FoLong, BaLong, Foo

[PATCH] D102392: [clang-format] PR50326 AlignAfterOpenBracket AlwaysBreak does not keep to the ColumnLimit

2021-05-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks. MyDeveloperDay added projects: clang-format, clang. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=50326 D93626: [clang-format] PR48535 clang-format Incorre

[PATCH] D102392: [clang-format] PR50326 AlignAfterOpenBracket AlwaysBreak does not keep to the ColumnLimit

2021-05-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D102392#2756779 , @curdeius wrote: > LGTM. Not blocking, but I'd add foo4 example from the description as a test > case (with a link to the bug maybe). Of course I should have added that too! let me do that! Reposito

[PATCH] D102392: [clang-format] PR50326 AlignAfterOpenBracket AlwaysBreak does not keep to the ColumnLimit

2021-05-15 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 345619. MyDeveloperDay added a comment. Add a test that shows it no longer impacts function ptrs CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102392/new/ https://reviews.llvm.org/D102392 Files: clang/lib/Format/TokenAnnotator.cpp clang/

[PATCH] D102392: [clang-format] PR50326 AlignAfterOpenBracket AlwaysBreak does not keep to the ColumnLimit

2021-05-15 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 345620. MyDeveloperDay added a comment. slightly closer test to the original failure CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102392/new/ https://reviews.llvm.org/D102392 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Fo

[PATCH] D102392: [clang-format] PR50326 AlignAfterOpenBracket AlwaysBreak does not keep to the ColumnLimit

2021-05-15 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGeae445f65d07: [clang-format] PR50326 AlignAfterOpenBracket AlwaysBreak does not keep to the… (authored by MyDeveloperDay). Repository: rG LLVM Gi

[PATCH] D102392: [clang-format] PR50326 AlignAfterOpenBracket AlwaysBreak does not keep to the ColumnLimit

2021-05-15 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This is also the cause of https://bugs.llvm.org/show_bug.cgi?id=49995, we might want to ask for this to be put into any 12.0.1 patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102392/new/ https://reviews.llvm.org

[PATCH] D89425: [Format/ObjC] Add NS_SWIFT_NAME() and CF_SWIFT_NAME() to WhitespaceSensitiveMacros

2021-05-15 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. After this change, this FOUNDATION_EXPORT NSString *const kFIRInstanceIDScopeFirebaseMessaging NS_SWIFT_NAME(InstanceIDScopeFirebaseMessaging) DEPRECATED_ATTRIBUTE; typedef NS_ENUM(NSInteger, FIRHTTPMethod) { /** HTTP Method GET */ FIRHTTPMetho

[PATCH] D102730: [clang-format] Support custom If macros

2021-05-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ReleaseNotes.rst:252 +- Option ``IfMacros`` has been added. This lets you define macros that get + formatted like conditionals much like ``ForEachMacros`` get stiled like + foreach loops. stiled? did

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

2021-05-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Is ```Signature``` effectively as is? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102706/new/ https://reviews.llvm.org/D102706 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

2021-05-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Could you clang-format the tests please so I can more easily read them. Comment at: clang/docs/ClangFormatStyleOptions.rst:2790 +**LambdaBodyIndentation** (``LambdaBodyIndentationKind``) + What is the lambda body indented relative to (default

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This looks like a good start.. All your tests are 3x3 have you considered mixing it up a bit. i.e. 2x3, what is the impact on 1x5 and 5x1 ? Also how about nested structs, I'm interested to see what happens {56, 23, { "ABC", 35 }} {57, 24, { "DEF", 36 }} R

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This is a valiant effort, and I definitely don't want to discourage..especially as you shown a good understand of clang-format. Its been a long asked for feature.. but we should really ensure this covers the bulk of the complaints if possible.. For me I use this

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-05-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:16389 + for (const auto &Style : Styles) { +verifyFormat("struct test demo[] = {\n" + "{56, 23,\"hello\" },\n" may be worth testing a case with co

[PATCH] D102730: [clang-format] Support custom If macros

2021-05-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a subscriber: krasimir. MyDeveloperDay added a comment. This kind of looks ok to me, I'm not sure about the "lexer" part, I guess I don't know the impact but I'd expect it to be small because it's conditional on your IfMacro I'd like @curdeius or @krasimir to take a look.

[PATCH] D102706: [clang-format] Add new LambdaBodyIndentation option

2021-05-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I personally like to keep clang-format and clang-format tests clang formatted all the time so we have a reasonable set of "clang-format-clean" code we can sanity check any change against, the more of LLVM that can be clean the better our testing can be. ==

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

2021-05-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. When you made this commit you didn't regenerate the ClangFormatStyleOptions.rst from the Format.h using clang/docs/tool/dump_format_style.py now anyone else using the tool hit issues with code which is incorrect, we need to update Format.h to match what you expec

[PATCH] D101344: [clang-format] Add `SpacesInAngles: Leave` option to keep spacing inside angle brackets as is.

2021-05-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Whoops, We forgot to regenerate ClangFormatStyleOptions.rst here following the change to Format.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101344/new/ https://reviews.llvm.org/D101344 _

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

2021-05-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This patch still only considers structs? @krasimir and I are I think saying that really this should it support `class` in the same way? I mean isn't a struct and a class almost identical in this context why would we want treat them differently. class new_str

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

2021-05-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:19068 +" call_function(arg, arg);"; + verifyFormat(IfSourceShort); + Can you just put the code inline in the verifyFormat, please copy the conve

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

2021-05-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2546 +**InsertBraces** (``BraceInsertionStyle``) + The brace insertion style to use for control flow statements. I'm not convinced this code has been generated via doc/to

[PATCH] D102989: Align documentation in Format.h and ClangFormatStyleOptions.rst for EmptyLineAfterAccessModifier

2021-05-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102989/new/ https://reviews.llvm.org/D102989 ___ cfe-commits mailing l

[PATCH] D103204: [clang-format] New BreakInheritanceList style AfterComma

2021-05-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Adding such a feature like this needs unit tests, it won't be let in without them, you need to add them to clang/unittest/Format/Format.cpp You also need to update th

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

2021-05-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Are we happy with the way this behaves? I realise some of this needs semantic information but I think the fact it only handles 1 of these 4 cases leaves me feeling a little cold. (I just don't want to be having to defend this as to why it doesn't work in all case

[PATCH] D74265: [clang-format] Improve handling of C# attributes

2021-05-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This issue seems to have caused a regression https://bugs.llvm.org/show_bug.cgi?id=50515, I understand we removed the rule because it was too aggressive, now its not aggressive enough. I'm going to try and find some sort of compromise so we can gravitate to a so

[PATCH] D103307: [clang-format] successive C# attributes cause line breaking issues

2021-05-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: jbcoe, krasimir, HazardyKnusperkeks, curdeius. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. D74265: [clang-format] Improve handling of C# attributes

[PATCH] D74265: [clang-format] Improve handling of C# attributes

2021-05-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. The second property in a class is putting the property on the same line [XmlIgnore] public string property1 { get; set; } [XmlIgnore] public string property2{ get; set; } as are all successive properties Multiple properites are also not getting broken u

[PATCH] D103307: [clang-format] successive C# attributes cause line breaking issues

2021-05-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D103307#2786645 , @jbcoe wrote: > Thanks for this! I tried to handle the multiple attributes using the "parsing technique" you used, instead of the "mustBreaking technique" but every time I do this I end up putting a

[PATCH] D103307: [clang-format] successive C# attributes cause line breaking issues

2021-05-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 348495. MyDeveloperDay added a comment. Add additional unit test, and an additional check to ensure multiple attributes are broken, while @jbcoe this looks very similar to the original rule, the original rule actually had an || in the expression and n

[PATCH] D103204: [clang-format] New BreakInheritanceList style AfterComma

2021-05-28 Thread MyDeveloperDay via Phabricator via cfe-commits
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/D103204/new/ https://reviews.llvm.org/D103204 ___

[PATCH] D103307: [clang-format] successive C# attributes cause line breaking issues

2021-05-29 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGffb48d48e45c: [clang-format] successive C# attributes cause line breaking issues (authored by MyDeveloperDay). Changed prior to commit: https://reviews.llvm.org/D103307?vs=348495&id=348634#toc Reposito

[PATCH] D103286: [clang-format] Add PPIndentWidth option

2021-05-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:3447 + style); +} + can you add a test ``` #ifdef X void foo() { ... } #endif ``` its unclear if PPIndentWidth affects code in #ifdef or just # pr

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

2021-05-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. We can't rename options without giving some form of backwards compatibility, personally, I think we need to start again with this review if its still of interest, this one is 6 years old. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

2021-06-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > A small task that will make a lot of people happy Its not the happy people that concern me, its those who end up not happy that cause us the most pain ;-) 1. I question if the rst was generated via docs/tools/dump_format_style.py or by hand, it has to be gener

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

2021-06-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:14921 + verifyFormat("unsigned int *a;\n" + "int*b;\n" "unsigned int Const *c;\n" I seem to remember in the past there was

[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

2021-06-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. It maybe that others think that the PAS_Right IS that option. If thats is the case I'm fine with that too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103245/new/ https://reviews.llvm.org/D103245

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

2021-01-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a subscriber: djasper. MyDeveloperDay added a comment. I think this is one of those reviews that ultimately I think would be useful if we could ensure it works 100% correctly, but I think it goes against the original ethos of clang-format and I think if @djasper or @klimek t

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

2021-01-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:17921 + " another_function(arg, arg, arg, arg, arg, arg);"; + EXPECT_EQ(IfElseSourceLong, format(IfElseSourceLong, Style)); + can you use verifyFormat() instead of EXPE

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

2021-01-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:13548 " {\n" "class A\n" " {\n" you are removing the test with 1 namespace nesting Repository: rG LLVM Github Monorepo C

[PATCH] D94955: [clang-format] Treat ForEachMacros as loops

2021-01-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:995 TEST_F(FormatTest, ForEachLoops) { verifyFormat("void f() {\n" + " foreach (Item *item, itemlist) {\n" I'd like you to assert that short loops are off

[PATCH] D95017: [clang-format] add case aware include sorting

2021-01-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2039 +**IncludeSortCaseAware** (``bool``) + Specify if sorting should be done in a case aware fashion. curdeius wrote: > The name is somewhat awkward IMO (in the sense it

[PATCH] D95017: [clang-format] add case aware include sorting

2021-01-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2286 +**IncludeSortAlphabetically** (``bool``) + Specify if sorting should be done in an alphabetical and + case sensitive fashion. Are you sure `IncludeSortAlphabetically`

[PATCH] D95017: [clang-format] add case aware include sorting

2021-01-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:2286 +**IncludeSortAlphabetically** (``bool``) + Specify if sorting should be done in an alphabetical and + case sensitive fashion. curdeius wrote: > kentsommer wrote: > >

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

2021-01-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM I think, it would be good to get input from other reviewers Comment at: clang/unittests/Format/FormatTest.cpp:13696 " }\n" -

[PATCH] D95479: [clang-format] Avoid considering include directive as a template closer.

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

<    6   7   8   9   10   11   12   13   14   15   >