Page MenuHomePhabricator

Improve clang-format support
Open, WishlistPublic

Description

Visual Studio is going to support it in the IDE:

https://blogs.msdn.microsoft.com/vcblog/2018/03/13/clangformat-support-in-visual-studio-2017-15-7-preview-1/

@kislinsk Do you have that VS version available for testing?

In addition, a "make format" target would be nice which calls clang-format in the correct way.

@goch I remember there were a fewe things that couldn't/shouldn't applied automatically, can you point to them here?

Related Objects

Event Timeline

kislinsk triaged this task as Wishlist priority.Mar 15 2018, 3:58 PM

I just picked this up in a small experiment:

find . -name \*.cpp -o -name \*.h | xargs clang-format -i
git diff --stat | sort -n -k 3 -r

in the MITK source tree presents you with a sorted list of the files with the biggest diff to our current clang-format config. The berry related diffs are mainly caused by (missing) namespace indentation:

Modules/CppMicroServices/third_party/jsoncpp.cpp   | 6439 +++++++++-----------
.../src/internal/berryWorkbenchPage.cpp            | 6131 +++++++++----------
.../src/internal/berryWorkbenchWindow.cpp          | 3123 +++++-----
.../src/internal/berryWorkbench.cpp                | 3117 +++++-----
.../src/internal/berryPerspective.cpp              | 2984 +++++----
Modules/CppMicroServices/third_party/jsoncpp.h     | 2837 +++++----
.../src/internal/berryWorkbenchRegistryConstants.h | 2490 ++++----
.../src/internal/berryPartStack.cpp                | 2283 ++++---
.../src/internal/berryWorkbenchPage.h              | 2273 ++++---
Modules/Classification/CLLibSVM/src/svm.cpp        | 2266 +++----
.../src/internal/berryPerspectiveHelper.cpp        | 2265 ++++---
.../src/internal/berryEditorManager.cpp            | 2248 ++++---
.../src/internal/berryEditorRegistry.cpp           | 2186 ++++---
.../src/internal/berryExtensionRegistry.cpp        | 2178 +++----
.../Testing/mitkImageStatisticsCalculatorTest.cpp  | 2030 +++---
.../src/internal/berryJobManager.cpp               | 1960 +++---
.../src/actions/berryQActionContributionItem.cpp   | 1823 +++---
Utilities/IpPic/mitkIpPicTypeMultiplex.h           | 1815 ++++--
.../src/internal/berryPartSashContainer.cpp        | 1784 +++---
.../src/internal/berryWorkbenchMenuService.cpp     | 1722 +++---
.../org.blueberry.ui.qt/src/berryIWorkbenchPage.h  | 1660 +++--
.../src/internal/QmitkSegmentationView.cpp         | 1500 ++---
.../src/internal/berryEditorRegistry.h             | 1370 +++--
.../src/internal/QmitkBasicImageProcessingView.cpp | 1352 ++--
.../src/internal/berryRegistryObjectManager.cpp    | 1337 ++--

changing the config to "indent only inner namespaces" leads to a nicer result:

Modules/CppMicroServices/third_party/jsoncpp.cpp   | 5341 +++++++++-----------
.../src/legacy/mitkDicomSeriesReader.cpp           | 3014 ++++++-----
Modules/CppMicroServices/third_party/jsoncpp.h     | 2805 +++++-----
Modules/Classification/CLLibSVM/src/svm.cpp        | 2266 ++++-----
.../Testing/mitkImageStatisticsCalculatorTest.cpp  | 2030 ++++----
Utilities/IpPic/mitkIpPicTypeMultiplex.h           | 1815 ++++---
.../DICOMReader/src/legacy/mitkDicomSeriesReader.h | 1614 +++---
.../Core/src/DataManagement/mitkPlaneGeometry.cpp  | 1555 +++---
.../mitkColourImageProcessor.cpp                   | 1535 +++---
.../src/internal/QmitkSegmentationView.cpp         | 1500 +++---
Modules/Core/include/mitkInteractionConst.h        | 1468 +++---
.../src/internal/berryPerspective.cpp              | 1467 +++---
.../src/internal/berryEditorManager.cpp            | 1438 +++---
.../src/internal/QmitkBasicImageProcessingView.cpp | 1352 +++--
Modules/Core/include/mitkImage.h                   | 1348 +++--
.../src/berryIPreferences.h                        | 1317 +++--
Modules/AppUtil/src/mitkBaseApplication.cpp        | 1266 +++--
Modules/IGT/TrackingDevices/mitkNDIProtocol.cpp    | 1265 ++---
Modules/Core/include/mitkBaseGeometry.h            | 1242 +++--
.../src/internal/berryPartStack.h                  | 1234 ++---
Modules/CameraCalibration/mitkTransform.cpp        | 1218 +++--
Modules/PhotoacousticsLib/MitkMCxyz/MitkMCxyz.cpp  | 1191 +++--
.../QmitkMatchPointRegistrationVisualizer.cpp      | 1185 +++--
.../src/internal/PerfusionDataSimulationView.cpp   | 1166 +++--
Modules/Core/src/IO/mitkIOUtil.cpp                 | 1156 ++---

Any opinions or preferences?

Next step would be to start applying the config on a regular basis

For the transition process, these bits from the git merge manpage could be useful as well:

ignore-space-change, ignore-all-space, ignore-space-at-eol, ignore-cr-at-eol
               Treats lines with the indicated type of whitespace change as unchanged for the sake of a three-way merge. Whitespace changes mixed with other changes to a line are
               not ignored. See also git-diff(1) -b, -w, --ignore-space-at-eol, and --ignore-cr-at-eol.

               ·   If their version only introduces whitespace changes to a line, our version is used;

               ·   If our version introduces whitespace changes but their version includes a substantial change, their version is used;

               ·   Otherwise, the merge proceeds in the usual way.

Mostly as a reminder I tried on top of the branch above:

NamespaceIndentation: All
PointerAlignment: Left

Adding @kalali to the subscribers as he also is interested in the definition of our clang format file.