Add clang-format file

[piponazo]

In this PR I propose the addition of a clang-format file for defining the style of the Exiv2 code.

I tried to be conservative and the options selected are matching (more or less) the style that was used in most parts of the project. Of course, we can discuss about some of these values and change them accordingly to our discussion :).

I also applied these changes to one complete file (that was small) and to few lines of other file. Modern IDEs (like QtCreator) have shortcuts to apply clang-format in a whole file or to the selected text.

You can find more information about clang-format in the following links:

• https://clang.llvm.org/docs/ClangFormat.html
• https://clang.llvm.org/docs/ClangFormatStyleOptions.html

[piponazo]

Note that this PR is a first step to address the issue #106

[D4N]

This setting will reformat double closing > braces in templates to >> which will be interpreted as a shift operator in C++03.

[piponazo]

Wow, I did not notice that. I will change it. Thanks!

[piponazo]

I just addressed this issue. I will wait for your approval to merge this ;)

[D4N]

Very good working through all the options clang format offers! (I am usually a billion times lazier and stick with only slightly modified defaults.)
Yet, the diffs are still huge, especially if you apply clang format to a file with nested namespaces (try for instance src/tiffvisitor.cpp). But I think there is no way around it, as although there is a generally visible formatting style in exiv2, it is not following strict rules.

In principle I am very much in favor of this, as it would mean I can stop worrying about formatting and just press the key combo to format the source. But the diff is imho too big before a minor release. Maybe we could merge this after a bigger release?

[piponazo]

I would merge this now but without applying it to the whole code base.

At my company we started to use clang-format 2 years ago (introducing a .clang-format) file and we are still far from having all the code well formatted. Basically we follow the same guidelines:

• New files should follow the clang-format style.
• Old files will be completely re-formatted only if we need to touch several lines/functions/methods of that file. In that case, we first create a Pull Request re-formatting the files that will be touched. Later we create another Pull Request with the real changes.
• If we only need to fix a small portion of a file we do not apply clang-format at all, or we just do it in the code block that we touch.

We defined those guidelines after several iterations, and they are working pretty well in our case. I think it might be interesting to do something similar in Exiv2. When I write new code I like to have these utilities in place.

[D4N]

That is actually a very good approach. It will keep the diffs reasonable and will just result in a gradual migration.

[tbeu]

Why two blanks here?

[piponazo]

Good point. We can control this with the field:

SpacesBeforeTrailingComments: 2

I will change it to 1.

But ... should not we just remove these comments ? I think they are useless. If there are functions so long that somebody can think it is a good idea to have such comments, I would rather think that we should refactor the code to have smaller functions that can be read easily.

[tbeu]

Indeed, good point to get rid of such useless comments.

[D4N]

Yes, a good editor will tell you anyway and if you actually need such comments the function is too long. However, I'd keep these comments for namespaces and #ifdefs

[piponazo]

Note that I am adding individual commits to the small changes I am doing while discussing about the final configuration. Once we agree in a final configuration, I will squash all the commits.

[tbeu]

Personally I do not like braces AfterControlStatement=true these days. It also introduces new lines in your example file.

[tbeu]

One other non-conservative option is the new line length limit 120.

[piponazo]

@tbeu I have changed the configuration so now it is based on the google style (as we discussed on slack).

About the length limit, most of the existing files are respecting already this configuration. Examples:

• asfvideo.cpp:127 longest line with 116 characters.
• canonmn_int.h:91 longest line with 113 characters.

Of course, there are others files that have lines exceeding this limit, and in some cases it makes sense to apply the formatting and in others not.

[D4N]

I think this is good, the looks of the code will change, but it will be more consistent.

What's your opinion on lowering the line limit while removing indentations for namespaces? This will penalize spaghetti code with lots of indentation more and force refactoring just for the sake of readability. The Linux kernel has such a policy (1 indentation = 8 spaces, line limit = 80 spaces). However the diffs will be bigger, as we currently indent namespaces.

[piponazo]

In my case I always work with indentations for the namespaces, 4 spaces for indentations and a limit of 100 or 120 chars for each line. I think that removing the indentations from Exiv2 would make a huge diff even for the simpler changes, but again, this PR is for discussing about all these things.

My vote is for staying with the indentation for namespaces, but if there are more people preferring the other option, I will change it.

[tbeu]

I have no strong opinion on the namespace identation, but rather would prefer conservative formating, i.e., do not touch it for now. That's why I approved the PR as is.

[piponazo]

Guys, I'm merging this so we can give it a try while we touch existing code or create new one. If we see that we can refine a bit more the clang-format file, do not hesitate to create new PRs to update it ;)

[D4N]

I think we should start the contributing.md file and document how clang format should be used.

[tbeu]

See http://dev.exiv2.org/projects/exiv2/wiki/Contributing_to_Exiv2 for what should go to Contributing.md.

[piponazo]

I created a new branch in which I started to write the CONTRIBUTING.md file. I will create a PR in the following days

[tbeu]

See also https://help.github.com/articles/setting-guidelines-for-repository-contributors/

[D4N]

Thanks for starting the work. Please make the PR sooner than later, as I suspect we'll have to discuss a lot of things.