clang-format: constructor initializers are aligned in an hard-to-read way #5068

For example:

-NetworkStyle::NetworkStyle(const QString &appName, const QString &appIcon, const char *titleAddText, const QString &splashImage):
-    appName(appName),
-    appIcon(appIcon),
-    titleAddText(qApp->translate("SplashScreen", titleAddText)),
-    splashImage(splashImage)
+NetworkStyle::NetworkStyle(const QString& appName, const QString& appIcon, const char* titleAddText, const QString& splashImage) : appName(appName),
+                                                                                                                                   appIcon(appIcon),
+                                                                                                                                   titleAddText(qApp->translate("SplashScreen", titleAddText)),
+                                                                                                                                   splashImage(splashImage)

Aligning the arguments all the way to the right doesn't look too good in the case of a long constructor. I'd prefer if they started on the next line instead, and have the normal indentation. @sipa Is this possible with clang-format?

This is not a fluke; I've noted this before (https://github.com/bitcoin/bitcoin/pull/4933#issuecomment-56338845) as well.

Looks like there is an option for this (http://clang.llvm.org/docs/ClangFormatStyleOptions.html):

ConstructorInitializerIndentWidth (unsigned)
The number of characters to use for indentation of constructor initializer lists.

clang-format is not fully deterministic between versions and platforms,.. also it would produce relatively high noise (diffs).

A better way would be to provide a simple script in /contrib that would call clang-format before git commit (hook) or something similar. Most code formatting nits then would be gone (and more space for the important discussions).

After

-ColumnLimit:     0
+ColumnLimit:     80

the clang-format indents the constructor nicely:

NetworkStyle::NetworkStyle(const QString& appName,
                           const int iconColorHueShift,
                           const int iconColorSaturationReduction,
                           const char* titleAddText)
    : appName(appName),
      titleAddText(qApp->translate("SplashScreen", titleAddText))
{

Is it what you expect?

Yes, but ColumnLimit:80 will also introduce diff in all lines longer than 80 chars which is not wanted, imo.

@MarcoFalke IMHO the only reason to ask for the alignment is if you have a problem with the ColumnLimit anyway. I agree with clang's attempt at consistency here, either you have the limit or you don't.

If there was a limit, 120 would be my vote.

The nearest I could get:

If 59a83686fe483130d2f383e3e1cdf347810775df fixes this problem to everyone's satisfaction, I am happy to add the commit to #6839.

Example from OP:

-NetworkStyle::NetworkStyle(const QString &appName, const QString &appIcon, const char *titleAddText, const QString &splashImage):
-    appName(appName),
-    appIcon(appIcon),
-    titleAddText(qApp->translate(SplashScreen, titleAddText)),
-    splashImage(splashImage)
+NetworkStyle::NetworkStyle(const QString& appName, const QString& appIcon, const char* titleAddText, const QString& splashImage)
+    : appName(appName)
+    , appIcon(appIcon)
+    , titleAddText(qApp->translate(SplashScreen, titleAddText))
+    , splashImage(splashImage)

@MarcoFalke what if 120 was chosen as the column limit? Or even 160? I feel like both would solve this issue and hopefully not introduce too many changes.

As I said, this will combine/break a lot of lines (also strings). Moreover, I think it's bad to hardcode the line width; Usually the developer theirself or the developer's software takes care of properly displaying (or breaking) long lines.

• 120 (1224eeb58a95aecb0ae6bb238402c8cd2ed0e954)

-NetworkStyle::NetworkStyle(const QString &appName, const QString &appIcon, const char *titleAddText, const QString &splashImage):
-    appName(appName),
-    appIcon(appIcon),
-    titleAddText(qApp->translate(SplashScreen, titleAddText)),
-    splashImage(splashImage)
+NetworkStyle::NetworkStyle(const QString& appName,
+    const QString& appIcon,
+    const char* titleAddText,
+    const QString& splashImage)
+    : appName(appName), appIcon(appIcon), titleAddText(qApp->translate(SplashScreen, titleAddText)),
+      splashImage(splashImage)

• 160 (ccebf2511134371146ae5329629ac5984660e593)

-NetworkStyle::NetworkStyle(const QString& appName,
-    const QString& appIcon,
-    const char* titleAddText,
-    const QString& splashImage)
-    : appName(appName), appIcon(appIcon), titleAddText(qApp->translate(SplashScreen, titleAddText)),
-      splashImage(splashImage)
+NetworkStyle::NetworkStyle(const QString& appName, const QString& appIcon, const char* titleAddText, const QString& splashImage)
+    : appName(appName), appIcon(appIcon), titleAddText(qApp->translate(SplashScreen, titleAddText)), splashImage(splashImage)

I woupd personally like to have an enforced max-line length rule eventually, whether it's 80, 100, or 120 characters. I think that 160 is excessive, though.

I don't think that should happen immediately, as it will cause large amounts of conflicting changes in the code base, though.