[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

This revision was automatically updated to reflect the committed changes. ksyx marked 2 inline comments as done. Closed by commit rG5e5efd8a91f2: [clang-format] Fix SeparateDefinitionBlocks issues (authored by ksyx). Changed prior to commit: https://reviews.llvm.org/D117520?vs=402078&id=402498#

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

curdeius accepted this revision. curdeius added a comment. LGTM. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:141-145 +if (NextLine->MightBeFunctionDecl && +NextLine->mightBeFunctionDefinition()) + if (NextLine->First->NewlinesBefore ==

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. Just this small detail. :) Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:131 + + // A single line identifier that is not in the last line + if (OperateLine->First->is(tok::ident

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx marked 2 inline comments as done. ksyx added a comment. In D117520#3263403 , @MyDeveloperDay wrote: > maybe slightly related https://github.com/llvm/llvm-project/issues/53183 in > that this is also impacted by the 5 character > `TT_FunctionLikeOrF

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added a comment. maybe slightly related https://github.com/llvm/llvm-project/issues/53183 in that this is also impacted by the 5 character `TT_FunctionLikeOrFreestandingMacro` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 __

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:134 + OperateIndex + 1 < Lines.size()) { +// UnwrappedLineParser's recognition of free-standing macro like +// Q_OBJECT may also recognize some uppercased type names tha

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx updated this revision to Diff 402078. ksyx marked an inline comment as done. ksyx added a comment. Apply review suggestions of renaming. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 Files: clang/lib/Format/DefinitionBlockSeparator.cp

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx marked an inline comment as done. ksyx added inline comments. Comment at: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp:47 llvm::StringRef ExpectedCode = "") { ::testing::ScopedTrace t(File, Line, ::testing::Message() << Code.str()

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:134 + OperateIndex + 1 < Lines.size()) { +// UnwrappedLineParser's recognition of free-standing macro like +// Q_OBJECT may also recognize some uppercased

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx updated this revision to Diff 402006. ksyx edited the summary of this revision. ksyx added a comment. Add token type `FunctionLikeOrFreestandingMacro` and use it to replace duplicated check with UnwrappedLineParser CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

curdeius added a comment. I'd like to see the refactor of the parts repeated from UnwrappedLineParser. No comments otherwise. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:134 + OperateIndex + 1 < Lines.size()) { +// UnwrappedLineParser's recognitio

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx added a comment. In D117520#3260443 , @curdeius wrote: > In D117520#3258454 , > @MyDeveloperDay wrote: > >> I think its been fixed, in the last but one diff. >> >> Generally speaking we simply cannot have --

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

curdeius added a comment. In D117520#3258454 , @MyDeveloperDay wrote: > I think its been fixed, in the last but one diff. > > Generally speaking we simply cannot have --output-replacements-xml , -n or > --dry-run show replacements that amount to nothing

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:135 +// UnwrappedLineParser's recognition of free-standing macro like +// Q_OBJECT may also recognize some uppercased type names that may be +// used as ret

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx updated this revision to Diff 401659. ksyx added a comment. Add unit test for formatting conflict CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 Files: clang/lib/Format/DefinitionBlockSeparator.cpp clang/lib/Format/DefinitionBlockSep

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added a comment. I think its been fixed, in the last but one diff. Generally speaking we simply cannot have --output-replacements-xml , -n or --dry-run show replacements that amount to nothing because this is the mechanism by which 100s of repos fail commits thinking their is a c

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

curdeius added a comment. In D117520#3251425 , @MyDeveloperDay wrote: > For > > class Test > { > public: > static void foo() > { > } > }; > > F21703157: image.png > > $ clang-format -n test

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx updated this revision to Diff 401460. ksyx added a comment. Apply suggestion from clangfmt CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 Files: clang/lib/Format/DefinitionBlockSeparator.cpp clang/lib/Format/DefinitionBlockSeparator.

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx updated this revision to Diff 401445. ksyx marked an inline comment as done. ksyx added a comment. - Apply suggestions from clangfmt - Split assertion conditions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 Files: clang/lib/Format/De

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:134 + OperateIndex + 1 < Lines.size()) { +// UnwrappedLineParser's recognition of free-standing macro like +// Q_OBJECT may also recognize some uppercased type names tha

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:120 const auto MayPrecedeDefinition = [&](const int Direction = -1) { + assert(Direction >= -1 && Direction <= 1); const size_t OperateIndex = OpeningLineIndex + D

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added a comment. Works nicely now... LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx updated this revision to Diff 401293. ksyx edited the summary of this revision. ksyx added a comment. Recognize long (len>=5) uppercased name taking a single line as return type and fix the problem of adding newline below it: void afunc(int x) { return; } TYPENAME func(int x, i

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added a comment. ok, this is better but not quite right still.. I though I mentioned this before (please add as a unit test) HRESULT Foo::bar() {} HRESULT Foo::bar() {} formats as HRESULT Foo::bar() {} HRESULT Foo::bar() {} If you are windows develop

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx updated this revision to Diff 401062. ksyx marked 2 inline comments as done. ksyx edited the summary of this revision. ksyx added a comment. - Resolves formatting conflict with options `EmptyLineAfterAccessModifier` and `EmptyLineBeforeAccessModifier` (prompts with `--dry-run` (`-n`) or `--

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:80 return; Whitespaces.replaceWhitespace(*TargetToken, NewlineToInsert, +TargetToken->OriginalColumn, HazardyKnusperkeks wrot

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:76 // Do not handle EOF newlines. if (TargetToken->is(tok::eof) && NewlineToInsert > 0) return; Is that needed? Comment a

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx added a comment. In D117520#3251425 , @MyDeveloperDay wrote: > For > > class Test > { > public: > static void foo() > { > } > }; > > F21703157: image.png > > $ clang-format -n test.cxx

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added a comment. For class Test { public: static void foo() { } }; F21703157: image.png $ clang-format -n test.cxx test.cxx:3:8: warning: code should be clang-formatted [-Wclang-format-violations ] public:

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx added a comment. In D117520#3251376 , @MyDeveloperDay wrote: > I'm getting some sort of seemingly empty replacements going on, after > `public:` I can't quite put my finger on what's happening. Not so sure what empty means here. Is it removing emp

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added a comment. I'm getting some sort of seemingly empty replacements going on, after `public:` I can't quite put my finger on what's happening. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 _

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM, I'll pull it down and try it here.. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 _

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx updated this revision to Diff 400821. ksyx marked 3 inline comments as done. ksyx edited the summary of this revision. ksyx added a comment. - Use function keyword for JavaScript instead of comparing strings - Apply review suggestions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

curdeius added a comment. Could you explain more in details the before/after of the fixed bugs, please? The best would be to develop the patch description. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:37 + auto LikelyDefinition = [this](const AnnotatedLine *Line,

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:44 + if (CurrentToken->isOneOf(tok::kw_class, tok::kw_struct) || (Style.isJavaScript() && CurrentToken->TokenText == "function")) return true;

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx updated this revision to Diff 400667. ksyx added a comment. Apply clangfmt's suggestion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 Files: clang/lib/Format/DefinitionBlockSeparator.cpp clang/unittests/Format/DefinitionBlockSepara

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

ksyx created this revision. ksyx added reviewers: MyDeveloperDay, curdeius, HazardyKnusperkeks. ksyx requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. - Fixes https://github.com/llvm/llvm-project/issues/53227 that wrongly indents multiline c