[PATCH] D127873: [clang-format] Fix misplacemnt of `*` in declartion of pointer to struct

2022-06-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:93 + EXPECT_EQ(Tokens.size(), 10u) << Tokens; + EXPECT_TOKEN(Tokens[6], tok::star, TT_PointerOrReference); } HazardyKnusperkeks wrote: > MyDeveloperDay wrote: > > C

[PATCH] D127270: [clang-format] Add space in placement new expression

2022-06-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:3495 +/// \code +///true: false: +///new (buf) T;vs.new(buf) T; should this be `Always/Never`

[PATCH] D127873: [clang-format] Fix misplacemnt of `*` in declaration of pointer to struct

2022-06-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2328 +} + if (PrevToken->Tok.isLiteral() || Thank you I wish more of the clauses were commented like this CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127873

[PATCH] D127270: [clang-format] Add space in placement new expression

2022-06-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/tools/dump_format_style.py:317 pass + elif state == State.InNestedEnum: +if line.startswith('///'): Can you show us a screenshot of how these changes will look in HTML? Reposit

[PATCH] D127270: [clang-format] Add space in placement new expression

2022-06-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. Thanks for the Screenshot, please remember to mark your comments as "Done" once you've addressed them. From what I can tell this looks good. Comment at: clan

[PATCH] D126758: [clang-format] Handle do-while loops for RemoveBracesLLVM

2022-06-01 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/D126758/new/ https://reviews.llvm.org/D126758 ___ cfe-commits mailing list cfe-com

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

2022-06-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @stasm thank you for taking it on, "Commandeer" the revision via the Revision actions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116638/new/ https://reviews.llvm.org/D116638 ___ cfe-commits mailing list cf

[PATCH] D127005: [clang-format][NFC] Clean up the unwrapped line parser

2022-06-06 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 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127005/new/ https://reviews.llvm.org/D127005 ___ cfe-commits mailing li

[PATCH] D126845: [clang-format] Handle Verilog numbers and operators

2022-06-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/FormatToken.h:1417 // Workaround for hashes and backticks in Verilog. IdentifierInfo *verilogHash; IdentifierInfo *verilogHashHash; maybe `kw_verilogHash` and `kw_verilogHashHash` and `kw

[PATCH] D127054: [clang-format] Handle attributes for for/while loops

2022-06-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127054/new/ https://reviews.llvm.org/D127054 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D155239: [clang-format] Add SpacesInParens with SpacesInParensOptions

2023-07-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:1035 IO.mapOptional("SpaceInEmptyBlock", Style.SpaceInEmptyBlock); -IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses); IO.mapOptional("SpacesBeforeTrailingComments",

[PATCH] D154755: [clang-format] Fix formatting of if statements with BlockIndent

2023-07-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:25486 "comment\n" - " return;\n" "}", Why remove? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D155529: [clang-format] Add SpaceInParensOption for __attribute__ keyword

2023-07-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. We should never make assumptions about what people do/don't use F28299562: image.png If you have this you have to honour it.. if 'SpacesInParentheses ' was true then 'InAttributeSpecifiers' needs to be true by default, shouldn

[PATCH] D154184: [clang-format] Correctly annotate */&/&& in operator function calls

2023-07-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D154184#4509744 , @rymiel wrote: > Shouldn't that regression already be handled by D155358 > ? nice catch @rymiel! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D155529: [clang-format] Add SpaceInParensOption for __attribute__ keyword

2023-07-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. We should never make assumptions about what people do/don't use F28299562: image.png If you have this you have to honour it.. if 'SpacesInParentheses ' was true then 'InAttributeSpecifiers' needs to be true by default, shouldn

[PATCH] D155273: [clang-format] Add TypeNames option to disambiguate types/objects

2023-07-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155273/new/ https://reviews.llvm.org/D155273 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D155239: [clang-format] Add SpacesInParens with SpacesInParensOptions

2023-07-18 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 looks good can you wait for one of the others to accept too Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155239/new/ https

[PATCH] D154755: [clang-format] Fix formatting of if statements with BlockIndent

2023-07-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. It looks like this was added in D109557: Adds a BlockIndent option to AlignAfterOpenBracket I'm trying to understand what bad stuff happens if you remove the "if" from `return !(Previous && Previous->is(tok::kw_for));` Reposit

[PATCH] D154755: [clang-format] Fix formatting of if statements with BlockIndent

2023-07-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: clang/unittests/Format/FormatTest.cpp:25504 + +#if 0 verifyFormat("if (quitelongarg !=\n" we don't do this. ==

[PATCH] D155273: [clang-format] Add TypeNames option to disambiguate types/objects

2023-07-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I used isSimpleTypeSpecifier() in the east/west const fixer, I think this could probably help there to catch more places too.. (you don't need to do that here) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155273/new

[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

2023-07-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D138263#3936536 , @HazardyKnusperkeks wrote: > In D138263#3936007 , @owenpan wrote: > >> I suppose it's fairly easy to annotate the `l_brace` of a namespace? If so, >> then wou

[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

2023-08-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTestCSharp.cpp:221 TEST_F(FormatTestCSharp, CSharpFatArrows) { - verifyFormat("Task serverTask = Task.Run(async() => {"); + verifyIncompleteFormat("Task serverTask = Task.Run(async() => {"); veri

[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

2023-08-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. I'm ok with this if @owenpan is Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154093/new/ https://reviews.llvm.org/D154093 ___ cfe-c

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

2023-07-03 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG899c86779440: [clang-format] Fixed bad performance with enabled qualifier fixer. (authored by Sedeniono, committed by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D154091: [clang-format] Recognize escape sequences when breaking strings

2023-07-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. What would happen in a \r\n example Comment at: clang/lib/Format/BreakableToken.cpp:207 + case 't': + case 'v': +AfterSpace = SplitPoint + 2; Are you testing \r \v \f Repository: rG LLVM Github Monorepo CHA

[PATCH] D154467: [clang-format] Add Verilog suffixes to the scripts

2023-07-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. What would happen for say unicode escapes? x = "XXXThis is a string \u0041 With stuff after"; would it break after the \u? x = "XXXThis is a string \u 0041 With stuff after"; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D154484: [clang-format] Add an option to remove redundant parentheses

2023-07-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. A thousand times yes! Comment at: clang/docs/ClangFormatStyleOptions.rst:4354 + + * ``RPS_None`` (in configuration: ``None``) +Do not remove parentheses.

[PATCH] D154091: [clang-format] Recognize escape sequences when breaking strings

2023-07-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. What would happen for say unicode escapes? x = "XXXThis is a string \u0041 With stuff after"; would it break after the \u? x = "XXXThis is a string \u 0041 With stuff after"; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D154091: [clang-format] Recognize escape sequences when breaking strings

2023-07-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/BreakableToken.cpp:199 +if (EscapeSequence && Advance == 2) { + switch (Text[1]) { Can we add a unit test for escape sequences > \X which I assume this handles Repository: rG LLVM G

[PATCH] D154091: [clang-format] Recognize escape sequences when breaking strings

2023-07-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/BreakableToken.cpp:199 +if (EscapeSequence && Advance == 2) { + switch (Text[1]) { sstwcw wrote: > MyDeveloperDay wrote: > > Can we add a unit test for escape sequences > \X which I ass

[PATCH] D154484: [clang-format] Add an option to remove redundant parentheses

2023-07-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Here is a potential failure scenario (test.cpp) void *bar(); void foo() { void *p; int i = 0; while ((p = bar())) { i++; } } (.clang-format) Language: Cpp RemoveParentheses: ReturnStatement clang-format test.cpp > test2

[PATCH] D154484: [clang-format] Add an option to remove redundant parentheses

2023-07-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I reckon you could detect the `=` condition between the matching paren don't you? I ran this on my quite large code base and this was one of the only issues I've seen thus far.. This is a great feature, for my codebase this generated changes in about 10% of all

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

2023-07-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I don't have a problem with this, but I'm not an ObjC person so I can't really review from the validity of what its changing. If @owenpan and @HazardyKnusperkeks don't have an problem with this then I don't either as it likely won't impact most C++ users.

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

2023-07-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. may remove the other build system files, I don't think its our responsiblity to keep them upto date. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 ___ cfe-commits m

[PATCH] D155239: [clang-format] Add SpacesInParens with SpacesInParensOptions

2023-07-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:4046 + ? (Style.SpacesInParens == FormatStyle::SIPO_Always || + Style.SpacesInParentheses) + : true; isn't SpacesInParentheses mappe

[PATCH] D159051: [clang-format][NFC] Change EXPECT_EQ to verifyFormat or verifyNoChang

2023-08-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. +1 to this before we goto github Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159051/new/ https://reviews.llvm.org/D159051 ___ cfe-

[PATCH] D159233: [clang-format][NFC] Change duplicate config files to symlinks

2023-09-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. out of interest what happens on windows? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159233/new/ https://reviews.llvm.org/D159233 ___ cfe-commits mailing list cfe-commit

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

2020-03-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @MarcusJohnson91 To get a review past its better that you mark the comments as done so the reviewers know when the comments have been addressed and not missed. (this is important as the number of comments grows) Developers need to explain why they haven't changed

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

2020-03-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > Not sure if tagging is considered rude, I figure that @MyDeveloperDay's > notification fell off your radar. Definitely not rude from my perspective...perfectly happy to come and look and be tagged to get my attention. If I'm ignoring its, its only becuase I'm b

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

2020-03-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:2468 "}"); - verifyFormat("extern \"C\" int foo() {}"); - verifyFormat("extern \"C\" int foo();"); - verifyFormat("extern \"C\" int foo() {\n" + verifyFormat("extern \"C\"

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

2020-03-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1088 if (FormatTok->Tok.is(tok::l_brace)) { -if (Style.BraceWrapping.AfterExternBlock) { - parseBlock(/*MustBeDeclaration=*/true); -} else { +if (Sty

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-03-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. is there overlap here D76197: clang-format: Use block indentation for braced initializations CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75034/new/ https://reviews.llvm.org/D75034 ___

[PATCH] D76621: [clang-format] No space inserted between commas in C#

2020-03-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. nice, thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76621/new/ https://reviews.llvm.org/D76621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-03-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I want to believe that it was a mistake on the original developers part, but I just can't tell. A search of github shows people using this in their repos https://github.com/search?q=ForContinuationAndIndentation&type=Code I almost feel unless the original author

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

2020-03-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:2440 TEST_F(FormatTest, FormatsExternC) { - verifyFormat("extern \"C\" {\nint a;"); - verifyFormat("extern \"C\" {}"); + verifyFormat("extern \"C\" {\nint a; /*2.1*/"); + verifyFormat("ex

[PATCH] D76850: clang-format: Fix pointer alignment for overloaded operators (PR45107)

2020-03-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 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76850/new/ https://reviews.llvm.org/D76850 ___ cfe-commits mailing li

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

2020-03-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Could the default be `Style.IndentExternBlock = Style.BraceWrapping.AfterExternBlock` Then it would match the prior behaviour off AddLevel being `true` when AfterExternBlock is `true` extern "C" { int a; } extern "C" { int a; } CHANGES SINC

[PATCH] D76197: clang-format: Use block indentation for braced initializations

2020-03-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. > New implementation that breaks fewer tests. `fewer` is not an option, `no` is all thats going to get accepted. Please add tests for you own use case CHANGES SINCE

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

2020-03-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D33029#1944919 , @bbassi wrote: > @MyDeveloperDay Is someone working on fixing the breaking tests and merging > it? I need this feature so if someone isn't working on it already, I can take > it. as @stringham isn't w

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-04-02 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, thank you Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75034/new/ https://reviews.llvm.org/D75034 ___

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

2020-04-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I have no concerns with this patch other than the consideration of the defaults. My concern is whatever our choice we will generate churn on existing code where the .clang-format file has AfterExternBlock = true (or its true in inbuilt BasedOnStyle type) https:

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

2020-04-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > Ok, I've got an idea to deprecate the AfterExternBlock option and map it to a > new option Really? can't you just model IndentExternBlock as an enum? then say bool shouldIndent = Style.IndentExternBlock==Indented || ( Aft

[PATCH] D75006: Wrap lines for C# property accessors

2020-02-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:443 +if (GetToken && +GetToken->isOneOf(tok::kw_public, tok::kw_protected, tok::kw_private)) + GetToken = GetToken->Next; I think we need to consider `i

[PATCH] D75006: Wrap lines for C# property accessors

2020-02-24 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, thank you for the patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75006/new/ https://reviews.llvm.org/D75006

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-02-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. If you are changing unit tests then I'm nervous, imagine if this many tests change what the impact is on a large code base. I think we need to understand more about why you are making this change and why you think its ok to change/remove existing tests which ass

[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.

2020-02-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. You need to add unit tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75022/new/ https://reviews.llvm.org/D75022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailm

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-02-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. you need to add any diff as a full context diff (normally by adding -U 999) to the diff command Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75034/new/ https://reviews.llvm.org/D75034 __

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-02-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:10293 Tab)); - EXPECT_EQ("/*\n" -"\t a\t\tcomment\n" Sorry this isn't clear to be which test this is a duplicate of, as this is an examp

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-02-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added subscribers: djasper, klimek. MyDeveloperDay added a comment. > Is this intentional, or is it an oversight? This is the key question, which I'm not sure how we can answer without the help of the original authors @djasper and @klimek For example I can't understand with the e

[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.

2020-02-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think its look good, please add the extra tests, then lets give people a couple of days Comment at: unittests/Format/FormatTest.cpp:570 + AllowsMergedLoops); } horrible code though they are, could you add a te

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-02-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I tend to agree with your assessment, I think its worth waiting for some others to comment. Comment at: clang/unittests/Format/FormatTest.cpp:10293 Tab)); - EXPECT_EQ("/*\n" -"\t a\t\tcomment\n"

[PATCH] D75006: [clang-format] Wrap lines for C# property accessors

2020-02-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM Comment at: clang/unittests/Format/FormatTestCSharp.cpp:246 - "get;\n" - "}"); Nit: I feel like this layout should really be an option, (maybe for the

[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.

2020-02-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, thank you for adding the extra test, please mark the inline comments as done CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75022/new/ https://reviews.llvm.org/D

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2020-02-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72326/new/ https://reviews.llvm.org/D72326 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D75410: [clang-format] Fixed BraceWrapping.AfterExternBlock

2020-03-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. This needs a full context diff, before we can even look at it properly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7

[PATCH] D75465: [clang-format] Do not merge target-name and : for C# attributes

2020-03-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This feels better than merging I think visual studio tends to create files via the new project wizard that do not have a space before but do have a space after the `:` [assembly: ComVisible(false)] CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75465/new

[PATCH] D75194: [clang-format] Do not merge very long C# automatic properties

2020-03-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @krasimir, well I learned something new, so thanks for taking the time to explain that to help improve our understanding. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75194/new/ https://reviews.llvm.org/D75194 _

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 226961. MyDeveloperDay added a comment. Trying to address review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69573/new/ https://reviews.llvm.org/D69573 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTe

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 4 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1347 Contexts.back().CanBeExpression = false; -} else if (Current.isOneOf(tok::semi, tok::exclaim)) { +} else if (Current.isOneOf(

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D69577#1726587 , @lichray wrote: > Should we find a way to set `->`'s type to `TT_TrailingReturnArrow`? that's possible then we might be able to use the existing spaces before rule if (Right.isOneOf(TT_TrailingReturn

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 227062. MyDeveloperDay added a reviewer: lichray. MyDeveloperDay added a comment. Detect deduction guides arrow as a TrailingReturn arrow, allowing use of existing space before/after rules CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69577/n

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 227083. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Extend this revision to cover additional https://bugs.llvm.org/show_bug.cgi?id=43783 issue (which has overlap) New revision correctly formats the following code:

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2090 continue; + if (Next->isOneOf(tok::star, tok::arrow)) +continue; sammccall wrote: > MyDeveloperDay wr

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 4 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2626 + Left.Previous && Left.Previous->is(tok::kw_operator)) +// No space between the type and the * +// operator void*(), operator

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 227109. MyDeveloperDay added a comment. Remove test regression CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69573/new/ https://reviews.llvm.org/D69573 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp Ind

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:955 CurrentToken->Previous->isOneOf(TT_BinaryOperator, TT_UnaryOperator, -tok::comma))

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a subscriber: hans. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1400-1403 +} else if (Current.Previous && Current.Previous->is(tok::r_paren) && + Curr

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Sorry ignore my last results, let me rerun, I was using the revision without the change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69577/new/ https://reviews.llvm.org/D69577 ___ cfe-commits mailing list

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 4 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1400-1403 +} else if (Current.Previous && Current.Previous->is(tok::r_paren) && + Current.startsSequence(tok::arrow, tok::ident

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 227166. MyDeveloperDay added a comment. move detection of deduction guides into a function, add additional negative tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69577/new/ https://reviews.llvm.org/D69577 Files: clang/lib/Format/Tok

[PATCH] D14927: clang-format: Add SpaceBeforeTemplateParameterList option

2019-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Style guide suggesting enforcing a space after template keyword https://mesos.readthedocs.io/en/1.1.0/c++-style-guide/ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14927/new/ https://reviews.llvm.org/D14927 ___

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 227241. MyDeveloperDay marked an inline comment as done. MyDeveloperDay set the repository for this revision to rC Clang. MyDeveloperDay added a comment. Address review comments, deduction guides with embedded parens Repository: rC Clang CHANGES SI

[PATCH] D14927: clang-format: Add SpaceBeforeTemplateParameterList option

2019-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: include/clang/Format/Format.h:500 + /// bracket of a template parameter list. + bool SpaceBeforeTemplateParameterList; + I suggest we make this an enumeration ``` enum { Unaltered, Never, Always, } ``` Introdu

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "&ref" Parameter

2019-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thanks for the patch, sorry I must have missed this coming in. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69649/new/ https://reviews.ll

[PATCH] D33944: git-clang-format: Add --cached option to format index

2019-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Is this revision still relavant, I find it annoying that I have to run git add again on any files that are already added but have been formatted. If you think this is useful and can rebase it perhaps we can go around the review cycle again with a fresh set of eye

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1371 +if (Current.Previous && Current.Previous->is(tok::r_paren) && +Current.startsSequence(tok::arrow, tok::identifier, tok::less)) { + // Find the TemplateCloser.

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

2019-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 227317. MyDeveloperDay marked 4 inline comments as done. MyDeveloperDay added a comment. Add additional test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69577/new/ https://reviews.llvm.org/D69577 Files: clang/lib/Format/TokenAnnotat

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-11-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 5 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2105 + if ((Next->isSimpleTypeSpecifier() || Next->is(tok::identifier)) && + Next->Next && Next->Next->is(tok::star)) { +//

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

2019-11-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 227405. MyDeveloperDay set the repository for this revision to rG LLVM Github Monorepo. MyDeveloperDay added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69573/new/ ht

[PATCH] D14927: clang-format: Add SpaceBeforeTemplateParameterList option

2019-11-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This I believe is covered by `SpaceAfterTemplateKeyword` which does the same thing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14927/new/ https://reviews.llvm.org/D14927 ___ cfe-commits mailing list cfe-co

[PATCH] D69764: [clang-format] Add East Const / West Const fixer

2019-11-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, mitchell-stellar, sammccall, owenpan, krasimir. MyDeveloperDay added a project: clang-format. Herald added a subscriber: mgorny. Herald added a project: clang. Developers these days seem to argue over east vs west const

[PATCH] D69764: [clang-format] Add East Const / West Const fixer

2019-11-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 227637. MyDeveloperDay added a comment. Support for template object const type declarations CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 Files: clang/docs/ClangFormatStyleOptions.rst clang/incl

[PATCH] D69764: [clang-format] Add East Const / West Const fixer

2019-11-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. A demo of `clang-format -i --const-style=east *.cpp *.h` being run on clang/lib/Format can be see here D69780: [clang-format] DO NOT COMMIT - Demo of East/West Const Fixer on clang-format CHANGES SINCE LAST ACTION https://rev

[PATCH] D69764: [clang-format] Add East Const / West Const fixer

2019-11-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:768 LLVMStyle.CommentPragmas = "^ IWYU pragma:"; + LLVMStyle.ConstStyle = FormatStyle::CS_Leave; LLVMStyle.CompactNamespaces = false;

[PATCH] D69764: [clang-format] Add East Const / West Const fixer

2019-11-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:768 LLVMStyle.CommentPragmas = "^ IWYU pragma:"; + LLVMStyle.ConstStyle = FormatStyle::CS_Leave; LLVMStyle.CompactNamespaces = false;

[PATCH] D69752: clang-format: Add a fallback style to Emacs mode

2019-11-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. I'm not an Emacs user, but this LGTM as much as I can read the rest of the file. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69752/new/ https

[PATCH] D69764: [clang-format] Add East Const / West Const fixer

2019-11-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. An example of even how setting the style to be West will generate new changes on a previously formatted lib/Format $ git diff . diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp old mode 100644 new mode 100755

[PATCH] D69764: [clang-format] Add East Const / West Const fixer

2019-11-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a subscriber: pollydev. MyDeveloperDay added a comment. @pollydev too (a previously clang-format clean directory) will generate changes running `clang-format --const-style=west -i -n *.cpp` ScopBuilder.cpp:74:9: warning: code should be clang-formatted [-Wclang-format-viol

[PATCH] D69764: [clang-format] Add East Const / West Const fixer

2019-11-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D69764#1732235 , @aaron.ballman wrote: > I like the functionality, but am slightly opposed to using "east/west" > terminology -- that's not a ubiquitous phrase and it takes a bit of thinking > before it makes sense. I

[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

2019-11-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 227708. MyDeveloperDay retitled this revision from "[clang-format] Add East Const / West Const fixer" to "[clang-format] Add Left/Right Const (East/West , Before/After) fixer capability". MyDeveloperDay edited the summary of this revision. MyDeveloperD

<    12   13   14   15   16   17   18   19   20   21   >