[PATCH] D45719: [clang-Format] Fix indentation of member call after block

klimek added a comment. Sorry that I lost track of that, but feel free to ping once / week - unfortunately the patch doesn't apply cleanly, can you rebase? Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list

[PATCH] D49840: [AST] Add MatchFinder::matchSubtree

klimek added a comment. Usually we use match(anyOf(node), hasDescendant(node)). Or did I misunderstand what you want? Repository: rC Clang https://reviews.llvm.org/D49840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. The problem here is that we have different opinions on how the formatting on namespace macros should behave in the first place- I think they should behave like namespaces, you want them to be formatted differently. Given that you want full control over the formatting of

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

klimek added a comment. Yea, if we go down this route I'd go with this by default: some ? thing : else ? otherthing : unrelated ? but : finally; Theoretically we could even use: some ? thing : else; by default ;) Repository: rC Clang https://reviews.llvm.org/D50078

[PATCH] D50697: [clang-format] fix PR38557 - comments between "default" and ':' causes the case label to be treated as an identifier

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang https://reviews.llvm.org/D50697 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1307 + (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr && +Current.Next->is(TT_LambdaLSquare))); State.Stack.back().IsInsideObjCArrayLiteral = klimek

[PATCH] D51261: Add preload option to clang-query

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51261 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51259: Allow binding to NamedValue resulting from let expression

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang https://reviews.llvm.org/D51259 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51258: Extract parseBindID method

klimek added inline comments. Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:362 +bool Parser::parseBindID(std::string , TokenInfo ) { + // Parse .bind("foo") CloseToken seems to not be used afterwards either here or in the follow-up patch? Repository:

[PATCH] D49724: [VFS] Cleanups to VFS interfaces.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang https://reviews.llvm.org/D49724 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1307 + (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr && +Current.Next->is(TT_LambdaLSquare))); State.Stack.back().IsInsideObjCArrayLiteral = I think

[PATCH] D43969: Improve completion experience for headers

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43969 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38615: [libclang] Only mark CXCursors for explicit attributes with a type

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D38615 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45726: Format closing braces when reformatting the line containing theopening brace.This required a couple of yaks to be shaved:1. MatchingOpeningBlockLineIndex was misused to also store the

klimek created this revision. klimek added a reviewer: krasimir. ...doesn't work correctly for "} else {". 2. We needed to change the API of AffectedRangeManager to not use iterators; we always passed in begin / end for the whole container before, so there was no mismatch in generality. 3.

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

klimek added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:438 + + AlignTokens(Style, + [&](const Change ) { I'm not sure whether we should use AlignTokens here, given that we pass in a parameter to basically skip all its

[PATCH] D45726: Format closing braces when reformatting the line containing theopening brace.

klimek added a comment. In https://reviews.llvm.org/D45726#1071925, @krasimir wrote: > Another point: for the example in the summary about bailing-out early, is > there a test for this already? If not, we should add one. Yep, there already is one - I regressed that with my change at first ;)

[PATCH] D45726: Format closing braces when reformatting the line containing theopening brace.

klimek updated this revision to Diff 143274. klimek marked 2 inline comments as done. klimek added a comment. Address comments. Repository: rC Clang https://reviews.llvm.org/D45726 Files: lib/Format/AffectedRangeManager.cpp lib/Format/AffectedRangeManager.h lib/Format/Format.cpp

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:4359 + "return 3;\n" + " }).as("");\n" + "}"); What would be interesting is tests that: a) have another value after the closing }; doesn't

[PATCH] D46062: [clang-format] Start formatting cpp code in raw strings in google style

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang https://reviews.llvm.org/D46062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45726: Format closing braces when reformatting the line containing theopening brace.

This revision was automatically updated to reflect the committed changes. Closed by commit rL330573: Format closing braces when reformatting the line containing the opening brace. (authored by klimek, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

klimek added inline comments. Comment at: include/clang/Format/Format.h:891 + /// \endcode + bool BreakBeforeLambdaBody; + I'd just make that default for Allman style. Comment at: lib/Format/TokenAnnotator.cpp:2844 if (isAllmanBrace(Left)

[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.

klimek added a comment. Is this written down somewhere? https://webkit.org/code-style-guidelines/ doesn't seem to mention it. Repository: rC Clang https://reviews.llvm.org/D46024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

klimek added a comment. In https://reviews.llvm.org/D28462#1147019, @enyquist wrote: > @klimek fair point. To be honest, I've pretty much lost interest / momentum > on this feature, I very much doubt I will ever go back and re-implement from > scratch as you suggest. > Not meaning to sound

[PATCH] D48159: [clangd] Implement hover for "auto" and "decltype"

klimek added a reviewer: rsmith. klimek added inline comments. Comment at: clangd/XRefs.cpp:559 + //- auto& i = 1; + bool VisitDeclaratorDecl(DeclaratorDecl *D) { +if (!D->getTypeSourceInfo() || malaperle wrote: > klimek wrote: > > sammccall wrote: > > >

[PATCH] D48159: [clangd] Implement hover for "auto" and "decltype"

klimek added inline comments. Comment at: clangd/XRefs.cpp:559 + //- auto& i = 1; + bool VisitDeclaratorDecl(DeclaratorDecl *D) { +if (!D->getTypeSourceInfo() || sammccall wrote: > malaperle wrote: > > sammccall wrote: > > > out of curiosity, why not

[PATCH] D54077: [clangd] Implemented DraftFileSystem

klimek added a comment. In https://reviews.llvm.org/D54077#1287040, @ilya-biryukov wrote: > Thanks for the patch! I believe many people I talked to want this behavior > (myself included). > Some people like what we do now more. It feels like it depends on the > workflow: for people who

[PATCH] D54077: [clangd] Implemented DraftFileSystem

klimek added a comment. In https://reviews.llvm.org/D54077#1287282, @LutsenkoDanil wrote: > @klimek If behavior will be configurable, is it ok for you? I have the same concerns as Sam for making this an option. > @sammccall Current behavior may confuse new users, since, other IDEs mostly >

[PATCH] D54453: Remove myself as owner of clang-query.

klimek added a comment. Thanks Aaron for volunteering, I'm very thankful for all your work on the reviews, it's much appreciated :D Repository: rL LLVM https://reviews.llvm.org/D54453 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54077: [clangd] Implemented DraftFileSystem

klimek added a comment. In https://reviews.llvm.org/D54077#1288404, @ioeric wrote: > In https://reviews.llvm.org/D54077#1288387, @sammccall wrote: > > > Someone mentioned to me that the interaction-between-features argument > > wasn't clear here: > > > > - we **don't** currently update

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#1253813, @Typz wrote: > In https://reviews.llvm.org/D37813#1184051, @klimek wrote: > > > The problem here is that we have different opinions on how the formatting > > on namespace macros should behave in the first place- I think they

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1307 + (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr && +Current.Next->is(TT_LambdaLSquare))); State.Stack.back().IsInsideObjCArrayLiteral = Wawha

[PATCH] D56841: [clangd] Filter out plugin related flags.

klimek added a comment. In D56841#1361395 , @kadircet wrote: > It seems like the most relevant place in tooling is ArgumentsAdjuster as > @ioeric pointed out. Happy to move it to there if @sammccall and @klimek > agrees as well. > > As a side note

[PATCH] D54309: [AST] Allow limiting the scope of common AST traversals (getParents, RAV).

klimek added a comment. In D54309#1326852 , @JonasToth wrote: > See PR39949 as well, as it seems to trigger the same or a similar problem. > @ioeric and @klimek maybe could give an opinion, too? Sounds like we might not correctly set the parent map

[PATCH] D54952: [clangd] DO NOT SUBMIT. Draft interfaces for build system integration.

klimek added inline comments. Comment at: clangd/BuildSystem.h:29 +/// Default compilation database used by clangd, based on the build system. +class Integration : public GlobalCompilationDatabase { +public: 'Integration' is a bit of a weird name, as it doesn't

[PATCH] D54672: clang-include-fixer.el: support remote files

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54672/new/ https://reviews.llvm.org/D54672 ___

[PATCH] D54952: [clangd] DO NOT SUBMIT. Draft interfaces for build system integration.

klimek added inline comments. Comment at: clangd/BuildSystem.h:29 +/// Default compilation database used by clangd, based on the build system. +class Integration : public GlobalCompilationDatabase { +public: ilya-biryukov wrote: > klimek wrote: > > 'Integration'

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

klimek added a comment. Just in general, I'd like to add that my experience over the years dealing with folks trying to do AST matchers is that the inability to express something is much more of a problem than matching too much, simply because the test cases people think of are usually small,

[PATCH] D56090: Add a matcher for members of an initializer list expression

klimek added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:3527 + return N < Node.getNumInits() && + InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder, + Builder); aaron.ballman

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

klimek added a comment. In D56444#1351056 , @aaron.ballman wrote: > In D56444#1350849 , @JonasToth wrote: > > > In D56444#1350746 , @sammccall > > wrote: > > > > >

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

klimek added a comment. In D56444#1350746 , @sammccall wrote: > In D56444#1350252 , @JonasToth wrote: > > > I still see the unit-test crashing for `ExprMutAnalyzer` (just apply the > > last two tests > >

[PATCH] D58404: [clang-format] Add basic support for formatting C# files

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. This looks pretty sharp (scnr). Comment at: clang/lib/Format/FormatTokenLexer.cpp:177 +if (Dollar->TokenText == "$") { + // this looks like $@"a" so we need

[PATCH] D42034: [clang-format] In tests, expected code should be format-stable

klimek added a comment. Great idea! LG from my side after addressing MyDeveloperDay's comments. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D42034/new/ https://reviews.llvm.org/D42034 ___ cfe-commits mailing list

[PATCH] D59684: [clang-format] [PR41187] moves Java import statements to the wrong location if code contains statements that start with the word import

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59684/new/ https://reviews.llvm.org/D59684 ___ cfe-commits mailing list

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

klimek accepted this revision. klimek added a comment. Oh, and LG :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org/D55170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

klimek added a comment. In D55170#1439864 , @MyDeveloperDay wrote: > @lebedev.ri Are we talking about a general ideology of the long term cost to > allow any new things in? or to not allow things in this specific case? > > because in this specific case

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

klimek added a comment. Minus me understanding the TT_TemplateString change, the rest looks nice now, thanks! Comment at: clang/lib/Format/TokenAnnotator.cpp:3177-3178 return false; // Don't split tagged template literal so there is a break between the tag //

[PATCH] D59774: [clang-format] Refine structured binding detection

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59774/new/ https://reviews.llvm.org/D59774 ___ cfe-commits

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: clang/lib/Format/TokenAnnotator.cpp:3177-3178 return false; // Don't split tagged template literal so there is a break between the tag //

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

klimek added a comment. I just verified why this doesn't break for our CI: The trick is that if the indent is actually off (as opposed to being correct, but tab vs spaces), we do re-indent until we find an indent that's correct. The fix that would make me happier, I think, is to do the same

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

klimek added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:520 + + AlignMacroSequence(StartOfSequence, EndOfSequence, MinColumn, MaxColumn, + FoundMatchOnLine, AlignMacrosMatches, Changes); Why are we calling

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

klimek added a comment. The previous behavior looks intentional, and much more regular. I'd be curious why you think the proposed behavior is more readable. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60374/new/ https://reviews.llvm.org/D60374

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

klimek accepted this revision. klimek added a comment. lg CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60320/new/ https://reviews.llvm.org/D60320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping

klimek accepted this revision. klimek added a comment. Apart from the typo I think this is a simple enough change and a widely enough used style that it LG. Comment at: lib/Format/UnwrappedLineParser.cpp:181 + CompoundStatementIndenter(UnwrappedLineParser *Parser, unsigned ,

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

klimek added inline comments. Comment at: cfe/trunk/docs/ClangFormatStyleOptions.rst:384 + + * ``SIS_WithoutElse`` (in configuration: ``WithoutElse``) +Allow short if functions on the same line, as long as else Is that actually the status quo or is the

[PATCH] D59746: [LibTooling] Fix '-h' option

klimek added a comment. In D59746#1440756 , @hintonda wrote: > A better alternative would have been to add a cl::aliasopt for '-h' in llvm's > CommandLineParser when '-help' was first added. However, that's no longer > possible since some llvm based

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

klimek added inline comments. Comment at: cfe/trunk/docs/ClangFormatStyleOptions.rst:384 + + * ``SIS_WithoutElse`` (in configuration: ``WithoutElse``) +Allow short if functions on the same line, as long as else MyDeveloperDay wrote: > klimek wrote: > > Is

[PATCH] D59746: [LibTooling] Fix '-h' option

klimek added a comment. In D59746#1458424 , @hintonda wrote: > In D59746#1458086 , @klimek wrote: > > > In D59746#1440756 , @hintonda > > wrote: > > > > > A better

[PATCH] D60362: [clang-format] [PR39719] clang-format converting object-like macro to function-like macro

klimek added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2467-2470 + if (Line.InPPDirective && Right.is(tok::l_paren) && + !Left.is(tok::identifier) && Left.Previous && + Left.Previous->is(tok::identifier) && Left.Previous->Previous && +

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

klimek added a comment. In D28462#1405360 , @MyDeveloperDay wrote: > I'm not a code owner here but we seem to be lacking people who are either > available, able, willing or allowed to approve code reviews for some of the > code in the clang-tooling

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

klimek added a comment. In D28462#1405855 , @MyDeveloperDay wrote: > In D28462#1405711 , @klimek wrote: > > > In D28462#1405360 , > > @MyDeveloperDay wrote: > > > > > I'm

[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

klimek added a comment. In D58345#1406892 , @sammccall wrote: > Sorry to be a pain here, but I'm not sure introducing Javascript is a good > idea unless there's a strong reason for it. > All LLVM developers have python installed, many are comfortable

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

klimek added a comment. In D33029#1423949 , @MyDeveloperDay wrote: > In D33029#1423947 , @lebedev.ri > wrote: > > > In D33029#1423944 , > > @MyDeveloperDay wrote: > > > >

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

klimek added a comment. In D55170#1423941 , @MyDeveloperDay wrote: > In D55170#1423798 , @reuk wrote: > > > Thanks for the review, and for approving this PR. It's very much > > appreciated! > > > > I don't have

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

klimek added a comment. Why did the numeric constant break this? Which path would the function run through? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59309/new/ https://reviews.llvm.org/D59309 ___ cfe-commits mailing list

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

klimek added a comment. In D59309#1428807 , @MyDeveloperDay wrote: > In D59309#1428799 , @klimek wrote: > > > Why did the numeric constant break this? Which path would the function run > > through? > > > as its

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

klimek accepted this revision. klimek added a comment. Given this doesn't add an extra option (but only an extra enum) and is fairly straight forward, LG. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

klimek added a comment. In D28462#1406716 , @jacob-keller wrote: > In regards to whether clang-format should support this feature, I think it's > pretty clear that a large number of users want it to. The tool already > supports formatting various other

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

klimek added a comment. In D59309#1428969 , @MyDeveloperDay wrote: > That works because the argument list is just tok::identifier and > TT_StartOfName Should we fix isStartOfName then? In A or A<8> A should be TT_StartOfName, too, right? CHANGES

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

klimek added a comment. Yea, the a b(c) problem is the olden C++ problem clang-format will never be able to solve, thus all these crazy heuristics. So, inching closer to understanding this - I totally missed the isLiteral() that short-circuits out before. I'm more happy with your current

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

klimek added a comment. In D55170#1432929 , @MyDeveloperDay wrote: > > I don't think risk is what matters - in the end it's about cost, and cost > > is a very technical reason > > I'm really sorry I'm not trying to be difficult its just over the years I

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

klimek added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2546-2560 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) || (Style.SpaceBeforeParens != FormatStyle::SBPO_Never && -(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for,

[PATCH] D58404: [clang-format] Add basic support for formatting C# files

klimek added inline comments. Comment at: include/clang/Basic/TokenKinds.h:80 K == tok::utf8_string_literal || K == tok::utf16_string_literal || - K == tok::utf32_string_literal; } This change looks pretty good, minus the changes outside

[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

klimek added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:401 + * ``SIS_AlwaysNoElse`` (in configuration: ``AlwaysNoElse``) +Allow short if/else if statements even if the else is a compound statement. + I'd try to make this either

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

klimek added a comment. In D55170#1432832 , @reuk wrote: > @klimek I agree that the rule is somewhat arbitrary. However, it's the style > rule of an established codebase with many users (I don't have a concrete > number, but the project has 1400 stars

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

klimek added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:436 +static void +AlignTokenSequenceAll(unsigned Start, unsigned End, unsigned Column, + F &, VelocityRa wrote: > klimek wrote: > > I don't think the 'All' postfix in

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

klimek added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2976 + +return Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None || + Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline || If SLS_All is

[PATCH] D59546: [clang-format] structured binding in range for detected as Objective C

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59546/new/ https://reviews.llvm.org/D59546 ___ cfe-commits mailing list

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

klimek added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2546-2560 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) || (Style.SpaceBeforeParens != FormatStyle::SBPO_Never && -(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for,

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

klimek added a comment. I don't understand yet why running clang-format locally would not do the same change? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54881/new/ https://reviews.llvm.org/D54881 ___ cfe-commits mailing list

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

klimek added a comment. In D55170#1431854 , @MyDeveloperDay wrote: > > I'm sorry for not having a positive answer here, but I'm not in favor of > > this change. The style rule looks like it introduces arbitrary complexity > > at a point where I don't

[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line

klimek added a comment. In D40988#1430502 , @MyDeveloperDay wrote: > So @russellmcc you've been bumping along this road nicely for 6 months, > doing what people say... pinging every week or so in order to get your code > reviewed, and you are getting

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57687/new/ https://reviews.llvm.org/D57687 ___ cfe-commits mailing list

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

klimek added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:482 + // containing any matching token to be aligned and located after such token. + auto AlignCurrentSequence = [&] { +if (StartOfSequence > 0 && StartOfSequence < EndOfSequence) {

[PATCH] D59332: [clang-format] AlignConsecutiveDeclarations fails with attributes

klimek added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:468 C.Tok->is(TT_FunctionDeclarationName) || + C.Tok->startsSequence( + tok::l_paren, tok::l_paren, tok::identifier, tok::coloncolon,

[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers

klimek added inline comments. Comment at: clang/include/clang/Format/Format.h:50 struct FormatStyle { + /// Indents after access modifiers. i.e. + /// \code I think we need to explain this a bit more: What this does is: Indent classes with access modifiers at

[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

klimek added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:401 + * ``SIS_AlwaysNoElse`` (in configuration: ``AlwaysNoElse``) +Allow short if/else if statements even if the else is a compound statement. + MyDeveloperDay wrote: > klimek

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

klimek added a comment. In D54881#1462804 , @russellmcc wrote: > Thanks for the explanation! I do understand your philosophy on this, and > agree with the desired behavior case you brought up where you have put in new > braces. > > After thinking about

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

klimek added a comment. In D54881#1462893 , @MyDeveloperDay wrote: > > I agree and would be happy with the change if it would only change the > > line-filtered workflow, but this afaict (unless I'm missing something :) > > will also affect the workflow

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

klimek added inline comments. Comment at: llvm/lib/Support/CommandLine.cpp:101 + SmallVector DefaultOptions; + A comment explaining what this contains and how it'll be used would help. Comment at: llvm/lib/Support/CommandLine.cpp:152 +

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

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

[PATCH] D37813: clang-format: better handle namespace macros

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37813/new/ https://reviews.llvm.org/D37813 ___ cfe-commits

[PATCH] D37813: clang-format: better handle namespace macros

klimek added inline comments. Comment at: unittests/Format/NamespaceEndCommentsFixerTest.cpp:526 EXPECT_EQ("namespace A {\n" " int i;\n" "} // namespace A", Typz wrote: > Should I also fix these tests? > > They already existed

[PATCH] D37813: clang-format: better handle namespace macros

klimek added inline comments. Comment at: unittests/Format/NamespaceEndCommentsFixerTest.cpp:582 + EXPECT_EQ("TESTSUITE() {\n" +" int i;\n" +"} // TESTSUITE()", All of the fixNamespaceEndComments tests are indented, but standard llvm

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In D37813#1512317 , @Typz wrote: > The patch goal is indeed to indent the content of namespace-macro blocks like > the content of any 'regular' namespace. So it should look like the content of > a namespace, possibly depending on

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In D37813#1254891 , @Typz wrote: > In D37813#1254865 , @klimek wrote: > > > In D37813#1253813 , @Typz wrote: > > > > > In D37813#1184051

[PATCH] D41911: [clangd] Include scanner that finds compile commands for headers.

klimek added inline comments. Comment at: clangd/IncludeScanner.cpp:75 + bool WasEmpty = Queue.empty(); + for (const auto : Cmds) { +QueueEntry E(Cmd, VFS); Usually I'd try to not lock a loop, as a large number of compile commands now blocks other

[PATCH] D63734: Update CODE_OWNERS.txt for clang-doc

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63734/new/ https://reviews.llvm.org/D63734 ___ cfe-commits mailing list

[PATCH] D59746: [LibTooling] Fix '-h' option

klimek added a comment. In D59746#1458539 , @hintonda wrote: > In D59746#1458461 , @hintonda wrote: > > > In D59746#1458432 , @klimek wrote: > > > > > If we make it an alias

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

klimek added a comment. In D54881#1449848 , @russellmcc wrote: > klimek: I'm sorry, I don't fully understand your proposed fix. Could you > explain it in more detail? > > Without being able to respond to your proposal in detail, I strongly believe > if

[PATCH] D66462: Removed the 'id' AST matcher, which is superseded by '.bind()'

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. Yay, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66462/new/ https://reviews.llvm.org/D66462

<    1   2   3   4   5   6   >