D24568: Provide clang-format target with a KDE Frameworks style file

cullmann retitled this revision from "Provide clang-format target with a common KDE style file" to "Provide clang-format target with a KDE Frameworks style file". REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann, #frameworks Cc: ochurlaud,

D24568: Provide clang-format target with a KDE Frameworks style file

ochurlaud added a comment. In D24568#546289 , @kossebau wrote: > There is also https://techbase.kde.org/Policies/Frameworks_Coding_Style which though missed the move from techbase to community, other than the other policies. > > I suspect th

D24568: Provide clang-format target with a KDE Frameworks style file

mwolff added a comment. In D24568#545942 , @cullmann wrote: > In D24568#545736 , @apol wrote: > > > I'm not sure how this works, but would it be possible to have a target that only works on a patch?

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added a reviewer: dfaure. cullmann added a comment. Perhaps David could give feedback if the file actually captures the intend to do proper KDE Frameworks/libs like formatting. I had a mistake with the indented case statements, that should be fixed. REPOSITORY R240 Extra CMake Mo

D24568: Provide clang-format target with a KDE Frameworks style file

dfaure added a comment. Do we want these, found in https://code.qt.io/cgit/qt/qt5.git/tree/_clang-format? # We use template< without space. SpaceAfterTemplateKeyword: false # macros for which the opening brace stays attached. ForEachMacros: [ foreach, Q_FOREACH, BOOST

D24568: Provide clang-format target with a KDE Frameworks style file

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I like the idea very much. INLINE COMMENTS > KDEClangFormat.cmake:76 > +else() > +message(STATUS "Could not set up the clang-format target as the > clang-format execu

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann updated this revision to Diff 68091. cullmann marked 4 inline comments as done. cullmann added a comment. - fix coding style issue, we don't want indented case labels - add initial docs - adjust style - just tell the user it will not work REPOSITORY R240 Extra CMake Modules C

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added a comment. In D24568#548490 , @dfaure wrote: > Do we want these, found in https://code.qt.io/cgit/qt/qt5.git/tree/_clang-format? > > # We use template< without space. > SpaceAfterTemplateKeyword: false > > # macro

D24568: Provide clang-format target with a KDE Frameworks style file

sitter added inline comments. INLINE COMMENTS > KDEClangFormat.cmake:53 > +# try to find clang-format in path > +find_program(KDE_CLANG_FORMAT_EXECUTABLE clang-format) > + I'm pretty sure you need to check the version the exectuable. When I use 6.0 I get ctors smushed into one line. REPOSITORY

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann marked an inline comment as done. cullmann added a comment. With BreakConstructorInitializers: BeforeColon you get collapsed stuff like; Range::Range(const KTextEditor::Cursor &c1, const KTextEditor::Cursor c2, MotionType mt) : Range(c1.line(), c1.column(), c2.line(), c

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann updated this revision to Diff 68186. cullmann marked an inline comment as done. cullmann added a comment. - avoid collapsing of constructor initializer lines REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24568?vs=68091&id=68186 BRANCH

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added a comment. Without the initializer change, the file works for me reasonable well, tried it again on KTextEditor. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann, #frameworks, dfaure Cc: sitter, mwolff, ochurlaud, nalvare

D24568: Provide clang-format target with a KDE Frameworks style file

zzag added inline comments. INLINE COMMENTS > dfaure wrote in clang-format.cmake:75 > typo: ternary I've been always wondering how one should break long ternary operators when writing KF code. There are several ways to do it (a) w/o breaking (BreakBeforeTernaryOperators: false) const FooBar

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added a comment. Has somebody tested the current state? I really would like to get this going as in my eyes this will finally put a working solution in place for the coding style enforcing on at least frameworks. One can later still improve stuff like "shall the gitlab stuff

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added inline comments. INLINE COMMENTS > zzag wrote in clang-format.cmake:75 > I've been always wondering how one should break long ternary operators when > writing KF code. There are several ways to do it > > (a) w/o breaking (BreakBeforeTernaryOperators: false) > > const FooBar *f

D24568: Provide clang-format target with a KDE Frameworks style file

dfaure added inline comments. INLINE COMMENTS > cullmann wrote in clang-format.cmake:75 > I have no strong opinion on that, we can remove that or keep it. (b) looks good to me, and more importantly, I like the goal of being as close as possible to the Qt coding style. REPOSITORY R240 Extra C

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added inline comments. INLINE COMMENTS > dfaure wrote in clang-format.cmake:75 > (b) looks good to me, and more importantly, I like the goal of being as close > as possible to the Qt coding style. removed that setting, back to Webkit default of BreakBeforeTernaryOperators: true REPOS

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann updated this revision to Diff 68369. cullmann added a comment. - keep default of BreakBeforeTernaryOperators = true REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24568?vs=68186&id=68369 BRANCH master REVISION DETAIL https://phabr

D24568: Provide clang-format target with a KDE Frameworks style file

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R240 Extra CMake Modules BRANCH master REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann, #frameworks, dfaure Cc: zzag, sitter, mwolff, ochurlaud, nalvarez, kossebau, aacid, davidedmun

D24568: Provide clang-format target with a KDE Frameworks style file

This revision was automatically updated to reflect the committed changes. cullmann marked an inline comment as done. Closed by commit R240:235fcabf5018: Provide clang-format target with a KDE Frameworks style file (authored by cullmann). REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST U

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added a comment. Ok, I pushed this now. I will use that myself in KTextEditor and Kate instead of my old file and shell script. We can think about what to do with other frameworks now. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D245

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added a comment. Volker Ok'd it that I used that on the co-maintained KSyntaxHighlighting framework, too. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann, #frameworks, dfaure Cc: zzag, sitter, mwolff, ochurlaud, nalvarez, koss

D24568: Provide clang-format target with a KDE Frameworks style file

davidedmundson added a comment. As an update on this from the Plasma POV. I added the macro to every repo and told every dev to do a final test before we commit the formatted results. I had some feedback and the result was that we can't proceed with in the current state [1]. Wh

D24568: Provide clang-format target with a KDE Frameworks style file

dhaumann added a comment. You can force the current clang format to keep the multi-line if as follows: if (roles.isEmpty() // comment || roles.contains(Notifications::UpdatedRole) || roles.contains(Notifications::ExpiredRole) The comment prevents clang to joi

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added a comment. In D24568#563092 , @davidedmundson wrote: > As an update on this from the Plasma POV. > > I added the macro to every repo and told every dev to do a final test before we commit the formatted results. > > I had s

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann reopened this revision. cullmann added a comment. This revision is now accepted and ready to land. Let's just reopen this and work on improving it. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann, #frameworks, dfaure Cc: zzag,

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added reviewers: broulik, davidedmundson. cullmann added a comment. For the lambda issue, I think we can add: # keep lambda formatting multi-line if not empty AllowShortLambdasOnASingleLine: Empty see https://clang.llvm.org/docs/ClangFormatStyleOptions.html That fi

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added a comment. Btw., I just tried e.g. https://code.qt.io/cgit/qt/qt5.git/tree/_clang-format that one has exactly the same issues as our file and re-flows stuff into single lines (even lambdas). The only thing that often hinders that is the column limit of 100 there.

D24568: Provide clang-format target with a KDE Frameworks style file

kossebau added a comment. This has been missing the link from an rst file in docs/, so the documentation generation picks up the file. Fixed with c4890d5c03ed79f0c87da861b6608bbd46c2162c REPOSITORY R240 Extra CMa

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added a comment. In D24568#580338 , @kossebau wrote: > This has been missing the link from an rst file in docs/, so the documentation generation picks up the file. Fixed with c4890d5c03ed79f0c87da861b6608bbd46c2162c

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added a comment. Ping? Any update if somebody tried the proposed changes? REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann, #frameworks, dfaure, broulik, davidedmundson Cc: zzag, sitter, mwolff, ochurlaud, nalvarez, kossebau, aa

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann added a comment. In D24568#563653 , @cullmann wrote: > For the lambda issue, I think we can add: > > # keep lambda formatting multi-line if not empty > AllowShortLambdasOnASingleLine: Empty > > > see https://clang.llvm.org

D24568: Provide clang-format target with a KDE Frameworks style file

cullmann closed this revision. cullmann added a comment. For my 2 wanted changes, I created a new merge request https://invent.kde.org/frameworks/extra-cmake-modules/-/merge_requests/20 REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann,