[PATCH] D93528: [clang-format] Add basic support for formatting JSON

2021-06-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. ping, if there are no other opinions I plan to land this sometime this weekend. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93528/new/ https://reviews.llvm.org/D93528 ___ cfe-commits mailing list cfe-commits@

[PATCH] D104900: [clang-format] PR50525 doesn't handle AlignConsecutiveAssignments correctly in some situations

2021-06-26 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 rGee3b2c47ce41: [clang-format] PR50525 doesn't handle AlignConsecutiveAssignments correctly in… (authored by MyDeveloperDay). Repository: rG LLVM Gi

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

2021-06-26 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG37c2233097ac: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and… (authored by MyDeveloperDay). Changed prior to commit: https://reviews.llvm.org/D104222?vs=352193&id=354671#toc

[PATCH] D93528: [clang-format] Add basic support for formatting JSON

2021-06-26 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8b7881a084d0: [clang-format] Add basic support for formatting JSON (authored by MyDeveloperDay). Changed prior to commit: https://reviews.llvm.org/D93528?vs=353570&id=354674#toc Repository: rG LLVM G

[PATCH] D104900: [clang-format] PR50525 doesn't handle AlignConsecutiveAssignments correctly in some situations

2021-06-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D104900#2842540 , @darwin wrote: > Sorry I haven't had a chance to look at this bug before it has closed. But I > do have a question: > > I observed that this code are formatted incorrectly by the same config: > > llv

[PATCH] D104388: [clang-format] PR50727 C# Invoke Lamda Expression indentation incorrect

2021-06-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104388/new/ https://reviews.llvm.org/D104388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

2021-06-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1270 !startsExternCBlock(*PreviousLine)) Newlines = 1; Nit:I do think at some point we need to have some sort of option for controlling these empty lines,

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

2021-06-27 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe5a8f230c765: [clang-format] Fix the issue that empty lines being removed at the beginning of… (authored by darwin, committed by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

2021-06-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @catskul can we now Abandon this revision seeing as D104096: [Clang-Format] Add ReferenceAlignment directive is landed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90238/new/ htt

[PATCH] D105087: [clang-format] PR49960 clang-format doesn't handle ASI after "return" on JavaScript

2021-06-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, mprobst. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=49960 clang-format can mutate legal javascript c

[PATCH] D105099: [clang-format] Add an option to put one constructor initializer per line

2021-06-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Seem similar to D90232: [clang-format] Formatting constructor initializer lists by putting them always on different lines (update to D14484) which seems to have got stalled I sort of feel I prefer the design where we have an enum

[PATCH] D105099: [clang-format] Add an option to put one constructor initializer per line

2021-06-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. We already have `AllowAllConstructorInitializersOnNextLine` and `ConstructorInitializerAllOnOneLineOrOnePerLine` Sort of feels like we need to combine them with this, I should also say I'd quite like this functionality, its just how do we deliver it via the opt

[PATCH] D105099: [clang-format] Add an option to put one constructor initializer per line

2021-06-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > is a smell that tells us to use an enum. I totally agree > Personally I find it harder to finder multiple options that concern the same > aspect of the code style rather than a single option. To be honest, I think we know the options some of the best (apart of

[PATCH] D104388: [clang-format] PR50727 C# Invoke Lamda Expression indentation incorrect

2021-06-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > I implemented a good fraction of the C# support and am confident that these > changes are an improvement. Thank you. I've been very grateful to you all (which is why I included you as reviewers) for filling in the gaps in the C# formatting, but I'm super grate

[PATCH] D104388: [clang-format] PR50727 C# Invoke Lamda Expression indentation incorrect

2021-06-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 355627. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Correct a couple of Nits prior to commit. (just updating patch for completeness) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104388/new/ https://review

[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-07-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. For the purposes of not checking a style we are not going to use and because I guess it might make clang-format startup, ever so slightly faster, I think its a reasonable change that likely does no harm. Repository: rG LL

[PATCH] D104388: [clang-format] PR50727 C# Invoke Lamda Expression indentation incorrect

2021-07-01 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf9937106b717: [clang-format] PR50727 C# Invoke Lamda Expression indentation incorrect (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D105087: [clang-format] PR49960 clang-format doesn't handle ASI after "return" on JavaScript

2021-07-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 355817. MyDeveloperDay added a comment. Move Peek next token into a function, add a comment for clarity CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105087/new/ https://reviews.llvm.org/D105087 Files: clang/lib/Format/UnwrappedLineParser

[PATCH] D105087: [clang-format] PR49960 clang-format doesn't handle ASI after "return" on JavaScript

2021-07-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1019 +FormatToken *NextNext = Tokens->getNextToken(); +Tokens->setPosition(StoredPosition); +if (NextNext && NextNex

[PATCH] D105087: [clang-format] PR49960 clang-format doesn't handle ASI after "return" on JavaScript

2021-07-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 355848. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay set the repository for this revision to rG LLVM Github Monorepo. MyDeveloperDay added a comment. Updating based on revision comments, but found that the indentation can be incorrec

[PATCH] D105408: [clang-format] Pass a TextDiagnosticPrinter when we can not create tempory file.

2021-07-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Including clangFrontend has been discourage many times before, we don't want to add that to the list of dependencies please see D90121: clang-format: Add a consumer t

[PATCH] D105408: [clang-format] Pass a TextDiagnosticPrinter when we can not create tempory file.

2021-07-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. also D68554: [clang-format] Proposal for clang-format to give compiler style warnings Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105408/new/ https://reviews.llvm.org/D105408 ___

[PATCH] D98429: [clang-format] Add new option to clang-format: SpaceBeforeForLoopSemiColon

2021-07-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Can you show `for (;;) {}` in your tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98429/new/ https://reviews.llvm.org/D98429 ___ cfe-commits mailing list cfe-commits

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

2021-07-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 356565. MyDeveloperDay removed reviewers: klimek, djasper. MyDeveloperDay added a comment. Rebase on clang12 as requested CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 Files: clang/docs/ClangForma

[PATCH] D105479: [clang-tidy] [PR50069] readability-braces-around-statements doesn't work well with [[likely]] [[unlikely]]

2021-07-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: alexfh, njames93, aaron.ballman. MyDeveloperDay added projects: clang-tools-extra, clang. Herald added subscribers: jdoerfert, xazax.hun. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=50

[PATCH] D105408: [clang-format] Pass a TextDiagnosticPrinter when we can not create tempory file.

2021-07-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Yeah, I made the almost identical change but got some push back on it extending the dependencies (I think it also meant that the clang-format target needed alot more source to be compiled) Dinking around in clangBasic is probably outside my pay grade ;-) Reposi

[PATCH] D105479: [clang-tidy] [PR50069] readability-braces-around-statements doesn't work well with [[likely]] [[unlikely]]

2021-07-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D105479#2862819 , @aaron.ballman wrote: > Thank you for working on this! So I looked into do what you suggested originally, but it became a little more involved, partially because you have to look inside the Attrbuted

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

2021-07-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. FYI, I'm not actually asking for this to be merged, its just I'm happy to keep rebasing it from time to time for others to use, but to do that I have to keep pushing new reviews, but if the general opinion was to put it in then I'd go for that. I knew it was go

[PATCH] D105479: [clang-tidy] [PR50069] readability-braces-around-statements doesn't work well with [[likely]] [[unlikely]]

2021-07-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 357229. MyDeveloperDay added a comment. Address the situation where we don't have braces after the `[[likely]]` on an if CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105479/new/ https://reviews.llvm.org/D105479 Files: clang-tools-extra/cl

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

2021-07-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. To be more sure of not breaking something we'd likely have to reduce the cases where tok::identifier was checked, it depends if "not catching every case" is caught is more acceptable. I certainly see that elsewhere in clang (like identifying where override is mis

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

2021-07-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D69764#2863648 , @owenpan wrote: > Has this been tested against a large code base? It also needs an unqualified > LGTM before it can be merged. D105701: [clang-format] test revision (NOT FOR COMMIT) to demonstrate east/

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

2021-07-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > Should any known failure modes be documented? At present, I don't know of any, I think I could probably trick it with macros but then that is #clang-format all over, if you are using it feel free to log bugs in the

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

2021-07-12 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. could you add tests showing the impact of using `[[likely]]` and `[[unlikely]]` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95168/new/ https://reviews.llvm.org/D95168 __

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

2021-07-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. If this is something we think we wouldn't want in clang-format, what about the idea of building a new binary which did allow the modification of the tokens, This comes up from time to time and I kind of feel its a weak argument for those that don't want to use it

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

2021-07-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > What you're describing sounds like clang-tidy but perhaps with improved > support for composing source modifications from fix-its, or do you envision > something rather different? All my uses of clang-tidy require extensive command line arguments to handle c

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

2021-07-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > FWIW, if you use the compile commands database, the only thing you need to do > on the command line is specify the checks to enable or disable. The project I work on doesn't have/use compile commands databases? if you are make based system or some other legacy

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

2021-07-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > so there's something like precedent here I think its worth mentioning, that my personal preference would STILL be to land this inside clang-format with default configuration of "OFF", where there is also significant existing precedent for passes that change non

[PATCH] D105943: [clang-format++] Create a new variant of the clang-format tool to allow additional code mutating behaviour such as East/West Const Fixer

2021-07-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, aaron.ballman. MyDeveloperDay added projects: clang-format, clang. Herald added a subscriber: mgorny. MyDeveloperDay requested review of this revision. There has been much discussion about the relat

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

2021-07-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Because I don't want to move this review on other than to maintain it for future rebasing, I've created a new review for the concept of the new tool D105943: [clang-format++] Create a new variant of the clang-format tool to allow additional code mutating behavi

[PATCH] D105964: [clang-format] Make AlwaysBreakAfterReturnType work with K&R C function definitions

2021-07-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Thanks for this, I'm not sure I have the heart to dig out my old K&R code to test it ;-), Assuming the tests are good then it LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105964/new/ https://reviews.llvm.or

[PATCH] D105964: [clang-format] Make AlwaysBreakAfterReturnType work with K&R C function definitions

2021-07-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Myabe give the others some time to take a look too Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105964/new/ https://reviews.llv

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

2021-07-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Interesting reference point here https://blog.jetbrains.com/rscpp/2021/03/30/resharper-cpp-2021-1-syntax-style Kind of suggests that Resharper brings many of these "mutating" capabilities into the same tool that fixes your style. I'd be really interest to know if

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

2021-07-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > So yes, I'm in favour of landing this patch (though not exactly in the > current form, I'd prefer more future-proof options for instance, not only > handling const) I am in agreement, but I don't want to not putting more effort into improving the current desig

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

2021-07-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I wanted to address your other points so we at least can be aligned as I respect your opinion. > I'm however even more wary of adding yet another tool that will be almost the > same as clang-format. Also agreed, but I see no other way forward. > It could work i

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

2021-07-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D69764#2877618 , @atomgalaxy wrote: > It would probably be better to make the config option names for passes that > may mutate whitespace be prefixed with MaybeIncorrect than fork the tool. > Scary options are better than

[PATCH] D105943: [clang-format++] Create a new variant of the clang-format tool to allow additional code mutating behaviour such as East/West Const Fixer

2021-07-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D105943#2877886 , @HazardyKnusperkeks wrote: > This could be a way, but I will also state here that I prefer adding it to > plain `clang-format`. I do too, and that is my preference, Perhaps this way we can establish

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

2021-10-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @lodato are you blocking this review for a reason? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90996/new/ https://reviews.llvm.org/D90996 ___ cfe-commits mailing list cf

[PATCH] D112572: [docs][clang-format] warn on \code block indentation error

2021-10-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. It seems like you are fixing more than just the bug you highlight, i.e looks you might be fixing some autopep8 or pylint issues too. As this isn't a tool we ship, I don't have

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

2021-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbee61aa7b638: [clang-format] Add --staged/--cached option to git-clang-format (authored by ortogonal, committed by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D112572: [docs][clang-format] warn on \code block indentation error

2021-10-30 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG931d20c5db05: [docs][clang-format] warn on \code block indentation error (authored by FederAndInk, committed by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D112887: [clang-format] [PR52228] clang-format csharp inconsistant nested namespace indentation

2021-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=52228 For multilevel namespaces in C# get their content i

[PATCH] D112887: [clang-format] [PR52228] clang-format csharp inconsistant nested namespace indentation

2021-10-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 383652. MyDeveloperDay added reviewers: jbcoe, exv, lunasorcery. MyDeveloperDay added a comment. Add a new more tests Remove unneeded include (adding C# Reviewers) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112887/new/ https://reviews.llv

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

2021-11-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 383744. MyDeveloperDay added a reviewer: HazardyKnusperkeks. MyDeveloperDay added a comment. I'd like to carry the mantle for this, before making any further changes I'd like to address some of the review comments 1. Add documentation warning about a

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

2021-11-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D95168#3099920 , @owenpan wrote: > In D95168#3099739 , @MyDeveloperDay > wrote: > >> - Look further into possible Removal (I have an idea for how this might be >> possible, and s

[PATCH] D112664: [clang-format][docs] fix indentation of rst code block

2021-11-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 should be Format.h, didn't we recently fix this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112664/new/ https:

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

2021-11-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This looks pretty good to me. Could you add a release note into docs/ReleaseNote.rst (we have a .clang-format section) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110833/new/ https://reviews.llvm.org/D110833 ___

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

2021-11-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3469 return true; - if (Right.is(TT_OverloadedOperatorLParen)) -return spaceRequiredBeforeParens(Right); - if (Left.is(tok::comma)) + if (Left.is(tok::comma) && !Right.is(TT_Overload

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

2021-11-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D95168#3102247 , @owenpan wrote: > In D95168#3100969 , @MyDeveloperDay > wrote: > >> In D95168#3099920 , @owenpan wrote: >> >>> In D95168#

[PATCH] D113000: [clang-format] NOT FOR COMMIT - Demo of AutomaticBraces: Remove

2021-11-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added a reviewer: HazardyKnusperkeks. MyDeveloperDay added a project: clang-format. MyDeveloperDay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is not for committing its a clang-f

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

2021-11-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This is a demo of what I mean {https://reviews.llvm.org/D113000} you can see its pretty aggressive, I could kind of imagine people wanting a little more control Sometimes this feels inconsistent F20029866: image.png and in t

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

2021-11-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 384070. MyDeveloperDay added a comment. Herald added a subscriber: mgorny. Move BraceInserter into its own file and tests Add experimental support for "Remove" CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95168/new/ https://reviews.llvm.org

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

2021-11-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 384171. MyDeveloperDay removed a reviewer: klimek. MyDeveloperDay set the repository for this revision to rG LLVM Github Monorepo. MyDeveloperDay added a comment. Add some more testcases to catch handling removing {} from the nested if, but not the out

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

2021-11-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > I already had an implementation of RemoveBraces for the LLVM style (minus > some exceptions). Why not share the implementation in a review then we can combine them here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

2021-11-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > I'm in favor of a struct of enums: > > AutomaticBraces: > AfterIf: OnlyIfElse > AfterElse: Remove #this is obviously only to remove, if the else body is > a single line > AfterWhile: ... > > And we can gradually add new enumerators to handle the d

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

2021-11-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > In my experience, the simplest thing to do is to remove all optional braces > as allowed by the language syntax. When you want to add exceptions for > matching if-else braces, avoiding dangling-else warnings, etc, things get > much more complicated very quickly

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

2021-11-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/BraceInserter.cpp:105 + FormatToken *getNext(int &Line, FormatToken *current) { +if (Line == 0 && current == nullptr) { + return Lines[0]->First; HazardyKnusperkeks wrote: > Remove the {

[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

2021-11-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 384715. MyDeveloperDay retitled this revision from "[clang-format] Add InsertBraces option" to "[clang-format] Add Insert/Remove Braces option". MyDeveloperDay edited the summary of this revision. MyDeveloperDay added a comment. This is very much a WIP

[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

2021-11-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:946 +/// Remove if there is no comment +BIS_RemoveNoComment + }; HazardyKnusperkeks wrote: > Maybe differentiate betwee

[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

2021-11-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:1131 + * ``BIS_Remove`` (in configuration: ``Remove``) +Always remove braces. + Oops missed regenerating this.! (next ti

[PATCH] D113320: [clang-format] Address fixme

2021-11-07 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/D113320/new/ https://reviews.llvm.org/D113320 ___ cfe-commits mailing list cfe-com

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

2021-11-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I feel anything you do here would be an improvement. its probably worth looking at recent issues logged around this issue at the same time https://bugs.llvm.org/show_bug.cgi?id=52401 https://bugs.llvm.org/show_bug.cgi?id=32165 When I added this support its was ba

[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

2021-11-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/test/ClangScanDeps/Inputs/header-search-pruning/cdb.json:1 -[ - { -"directory": "DIR", -"command": "clang -E DIR/header-search-pruning.cpp -Ibegin -I1 -Ia -I3 -I4 -I5 -I6 -Ib -I8 -Iend DEFINES -fmodules -fcxx-modul

[PATCH] D113369: [clang-format] Extend SpaceBeforeParens for requires

2021-11-09 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 (nit the clang-format check) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113369/new/ https://reviews.llvm.org/D113369 __

[PATCH] D97137: [clang-format] Fix AlignConsecutiveDeclarations handling of pointers

2021-02-25 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/D97137/new/ https://reviews.llvm.org/D97137 ___ cfe-commits mailing list cfe-commi

[PATCH] D96760: [clang-format] Suppress diagnostics on second parse

2021-03-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. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96760/new/ https://reviews.llvm.org/D96760 _

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

2021-03-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Just out of interest and we are supposed to ask for this but can you point to public style guide that uses this style. (actually I don't mind if other formatting tools have this capability and you highlight it, like astyle or editorConfig etc) From my perspect

[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

2021-11-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 387015. MyDeveloperDay marked an inline comment as done. MyDeveloperDay edited the summary of this revision. MyDeveloperDay added a comment. Add more LLVM rules, and don't remove when there are single or multi lines comments CHANGES SINCE LAST ACTION

[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

2021-11-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 8 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/BraceInserter.cpp:21-56 +bool SupportsAnyAutomaticBraces(FormatStyle &Style, bool insertion) { + if (insertion) { +if (Style.AutomaticBraces.AfterIf == FormatS

[PATCH] D112887: [clang-format] [PR52228] clang-format csharp inconsistant nested namespace indentation

2021-11-13 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 rG6e58d14e5b01: [clang-format] [PR52228] clang-format csharp inconsistant nested namespace… (authored by MyDeveloperDay). Repository: rG LLVM Github

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

2021-11-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: lichray, HazardyKnusperkeks, curdeius. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. Looks like the work of D113393: [c++2b] Implement P0849R8 auto(x)

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

2021-11-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 387021. MyDeveloperDay added a comment. Additional use cases CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113826/new/ https://reviews.llvm.org/D113826 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp In

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

2021-11-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 387024. MyDeveloperDay added a comment. Remove invalid testcases CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113826/new/ https://reviews.llvm.org/D113826 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

2021-11-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Sorry my mistake, the 1st 4 I want to keep because they related to the unit tests for D113393: [c++2b] Implement P0849R8 auto(x) foo(auto()); // expected-error {{initializer for functional-style cast to 'auto' is empty}}

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

2021-11-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 387028. MyDeveloperDay added a comment. Revert the last change but comment that the first 4 tests are from the unit tests of D113393 which are expected invalid (I wouldn't want the unit tests from formatting incorrec

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

2021-11-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 387073. MyDeveloperDay added a comment. minor comment and unit test change before committing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113826/new/ https://reviews.llvm.org/D113826 Files: clang/lib/Format/TokenAnnotator.cpp clang/unit

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

2021-11-14 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 rGfce3eed9f93a: [clang-format][c++2b] support removal of the space between auto and {} in… (authored by MyDeveloperDay). Repository: rG LLVM Github

[PATCH] D113844: [clang-format] [NFC] build clang-format with -Wall

2021-11-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. When building clang-format with -Wall on Visual Studio 20119 I see the following, prevent this

[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

2021-11-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/BraceInserter.cpp:21-56 +bool SupportsAnyAutomaticBraces(FormatStyle &Style, bool insertion) { + if (insertion) { +if (Style.AutomaticBraces.AfterIf == FormatStyle::BIS_Always) + return true; +if (St

[PATCH] D113844: [clang-format] [NFC] build clang-format with -Wall

2021-11-15 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D113844#3130186 , @HazardyKnusperkeks wrote: > Is okay, but I think the compiler is a bit overreacting, since there is only > one call (chain). I agree, but I think I saw that one of the buildbot was running with -Wal

[PATCH] D114073: [clang-format][NFC] Add a default value to parseBlock()

2021-11-17 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 like it being cleaner Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114073/new/ https://reviews.llvm.org/D114073 __

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

2021-11-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=52527 The follow patch ensures there is always a space be

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

2021-11-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3219 + // of comment. + if (Left.is(tok::star) && Right.is(TT_BlockComment)) +return true; owenpan wrote: > Isn't `tok::comment` better than `TT_BlockComment` if a space i

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

2021-11-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius, owenpan, ChuanqiXu. MyDeveloperDay added projects: clang, clang-format. Herald added subscribers: jrtc27, arichardson. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_b

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

2021-11-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. These don't seem to be issues double term = a * // first b; void foo(int* // this is the first , int second); Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114142/new/

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

2021-11-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'm thinking that those cases are handled else where as S=1 on those elements (which means space required before) M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=double L=6 PPK=2 FakeLParens=2/ FakeRParens=0 II=0x1d662dd3990 Text='double' M=0 C=1 T=StartOfName S=

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

2021-11-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 388241. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Add some more tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114151/new/ https://reviews.llvm.org/D114151 Files: clang/docs/ReleaseNotes.rst cl

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

2021-11-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Nasty, but let me take a look module: private; import ; import foo... bar; import..; module foo: private; module public : while; they all feel all kinds of wrong! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114151/new/ https://rev

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

2021-11-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > I only now this from the tests with `-debug`. Is there a way to get "normal" > clang-format to print these? Or did you write a test for that to print the > information? I just use the --debug option to clang-format.exe? I develop on windows, using Visual Studi

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

2021-11-19 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 388419. MyDeveloperDay added a comment. Add yet more unit tests and cover those. Mark one test as TODO as I'm not sure if its allowed syntax CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114151/new/ https://reviews.llvm.org/D114151 Files:

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

2021-11-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 388696. MyDeveloperDay added a comment. Upodate to include all comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114142/new/ https://reviews.llvm.org/D114142 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTe

<    9   10   11   12   13   14   15   16   17   18   >