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

kossebau added subscribers: nalvarez, ochurlaud. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann, #frameworks Cc: ochurlaud, nalvarez, kossebau, aacid, davidedmundson, dhaumann, apol, ognarb, kde-frameworks-devel, kde-buildsystem,

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

kossebau added a comment. 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 that page should be moved over now as well, to become the real KF coding style page (so

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

cullmann added a comment. I hope this file implements https://community.kde.org/Policies/Kdelibs_Coding_Style And that is the style all frameworks got astyled with on initial import. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To:

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

aacid added a comment. If the KDE Frameworks developers have agreed to a common style, then yes, naming it KDE Frameworks style makes sense :) REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann, #frameworks Cc: aacid, davidedmundson,

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

cullmann added a comment. In D24568#546227 , @aacid wrote: > > common KDE style file > > There's no such thing as a common KDE style Shall we name it kdelibs coding style? But actually the idea is to have one, as opt in, for the

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

aacid added a comment. > common KDE style file There's no such thing as a common KDE style REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann, #frameworks Cc: aacid, davidedmundson, dhaumann, apol, ognarb, kde-frameworks-devel,

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

cullmann added a comment. If there are more deviations from the kdelibs/frameworks coding style, please tell me. Otherwise, I am happy with the output of this file for KTextEditor & Kate. For the .clang-format instantiation question: I think it makes sense to instantiate it on

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

cullmann added a reviewer: Frameworks. cullmann marked an inline comment as done. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann, #frameworks Cc: davidedmundson, dhaumann, apol, ognarb, kde-frameworks-devel, kde-buildsystem, LeGast00n,

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

cullmann updated this revision to Diff 67797. cullmann added a comment. - add initial docs REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24568?vs=67796=67797 BRANCH master REVISION DETAIL https://phabricator.kde.org/D24568 AFFECTED

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

cullmann marked an inline comment as done. cullmann added inline comments. INLINE COMMENTS > ognarb wrote in KDEClangFormat.cmake:11 > need doc Added some initial docs REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann Cc: davidedmundson,

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

cullmann updated this revision to Diff 67796. cullmann added a comment. - fix coding style issue, we don't want indented case labels REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24568?vs=67739=67796 BRANCH master REVISION DETAIL

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

cullmann added inline comments. INLINE COMMENTS > davidedmundson wrote in KDEClangFormat.cmake:48 > it's a bit weird to put new files in the source directory when running cmake. > > Copying when running the target clang-format would probably be ok as there > you're already doing an explicit

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

davidedmundson added inline comments. INLINE COMMENTS > KDEClangFormat.cmake:48 > +if(KDE_CLANG_FORMAT_EXECUTABLE) > +configure_file(${CMAKE_CURRENT_LIST_DIR}/clang-format.cmake > ${CMAKE_CURRENT_SOURCE_DIR}/.clang-format @ONLY) > +endif() it's a bit weird to put new files in the source

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

cullmann added a comment. 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? You usually want to make sure what you modified didn't diverge from the code.

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

dhaumann added a comment. I'm all for it. This would unify how we can reformat any KDE module, which is very much desirable. Being able to just reformat a patch sounds interesting, too, but can be added later still. Don't let the perfect be the enemy of the good... The example

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

ognarb added a comment. There is probably a way to run clang-format only on a patch. See http://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting But should we not prefer running clang format one time, instead of having only some part of the code auto-formatted?

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

apol added a comment. I'm not sure how this works, but would it be possible to have a target that only works on a patch? You usually want to make sure what you modified didn't diverge from the code. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568

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

ognarb added inline comments. INLINE COMMENTS > KDEClangFormat.cmake:11 > +# > +# :: > +# need doc REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24568 To: cullmann Cc: ognarb, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, michaelh,

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

cullmann created this revision. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. cullmann requested review of this revision. REVISION SUMMARY Provides a clang-format target if wanted Example usage: