[PATCH] D116283: [clang-format] Add an option to add a space between operator overloading and opening parentheses

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Does it affect calling code? `a.operator++(5);` Should it? But please add tests for that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116283/new/ https://reviews.llvm.org/D116283 _

[PATCH] D116170: [clang-format] Add penalty for breaking after '('

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:2900 + /// \version 14 + unsigned PenaltyBreakOpenParenthesis; + Please resort. Comment at: clang/lib/Format/Format.cpp:1238 LLVMStyle.PenaltyBreakTe

[PATCH] D116314: [clang-format] Add style to separate definition blocks

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/Format.cpp:2027 +// namespaces by inserting a line break between them. +class DefinitionBlockSeparator : public TokenAnalyzer { +public: curdeius wrote: > Please consider moving it to a differ

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D116316#3211269 , @curdeius wrote: > Could you have a look at preceding reviews and see if there wasn't a similar > patch before? > I think that this option is a bit too limited. > Only removing braces doesn't seem

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:1903 /// lead to incorrect code formatting due to incorrect decisions made due to - /// clang-formats lack of complete semantic information. + /// clang-format's lack of complete sem

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. I'm against that patch. > Don't pay for what you don't use. We should not put the size check into every call. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116318/new/ https://reviews.llvm.org/D116318

[PATCH] D116314: [clang-format] Add style to separate definition blocks

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:3054 + enum SeparateDefinitionStyle { +/// Leave definition blocks separated as they are without changing anything +SDS_Leave, Add full stop at the end of sentences

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

2021-12-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Do you plan to refactor something for this review, or do you think you are done? Then I will look at it again as a whole. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72326/new/ https://reviews.llvm.org/D72326

[PATCH] D116314: [clang-format] Add style to separate definition blocks

2021-12-29 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.h:38-39 +}; +} // end namespace format +} // end namespace clang + HazardyKnusperkeks wrote: > I know you copied it. It is wrong where you copied it from. :) Not done.

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

2021-12-29 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2338 + KeepAncestorBraces(); + NestedTooDeep.push_back(false); if (FormatTok->is(tok::l_brace)) { owenpan wrote: > HazardyKnusperkeks wrote: > > I think it is wort

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

2021-12-29 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks marked an inline comment as done. HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/Format.cpp:3067 + if (!ChildFormatTextToApply.empty()) { +assert(ChildFormatTextToApply.size() == 1); + zwliew wrote: > zwliew wrote:

[PATCH] D72326: [clang-format] Add option to explicitly specify a config file

2021-12-29 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. Although there is no context. Maybe still update the uploaded diff for the archive. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72326/new/ https://reviews.llvm.org/D72326 _

[PATCH] D116170: [clang-format] Add penalty for breaking after '('

2021-12-29 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Looks good, use the `verifyFormat` and please mark comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116170/new/ https://reviews.llvm.org/D116170 ___ cfe

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

2021-12-29 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2303 + assert(IfRightBrace->MatchingParen == IfLeftBrace); + IfLeftBrace->MatchingParen = nullptr; + IfRightBrace->MatchingParen = nullptr; Why null that

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

2021-03-16 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added reviewers: MyDeveloperDay, HazardyKnusperkeks. HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added inline comments. This revision now requires changes to proceed. Comment at: clang/docs/ReleaseNotes.rst:168 +- Option ``SpaceB

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

2021-03-16 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:1914 +ELAAMS_Leave, +/// Always add empty line after access modifiers. +/// \code It does not always add, it does enforces one empty line, or am I mistaken? Re

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

2021-03-16 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a subscriber: curdeius. HazardyKnusperkeks added inline comments. Comment at: clang/docs/ReleaseNotes.rst:168 +- Option ``SpaceBeforeForLoopSemiColon`` has been added to control whether + spaces will be added before the semi-colons in for loops. --

[PATCH] D99031: [clang-format] Fix CompactNamespaces corner case when AllowShortLambdasOnASingleLine/BraceWrapping.BeforeLambdaBody are set

2021-03-21 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. Looks good. How about `verifyFormat`? Would that fail without your patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99031/new/ https://reviews.llvm.org/D99031 _

[PATCH] D99031: [clang-format] Fix CompactNamespaces corner case when AllowShortLambdasOnASingleLine/BraceWrapping.BeforeLambdaBody are set

2021-03-22 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D99031#2640425 , @aybassiouny wrote: >> How about verifyFormat? Would that fail without your patch? > > @HazardyKnusperkeks sorry, but not sure that I understand your point. Are you > suggesting to use verifyFormat i

[PATCH] D98214: [clang-format] Fix aligning with linebreaks

2021-03-28 Thread Björn Schäpers 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 rGc5243c63cda3: [clang-format] Fix aligning with linebreaks (authored by HazardyKnusperkeks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

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

2021-04-07 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. While the fix seems to be right, I don't think we should change the LLVM style (and possibly other styles). So my suggestion is to apply the fix, but actually change the style so that it behaves like before, so that braces of enums are wrapped. And maybe add

[PATCH] D99503: [clang-format] Inconsistent behavior regarding line break before access modifier

2021-04-14 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Look good in general, only the few comments. Comment at: clang/unittests/Format/FormatTest.cpp:8892 FormatStyle Style = getLLVMStyle(); + FormatStyle NoEmptyLines = getLLVMStyle(); + NoEmptyLines.MaxEmptyLinesToKeep = 0;

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

2021-04-14 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5077 + verifyFormat("asm volatile(\"string\", : : val);", Style); + verifyFormat("asm volatile(\"string\", : val1 : val2);", Style); + You should add a test case with lo

[PATCH] D100590: DeclContext: Fix iterator category

2021-04-15 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision. HazardyKnusperkeks added reviewers: rsmith, v.g.vassilev. HazardyKnusperkeks added projects: clang, clang-tools-extra, LLDB. Herald added a subscriber: JDevlieghere. HazardyKnusperkeks requested review of this revision. Herald added a subscriber: cfe-commit

[PATCH] D99503: [clang-format] Inconsistent behavior regarding line break before access modifier

2021-04-15 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. I don't know if you did elsewhere, but you have to give a name and email for the commit, so that someone can push it for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D100590: DeclContext: Fix iterator category

2021-04-15 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 337985. HazardyKnusperkeks added a comment. Trigger build. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100590/new/ https://reviews.llvm.org/D100590 Files: clang/include/clang/AST/DeclBase.h In

[PATCH] D100727: [clang-format] Correctly apply AllowShortIfStatementsOnASingleLine: Always to else branch.

2021-04-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. How is if (a) return; else return; formatted with the different options? And from the documentation I think it was intended that only `if` is short, never the `else`. So I think it behaves correctly, nevertheless I think it should be possible, but I

[PATCH] D100727: [clang-format] Correctly apply AllowShortIfStatementsOnASingleLine: Always to else branch.

2021-04-19 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D100727#2697490 , @curdeius wrote: > In D100727#2697419 , > @HazardyKnusperkeks wrote: > >> How is >> >> if (a) return; >> else >> return; >> >> formatted with the d

[PATCH] D99031: [clang-format] Fix CompactNamespaces corner case when AllowShortLambdasOnASingleLine/BraceWrapping.BeforeLambdaBody are set

2021-04-19 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99031/new/ https://reviews.llvm.org/D99031 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D100727: [clang-format] Correctly apply AllowShortIfStatementsOnASingleLine: Always to else branch.

2021-04-19 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D100727#2697815 , @curdeius wrote: > Oh, you're right. Should we go for a separate option then? > AllowShortElseStatementsOnASingleLine? What would control else if statements > then? I think I would prefer to expa

[PATCH] D100727: [clang-format] Correctly apply AllowShortIfStatementsOnASingleLine: Always to else branch.

2021-04-19 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D100727#2698422 , @curdeius wrote: > More I look at the current state of this option, more I think it's misleading. > I would expect that the if after else is also controlled by this option. And, > the last else as

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

2021-04-21 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added a comment. This revision now requires changes to proceed. Looks good, but please fix the clang-format notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950

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

2020-12-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks marked an inline comment as done. HazardyKnusperkeks added a comment. In D92257#2452063 , @MyDeveloperDay wrote: > This didn't really address the comments, what is the point of the maximum? > what if the maximum is > the ColumnLimit?

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

2020-12-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:252 + + if (Tok && Tok->is(tok::kw_template) && + Style.BraceWrapping.SplitEmptyRecord && EmptyBlock) { Why is this not just also in the previous if? I

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

2020-12-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision. HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius, krasimir, klimek, njames93, mitchell-stellar. HazardyKnusperkeks added a project: clang-format. HazardyKnusperkeks requested review of this revision. Herald added a project: clang. Herald added

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

2020-12-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D93844#2472346 , @MyDeveloperDay wrote: > I'm a little confused why this is needed as clang-format always read up the > directory tree until it see a .clang-format file, perhaps I don't quite > understand the use c

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

2020-12-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:68 + /// \endcode + bool InsertEmptyLineBeforeAccessModifier; + MyDeveloperDay wrote: > quite a mouthful... maybe just `NewLineBeforeAccessModifier` ? A new line is always

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

2020-12-29 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D93844#2472352 , @njames93 wrote: > In D93844#2472346 , @MyDeveloperDay > wrote: > >> I'm a little confused why this is needed as clang-format always read up the >> directory

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

2020-12-29 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D93844#2473725 , @MyDeveloperDay wrote: > I like what you are suggesting, do you think `BasedOnStyle: File` is the best > terminology as the term `File` is used elsewhere to mean read from the > .clang_format file,

[PATCH] D93938: Fixed AfterEnum handling

2020-12-30 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. You should add tests to prove what you are doing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93938/new/ https://reviews.llvm.org/D93938 ___ cfe-commits mailing list

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

2021-01-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D93938#2476432 , @atirit wrote: > I think accepting a revision that includes only a fix for `AfterEnum` being > ignored (not the corner case) and the new unit test would be the best way to > go about this, since the

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

2021-01-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added a comment. This revision now requires changes to proceed. In D93986#2477474 , @tinloaf wrote: > In D93986#2477383 , @MyDeveloperDay >

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

2021-01-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1221 +if (Style.EmptyLineBeforeAccessModifier && +PreviousLine->Last->isOneOf(tok::semi, tok::r_brace) && +RootToken.NewlinesBefore == 1) thezbyg

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

2021-01-05 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1221 +if (Style.EmptyLineBeforeAccessModifier && +PreviousLine->Last->isOneOf(tok::semi, tok::r_brace) && +RootToken.NewlinesBefore == 1) thezbyg

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

2021-01-07 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. I think your base for the diff is wrong, there are many "added" things which are already in clang-format. Could you update the diff? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93846/new/ https://reviews.llvm.org/D93846 ___

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

2021-01-07 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. I don't know if I would create that many test functions, but put it all in one (maybe even one existing). Otherwise this looks ok. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94206/new/ https://reviews.llvm.or

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

2021-01-07 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Should the BOM not be ignored in ANY case, not just while sorting includes? And thus a different code should be touched? How about aligned assignments at the beginning of the line with a BOM, does it count to the type name, or does clang-format handle that co

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

2021-01-07 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D94217#2483925 , @rjelonek wrote: > Currently at work we use Embarcadero Builder C++ I'm very sorry for you! (As someone who has (had?) also to use it.) > In builder all above #pragma hdrstop is a precompiled header

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

2021-01-09 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D93846#2488469 , @thezbyg wrote: > Diff updated. Previous diff was generated after rebase, and Phabricator > change preview did not show any unrelated changes, so I thought that > everything is fine. Now your diff

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

2021-01-10 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Apart from my inline comment and the pre merge check this is superb. (I will not accept it, until we have reached a conclusion for the documentation.) Comment at: clang/docs/ClangFormatStyleOptions.rst:338 + + int = 12; + int

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

2021-01-13 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Apart from that last naming issue looks good to me. Comment at: clang/lib/Format/Format.cpp:235 +IO.enumCase(Value, "Never", FormatStyle::ELBAMS_Never); +IO.enumCase(Value, "DontModify", FormatStyle::ELBAMS_DontModify); +IO.enumCa

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

2021-01-13 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnalyzer.cpp:71 Env.getFirstStartColumn(), Style, Encoding, Allocator, - IdentTable); Unrelated change (although I think it's good one).

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

2021-01-13 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnalyzer.cpp:71 Env.getFirstStartColumn(), Style, Encoding, Allocator, - IdentTable); timwoj wrote: > HazardyKnusperkeks wrote: > > Unrelat

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

2021-01-14 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D92257#2497535 , @MyDeveloperDay wrote: > My assumption is that you want to stick with the minimum and maximum is that > correct? Otherwise I have to make a breaking change, or not achieve at all what I want. So e

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

2021-01-14 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. I would add a test where you have a member before the first access modifier. Also this is not exactly what they wanted in the bug, as far as I can see members of structs or classes with no access modifier at all should only be indented once. Repository: rG

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

2021-01-15 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. *ping* And I don't have a better name. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93776/new/ https://reviews.llvm.org/D93776 ___ cfe-commits mailing list cfe-commit

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

2021-01-15 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D93986#2495561 , @MyDeveloperDay wrote: > I think I would remove the code examples from the "AlignConsecutive style" to > avoid confusion (that would be the first change) > > then perhaps regenerate and update the d

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

2021-01-16 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Not what I had in mind, but for me that's okay. I can not say anything to the change of the script though. What I hat in mind was explaining (maybe in length) the enum and for the alignment settings just show an example with `None` and one of the other optio

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

2021-01-17 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 317268. HazardyKnusperkeks marked an inline comment as done. HazardyKnusperkeks added a comment. Sorting corrected. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93776/new/ https://reviews.llvm.org/D

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

2021-01-17 Thread Björn Schäpers 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 rGbcc1dee60019: [clang-format] Add StatementAttributeLikeMacros option (authored by HazardyKnusperkeks). Repository: rG LLVM Github Monorepo CHANGE

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

2021-01-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 317292. HazardyKnusperkeks added a comment. Fix documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93776/new/ https://reviews.llvm.org/D93776 Files: clang/docs/ClangFormatStyleOptions.rs

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

2021-01-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 317294. HazardyKnusperkeks added a comment. - Rebased - Fixed(?) the last UnitTest, please take a look @krasimir Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92257/new/ https://reviews.llvm.org/D922

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

2021-01-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 317295. HazardyKnusperkeks marked 2 inline comments as done. HazardyKnusperkeks edited the summary of this revision. HazardyKnusperkeks added a comment. - Rebased - Renamed to `InhteritParentConfig` - Reworked to remove the recursion of `getStyle` -

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

2021-01-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D93844#2504376 , @MyDeveloperDay wrote: > What I can't easily tell from the tests is if you are overriding any styles > defined in the parent with a local style. Yes I do, but I will try to make it clearer in the t

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

2021-01-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision. HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius. HazardyKnusperkeks added a project: clang-format. HazardyKnusperkeks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. According to https:

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

2021-01-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 317358. HazardyKnusperkeks added a comment. Formatting corrected. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94906/new/ https://reviews.llvm.org/D94906 Files: clang/lib/Format/Format.cpp clan

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

2021-01-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D93844#2504388 , @HazardyKnusperkeks wrote: > In D93844#2504376 , @MyDeveloperDay > wrote: > >> What I can't easily tell from the tests is if you are overriding any styles >

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

2021-01-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. I think that would have helped me also. I think it's not that easy, but honestly never checked. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94906/new/ https://reviews.llvm.org/D94906 _

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

2021-01-19 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGcbdde495ba28: [clang-format] Apply Allman style to lambdas (authored by HazardyKnusperkeks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94906/new/ https:

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

2021-01-19 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 317679. HazardyKnusperkeks added a comment. Found and fixed the case when no `BasedOnStyle` apart from `InheritParentConfig` is used. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93844/new/ https:/

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

2021-01-19 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Is there a test with indentation within a namespace? If not maybe you should add one. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:613 + // settings. This allows the format to back up one level in those cases. + if (!AddLevel && Na

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

2021-01-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. How is it going? I would like to add some new alignment options and it would be stupid not to do it on top of this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93986/new/ https://reviews.llvm.org/D93986 _

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

2021-01-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D93986#2509079 , @tinloaf wrote: > In D93986#2509045 , > @HazardyKnusperkeks wrote: > >> How is it going? I would like to add some new alignment options and it would >> be st

[PATCH] D95078: [clang-format] [NFC] Use some constexpr StringRef instead of const char*.

2021-01-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision. HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius. HazardyKnusperkeks added a project: clang-format. HazardyKnusperkeks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LL

[PATCH] D95081: [clang-format] [NFC] Restructure getLineCommentIndentPrefix

2021-01-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision. HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius. HazardyKnusperkeks added a project: clang-format. HazardyKnusperkeks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When sorting the kn

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

2021-01-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Please expand the test case to different options like grouping. Otherwise looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 ___

[PATCH] D95081: [clang-format] [NFC] Restructure getLineCommentIndentPrefix

2021-01-21 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 318151. HazardyKnusperkeks added a comment. Now with `assert`. We could do a `static_assert`, but then we would have to write our own `is_sorted`, since the `std` is only `constexpr` in C++20. Repository: rG LLVM Github Monorepo CHANGES SINCE

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

2021-01-21 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks 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/D95017/new/ https://reviews.llvm.org/D95017

[PATCH] D95081: [clang-format] [NFC] Restructure getLineCommentIndentPrefix

2021-01-21 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 318156. HazardyKnusperkeks added a comment. Empty lines removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95081/new/ https://reviews.llvm.org/D95081 Files: clang/lib/Format/BreakableToken.cpp

[PATCH] D95128: [clang-format] [NFC] Remove unsued arguments

2021-01-21 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision. HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius. HazardyKnusperkeks added a project: clang-format. HazardyKnusperkeks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LL

[PATCH] D110252: Added note about Whatstyle and Unformat

2021-10-06 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks resigned from this revision. HazardyKnusperkeks added a comment. I too think this should not directly mentioned in clang-format documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110252/new/ https://reviews.llvm.org/D11

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

2021-10-19 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. Otherwise it looks good. Comment at: clang/include/clang/Format/Format.h:245 + /// \version 14 + bool AlignConsecutiveMacrosIgnoreMax; + ---

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

2021-10-19 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. In D112056#3073569 , @MyDeveloperDay wrote: > In D112056#3073142 , @lhames wrote: >

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

2021-10-21 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:3341 +/// \endcode +bool AfterFunctionDeclarationName; +/// If ``true``, put a space between function definition name and opening Could you sort these? AfterCont

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

2021-10-25 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/docs/ReleaseNotes.rst:227 correctly places the opening brace according to ``BraceWrapping.AfterEnum``. +- Option ``AlignAfterOpenBracket: BlockIndent`` has been added. If set, it will + always break after an open bra

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

2021-10-25 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:3422 ///true: false: - ///for (auto v : values) {} vs. for(auto v: values) {} + ///for (auto v : values) {} vs. for (aut

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

2021-08-26 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:18472 + FormatStyle::PCIS_Never); + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock; Shou

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

2021-08-26 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added a comment. As the one who wrote that: 1. Yes that part is not auto generated, because it is not in the `format.h`. 2. I'm more in favor of removing the `std::` from the documentation. 3. It is for the YAML documentati

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

2021-08-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. Looks good and I agree with the choices. Comment at: clang/docs/tools/dump_format_style.py:23 + plurals = { +'IncludeCategory': 'IncludeCategorie

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

2021-08-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/docs/tools/dump_format_style.py:23 + plurals = { +'IncludeCategory': 'IncludeCategories' + } FederAndInk wrote: > HazardyKnusperkeks wrote: > > Could you not just check if there is a y at the end a

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

2021-08-29 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/docs/tools/dump_format_style.py:23 + plurals = { +'IncludeCategory': 'IncludeCategories' + } MyDeveloperDay wrote: > HazardyKnusperkeks wrote: > > FederAndInk wrote: > > > HazardyKnusperkeks wrote:

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

2021-08-30 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D108765#2972159 , @FederAndInk wrote: > And again, I don't really understand if we are allowed or not to pull in a > dependency such as pluralizer or inflect, this would be another idea My Python knowledge is very

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

2021-09-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. From my point we can try that one, if there are problems we have plenty of time to revert it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108765/new/ https://reviews.

[PATCH] D109557: Adds an AlignCloseBracket option

2021-09-13 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D109557#2997899 , @csmulhern wrote: > I've added more information to my original message. Please let me know if > further context is desired. With context he meant the diff context. https://llvm.org/docs/Phabricat

[PATCH] D109557: Adds an AlignCloseBracket option

2021-09-13 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. You state in the documentation that it is also for angle brackets and more, but there are no test cases for that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109557/new/ https://reviews.llvm.org/D109557 _

[PATCH] D109752: [clang-format] Top-level unwrapped lines don't follow a left brace

2021-09-14 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. If there are no new tests, what went wrong before? Said invalid code? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109752/new/ https://reviews.llvm.org/D109752 ___ cf

[PATCH] D109557: Adds an AlignCloseBracket option

2021-09-14 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D109557#2998727 , @csmulhern wrote: > In D109557#2998213 , > @HazardyKnusperkeks wrote: > >> With context he meant the diff context. >> https://llvm.org/docs/Phabricator.ht

[PATCH] D109557: Adds a BreakBeforeClosingParen option

2021-09-15 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:22301 + auto Style = getLLVMStyle(); + Style.BreakBeforeClosingParen = true; + Could you also add tests for `false`, even though they are spread over the other test cas

<    6   7   8   9   10   11