I'd like to mention clang-format-diff, which can be used to format only code touched by a patch to introduce formatting changes gradually. It proved itself useful for me on several occasions.

On the current style: I think people mainly hate for( i = 0; i < splitDim; i++ ) "spaces in parenthesis rule". This rule is largely ignored in the code base, even in the new code (e.g. dnn).

The second most inconsistency is IMHO in brace wrapping e.g.

if () {
}

vs

if ()
{
}

some files use the first style some use the second style.

I'd like to mention clang-format-diff, which can be used to format only code touched by a patch to introduce formatting changes gradually. It proved itself useful for me on several occasions.

That's a useful utility. In addition, clang-format has an option
-lines=<string> to format a range of lines.

On the current style: I think people mainly hate for( i = 0; i < splitDim; i++ ) "spaces in parenthesis rule". This rule is largely ignored in the code base, even in the new code (e.g. dnn).

I also vote for SpacesInParentheses: false, which set already by
BasedOnStyle: WebKit. I prefer for (...) instead of for(...). So
SpaceBeforeParens: ControlStatements is appealing than
SpaceBeforeParens: Never.

The second most inconsistency is IMHO in brace wrapping e.g.

if () {
}

vs

if ()
{
}

some files use the first style some use the second style.

The abve one is more compact and more beautiful in my personal view.
BreakBeforeBraces: Mozilla seems more similar to OpenCV's preference.

Comments on my view of AlignAfterOpenBracket and ColumnLimit are
also needed.

Here is update to my proposal. The final decision should be made by
OpenCV team.

# clang-format 3.9 later
---
Language: Cpp
BasedOnStyle: WebKit
AlignAfterOpenBracket: Align
AllowAllParametersOfDeclarationOnNextLine: false
AlwaysBreakTemplateDeclarations: true
BreakBeforeBraces: Mozilla
ColumnLimit: 78
KeepEmptyLinesAtTheStartOfBlocks: false
NamespaceIndentation: None
...

BreakBeforeBraces is also connected to "mandatory braces" rule i.e. if you put braces around one-line expressions or not. Always using braces mitigates errors like

if ()
    expr1;
    expr2;

Current coding style guide snippet uses mandatory braces. However, again this is largely ignored in the codebase (even in new code like dnn). From my experience, it is only feasible to enforce mandatory braces with the first brace wrapping style. With the second brace wrapping style it seems people don't want to add those two extra lines to one-liners. It is no surprise that you will find many one-liners without braces in files that use the second brace wrapping style, even when those files adhere to the rest of the code style guide.

if ()
    expr1;
    expr2;

clang-format will format the above code into

if ()
    expr1;
expr2;

So this kind of human errors will be more visually detectable after
automatic formatting, which makes "mandatory braces" not so much needed.

BTW, recent compilers will produce a warning (GCC7):

test.cpp:100: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
     if (...)
     ^~
test.cpp:102: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
         expr2;
         ^~~~~

Since OpenCV 4.0 is switching to C++11/C++14,
using Standard: Cpp11 to make format compatible with C++11 standard is
more appealing.

update

# clang-format 3.9 later
---
Language: Cpp
BasedOnStyle: WebKit
AlignAfterOpenBracket: Align
AllowAllParametersOfDeclarationOnNextLine: false
AlwaysBreakTemplateDeclarations: true
BreakBeforeBraces: Mozilla
ColumnLimit: 78
KeepEmptyLinesAtTheStartOfBlocks: false
NamespaceIndentation: None
Standard: Cpp11
...

We can compare proposed styles:

find modules -iname *.cpp -or -iname *.hpp -or -iname *.h | xargs clang-format -i -style=file
git diff --shortstat

For example, the config in the PR gives us:

1213 files changed, 324380 insertions(+), 283268 deletions(-)

And the config from the last comment:

1247 files changed, 412979 insertions(+), 335167 deletions(-)

For core, imgproc only:

# from PR
 449 files changed, 126528 insertions(+), 105896 deletions(-)

# from last comment
 460 files changed, 170994 insertions(+), 134424 deletions(-)

A commit with huge diffs will be introduced if all source code are
reformatted at once. But if formatting changes are introduced gradually,
more inconsistencies will emerge in a medium term. Perhaps we have to
choose one of them.

Config in my latest comment is more aggressive to enforce a fresher
style which is used in many latest added code. And the PR is
conservative in order to comply with the current coding style guide
snippet.

There're also some interesting utilities to generate a .clang-format
file from example code-base. The config file that the reformatted source
fits its original formatting as closely as possible will be generated.

Ref.

[1] https://github.com/mikr/whatstyle
[2] https://github.com/johnmcfarlane/unformat

a huge commit that reformats all the OpenCV source is definitely not an option.

If we could smoothly integrate clang-format-diff into our CI, probably, it will be the way to go. Possibly, we could even add a heuristic - when there are many changes in a single source file, the whole file may be reformatted. Of course, we should try it first somehow.

Another question - once some parts of OpenCV are brought to the new consistent style, how do we enforce that the patches do not break it?

On the braces. Maybe there are some places where we use

if() {
}

and it's an exception. Let's use

if()
{
}

whatever the style is called. Let's call it OpenCV style, I have no problems with it.

Also, I do not see anywhere in the style description that spaces should be used instead of TAB and the indentation is 4 spaces. Probably, it should be added as well.

Column limit 100 may be a bit too limiting in some cases. Shall we put any restriction? Or maybe we should raise it to 120 or 128 or something like that?

• this file is useful
• I believe we should not enforce coding style immediately:
  • there are no reasons to massively touch legacy code.
  • 3rdparty code has own coding style and should not be changed.
• lets keep it as a recommendation for newcomers

namespaces

Here:
https://stackoverflow.com/questions/38986769/how-to-make-clang-format-keep-nested-namespaces-on-the-same-line

I do not see anywhere in the style description that spaces should be
used instead of TAB and the indentation is 4 spaces.

BasedOnStyle: WebKit will set default values of IndentWidth: 4 and
UseTab: Never. Hence extra settings are not necessary.

• CompactNamespaces: true

This ensure consecutive namespace declarations be on the same line. But
this option only available after clang-format 5. The above
configuration file was generated with clang-format 3.9. Also there are
cases where different versions of clang-format might format code
slightly different.

Here is an example showing clang-format-3.9 format code different to
clang-format-6 with regard to the same .clang-format configuration
file.

         if (board.type == BoardInfo::CHESSBOARD && found) {
             // improve the found corners' coordinate accuracy for chessboard
             cv::cornerSubPix(cvI, pointBuf, cv::Size(winSize, winSize),
-                cv::Size(-1, -1),
-                cv::TermCriteria(
-                    cv::TermCriteria::EPS + cv::TermCriteria::COUNT, 300,
-                    0.0001));
+                cv::Size(-1, -1), cv::TermCriteria(cv::TermCriteria::EPS
+                                          + cv::TermCriteria::COUNT,
+                                      300, 0.0001));
         }

My suggested .clang-format acts only as an initial effort to keep a
consistent style. I think the integration process might be following.

• After careful consideration, .clang-format file is introduced
  dictatorially by the core team. A minimum or fixed version of
  clang-format should be declared.
• Instructions on how to enforce the consistent format are added to
  the Wiki page of Coding Style Guide.
  • Newly added non-3rdparty source files must be formatted with
    clang-format before committing.
  • Patches are encouraged to be formatted with clang-format-diff
    or clang-format -lines=<string> before committing. Or patches
    should be consistent with style of the original file.

ok, can the file be moved to some subdirectory instead of the root directory?

Probably not, example: https://github.com/google/googletest/

When using -style=file, clang-format for each input file will try to
find the .clang-format file located in the closest parent directory of
the input file.

.clang-format in the top most directory is more versatile than in
other locations.

ok, I'm fine with it. @alalek, I think, it can be put in.

This PR should be further refined by core team before being merged, and xoox:clang-format branch can be edited (add commits) directly by opencv maintainers. Or a new PR could be created by the core team.

@vpisarev , @alalek , please take a look at some examples of code after applying style:

master...mshabunin:style-core
master...mshabunin:style-dnn
master...mshabunin:style-imgproc
master...mshabunin:style-features2d
master...mshabunin:style-ml
master...mshabunin:style-imgcodecs
master...mshabunin:style-videoio

I used the following format (differs from this PR) and clang-format 8.0.0:

AccessModifierOffset: -4
AlignAfterOpenBracket: Align
AlignEscapedNewlines: DontAlign
AlignOperands: true
AlignTrailingComments: false
AllowAllParametersOfDeclarationOnNextLine: false
AllowShortFunctionsOnASingleLine: All
AllowShortIfStatementsOnASingleLine: false
AlwaysBreakTemplateDeclarations: true
BinPackArguments: true
BinPackParameters: true
BraceWrapping:
  AfterClass: true
  AfterControlStatement: true
  AfterFunction: true
  AfterStruct: true
  AfterUnion: true
  AfterEnum: true
  BeforeCatch: true
  BeforeElse: true
BreakBeforeBinaryOperators: NonAssignment
BreakBeforeBraces: Custom
ColumnLimit: 99
CompactNamespaces: true
DerivePointerAlignment: true
FixNamespaceComments: true
IndentCaseLabels: false
IndentPPDirectives: AfterHash
IndentWidth: 4
MaxEmptyLinesToKeep: 2
NamespaceIndentation: None
PenaltyBreakAssignment: 50
PenaltyBreakBeforeFirstCallParameter: 50
PenaltyExcessCharacter: 10
PointerAlignment: Left
ReflowComments: false
SortIncludes: false
SortUsingDeclarations: false
SpaceAfterTemplateKeyword: false
SpacesBeforeTrailingComments: 1
UseTab: Never

@mshabunin, thank you very much for doing this experiments. I looked at the produced diff files. In my opinion, ~90% or even ~95% of the changes the script made are unnecessary. I suggest to make the formatting script much, much more liberal to the current formatting. Most of the time it should just keep the existing formatting.

@mshabunin Please update file in this patch with proposed changes. And I believe we can merge it (but without any forcing to existed code).

I used the following format (differs from this PR) and clang-format 8.0.0

clang (llvm) 8.0 is not officially released yet. It's recommended to use
a specific stable release. Official releases with pre-built binaries or
binary packages distributed with OS is most users' choice.

@mshabunin Could you reduce the format configuration by adding an item
of BasedOnStyle? Fortunately most of the options would be covered by a
predefined style (LLVM, Google, Chromium, Mozilla, WebKit). Using
BasedOnStyle will make the format configuration file lean and neat.

The leading lines are suggested as following. The first line is comment
of what version of clang-format is need. The second line --- is a
start indicator of YAML data. The third line specifies the scope of this
format configuration applies to. The fourth line indicates the base
style. Then other options are listed alphabetically bellow. The last
line, the YAML file ends with ... or --- indicator.

# clang-format 3.9 later
---
Language: Cpp
BasedOnStyle: WebKit
# all other options listed alphabetically bellow
...

Till the formatting is checked/enforced by CI the patch is useless. @alalek @vpisarev @vladimir-dudnik @asmorkalov discussed the solution offline and decided not to merge the solution. See @vpisarev comments above.