- User Since
- Aug 18 2015, 2:16 AM (204 w, 2 d)
Mon, Jul 15
I appreciate what you've done to make this patch less duplicated code..but now you've almost got to the point where being able to control it all via configuration.
Sun, Jul 14
There also seems like alot of duplication between the existing sortCppIncludes
Do you think there is anything about this algorithm that could be parameterized so that other projects could utilize it? I guess its not completely clear how this differs from just using the IncludeCategories?
Wed, Jul 10
Tue, Jun 25
It was more about not having time to pursue it at this time.. I didn't feel I should hog the PR if someone else wanted to take a look.
Thu, Jun 20
Jun 13 2019
Jun 12 2019
Nit: if you going for "OnePerLine" in the name rather than "Break" can you keep it consistent with ConstructorInitializerAllOnOneLineOrOnePerLine and put the type first and then the operation i.e. BitFieldOnePerLine (this will help keep all BitField options together if there are ever more than one), I also think either don't use Decl at all or use the full word Declarations as in AlignConsecutiveDeclarations , AlwaysBreakTemplateDeclarations etc
Jun 11 2019
mark sure you mark off the comments as you consider them done.
Jun 10 2019
Jun 9 2019
You need to add a unit test in clang/unittests
May 21 2019
Just a passing comment, Is this:
May 15 2019
May 14 2019
please simply remove line 249
May 10 2019
@klimek responded via the mailing list
When I spoke with one of the code owners, they seemed to have a problem accepting this review based on there not being a general description/understanding of how this algorithm works.
May 3 2019
May 1 2019
Thanks for the patch, thanks also for changing to https as I believe this was a change suggested in clang tidy docs too
Thanks for the patch, is this case only limited to *<tab>
Did this case some issue? Does this fix something if so can we add a test, because maybe the line isn't needed
Apr 27 2019
Apr 23 2019
This looks logical to me, seems to fit with what `WhitespaceManager::appendNewlineText` is doing
Apr 18 2019
Abandoning in favor of D60853: clang-format converts a keyword macro definition to a macro function
Apr 17 2019
@klimek one possible solution to this might be to replace the "keyword" back to an identifier in a '#define <keywoord>' scenario
Apr 15 2019
Apr 14 2019
use endsWith() as it ignored comments
Apr 11 2019
I suppose we could keep the names and directory structure and just change the namespace. That would just be a special case in the scripts. Haven't looked into it yet, but will do so as soon as I can.
Isn't that matching done on strings? I.e. is there difference between -llvm-* and -ll* ?
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 where the provided range is 0-length range, which has an implicit "format stuff around this" request from the user inside it. I'd be happy with a patch that differentiates these two sides.
are we supporting "-llvm-*" in existing .clang-tidy files?
Apr 9 2019
Apr 8 2019
Addressing review comments
Reduce test cases
@ownenpan might be worth checking with @reuk who has been adding some options for the JUCE style guide and looking at the JUCE code it seems this style might match their style.
Apr 7 2019
LGTM , if you also think the test will help show the use case then please add it, otherwise this revision notes might be information enough
maybe add the following as a test because I think it shows the inconsistency
LGTM (minor nit)
Apr 6 2019
@owenpan can I help you bring this back up to date and review, I think I saw another issue in the bugzilla requesting this.
Thanks for doing this, it LGTM, I like your solution better ;-)
Apr 5 2019
I actually also tend to change doc/ClangFormatStyleOptions.rst too
just a few nits, apart form that it LG
Apr 3 2019
Apr 1 2019
Mar 30 2019
Mar 27 2019
We normally add something to the documentation about the checker and/or the release notes to say what had changed
Mar 23 2019
Mar 22 2019
@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?
Mar 21 2019
Address comment nits
Addressing Review Comments
- Remove all non Format only code (i.e. changes to clang/Basic and clang/Lexer to support verbatim interpolated strings C# 6)
- For the most part the need to for them is removed by merging of tokens during the FormatLexer phase
- There is one obscure failure scenario, but for a first pass I think it can be ignored (tests commented out) where a \ is used unescaped just before a " (because of paths can be more common than you think)
- string s = @"ABC\" + ToString("B");";
Mar 20 2019
The cost is financial, as it's developer time, which costs real money to companies. In the end, to support this, people like myself who are doing this as part of their job spend time that they'd otherwise spend to make other things better that might be more important.
Sorry for the spam:
- write this a little better
Addressing code review comment
@klimek I think is what you meant, a much smaller and neater version of what I had before.