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

ksyx added a comment. In D116314#3221488 , @MyDeveloperDay wrote: > @ksyx > > Did you see this issue https://github.com/llvm/llvm-project/issues/52976 > > In its current form, this patch causes huge amounts of flux in projects > that document code like (

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

MyDeveloperDay added a comment. @ksyx Did you see this issue https://github.com/llvm/llvm-project/issues/52976 In its current form, this patch causes huge amounts of flux in projects that document code like (anyone using doxygen likely, + most normal people ;-) ) ... } // My function -

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

MyDeveloperDay added a comment. I'm seeing some odd behaviour here.. void foo() {} /* ABC */ void bar() {} becomes void foo() {} /* ABC */ void bar() {} If the ABC comment is a doxygen style comment it doesn't make sense to separate it from the function below Repository:

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6f6f88ffdae1: [clang-format] Style to separate definition blocks (authored by ksyx). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

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

curdeius accepted this revision. curdeius added a comment. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin

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

ksyx updated this revision to Diff 396926. ksyx added a comment. Add release note entry. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Format

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

MyDeveloperDay added a comment. Can you add a release note to docs/ReleaseNotes.rst CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

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

ksyx added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3398 +**SeparateDefinitionBlocks** (``SeparateDefinitionStyle``) :versionbadge:`clang-format 14` + Specifies the use of empty lines to separate definition blocks, including classes, --

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM thank you for this patch, I'll also definately use this myself. Comment at: clang/docs/ClangFormatStyleOptions.rst:3398 +**SeparateDefinitionBlocks** (``SeparateDefinitionStyle``) :versionbadge:`clan

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

ksyx updated this revision to Diff 396835. ksyx marked 3 inline comments as done. ksyx added a comment. Change the `SDS_Leave` style check to be assert not having this style. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 Files: clang/docs/

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

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:25 +FormatTokenLexer &Tokens) { + if (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Leave) +return {{}, 0}; ksyx wrote: > HazardyKnusperkeks wrote: >

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

ksyx marked an inline comment as done. ksyx added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:25 +FormatTokenLexer &Tokens) { + if (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Leave) +return {{}, 0}; HazardyKnusperke

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

ksyx updated this revision to Diff 396826. ksyx added a comment. Use `reformat` method instead of calling the single method only. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/c

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

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:25 +FormatTokenLexer &Tokens) { + if (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Leave) +return {{}, 0}; ksyx wrote: > HazardyKnusperkeks wrote: >

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

ksyx added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:25 +FormatTokenLexer &Tokens) { + if (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Leave) +return {{}, 0}; HazardyKnusperkeks wrote: > Better, but I still think w

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

ksyx updated this revision to Diff 396782. ksyx added a comment. Change lambda variable names' leading letter to uppercase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/F

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

ksyx updated this revision to Diff 396781. ksyx marked 5 inline comments as done. ksyx added a comment. Change lambda variable names' leading letter to uppercase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 Files: clang/docs/ClangFormatS

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

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:25 +FormatTokenLexer &Tokens) { + if (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Leave) +return {{}, 0}; Better, but I still think we should have

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

ksyx added inline comments. Comment at: clang/include/clang/Format/Format.h:4097 + ArrayRef Ranges, + StringRef FileName = ""); + HazardyKnusperkeks wrote: > The only use

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

ksyx updated this revision to Diff 396727. ksyx marked 9 inline comments as done. ksyx added a comment. - Apply review suggestions. - Assert the style is not `SDS_Leave` in private method, while return no change in public method if so. - Rename loop variable `Line` to `CurrentLine` CHANGES SINC

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

HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added a comment. This revision now requires changes to proceed. Only some small changes, then it looks good to me. And I will definitely use it, once it is landed. Comment at: clang/include/clang/Format/

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

ksyx updated this revision to Diff 396709. ksyx marked 2 inline comments as done. ksyx added a comment. minor fixes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/F

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

HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:3104-3105 + /// + ///class C {} + ///}; + /// \endcode Comment at: clang/lib/

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

ksyx updated this revision to Diff 396577. ksyx added a comment. Apply suggestion from clangfmt CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/F

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

ksyx updated this revision to Diff 396575. ksyx added a comment. Apply clangfmt's suggestions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/For

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

ksyx updated this revision to Diff 396574. ksyx marked 9 inline comments as done. ksyx edited the summary of this revision. ksyx added a comment. - Improve compatibility to other languages - Improve unit test by test format stability, inverse result, and generate some input from expected output -

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

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] D116314: [clang-format] Add style to separate definition blocks

curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. Nice job. Some minor comments. Please don't forget to reformat too. Comment at: clang/docs/ClangFormatStyleOptions.rst:3415 + + SeparateDefinitions + Ne

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

ksyx updated this revision to Diff 396430. ksyx added a comment. Fix typo. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/Format/CMakeLists.txt

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

ksyx updated this revision to Diff 396428. ksyx marked 5 inline comments as done. ksyx added a comment. - Apply suggestions from clangfmt - Add missing period of comments - Fix namespace ending comment - Add comment for notes on return value of WhitespaceManager's method CHANGES SINCE LAST ACTIO

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

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] D116314: [clang-format] Add style to separate definition blocks

ksyx marked an inline comment as done. ksyx added inline comments. Comment at: clang/lib/Format/Format.cpp:2114 + if (Result.add(R)) +return; + } HazardyKnusperkeks wrote: > Why only add the first replacement? This worked well as the `Error` class `

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

ksyx updated this revision to Diff 396421. ksyx marked 4 inline comments as done. ksyx added a comment. Herald added a subscriber: mgorny. - Change version from 15 to 14 - Add option `SDS_Never`, rename `SDS_Between` => `SDS_Always` - Add unit tests - Separate class into new file CHANGES SINCE L

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

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