Clang format opt

[fghoussen]

Add options in clang-format.

Following:

• https://pcl.readthedocs.io/en/latest/pcl_style_guide.html#functions-methods
• https://pcl.readthedocs.io/en/latest/pcl_style_guide.html#id2

[mvieth]

Sorry, but this PR is far too large to merge. Currently, not all PCL modules are checked for correct formatting. See here for all modules that are checked in the CI. We are slowly adding more modules one by one, but the problem is, if we format a file for which an open pull request exists, that pull request will have merge conflicts, often so severe that they are impossible to resolve. We have made some experiments writing a script to find out which files are not modified by any PR and are therefore safe to be formatted. Regarding the space before parens option: I am unsure whether it is a good idea to enforce this rule from the style guide in the .clang-format file (given how many additional changes to already formatted modules this would mean), or whether we should rather update the style guide to reflect .clang-format. I don't know when the style guide was written or how much it was followed in existing code, so I think I would prefer updating it regarding the space before parens

[fghoussen]

pull request will have merge conflicts, often so severe that they are impossible to resolve.

OK, got it.

would prefer updating it regarding the space before parens

Let me know what to do: close the PR? Keep space before parens only? With or without modified files in the PR?

[mvieth]

would prefer updating it regarding the space before parens

Let me know what to do: close the PR? Keep space before parens only? With or without modified files in the PR?

If you like, you could update the style guide so that it follows the same rules as the current .clang-format, meaning no space before parens: exampleMethod (int example_arg); becomes exampleMethod(int example_arg); in the style guide etc.

[fghoussen]

you could update the style guide so that it follows the same rules as the current .clang-format

New to PCL and not so used to clang-format: this would take forever... Anyway, no big deal, I close this.

[fghoussen]

@mvieth: you may also merge only the .clang-format file and ask in each PR to run clang-format on modified files only before merging (or delegate that to a git-hook): this way you may have a chance to integrate correct style step-by-step. If you think this is a good move, let me know and I'll reopen this with only the .clang-format file.

[mvieth]

We have thought about that approach too at some point, but it has some disadvantages: the main purpose of a pull request would become less clear, and the main changes might become sort of hidden within the formatting changes. Reviewing a pull request would become more difficult, and for first-time contributors especially this additional effort could be a burden. In the end, I would prefer to enforce the formatting rules step-by-step in dedicated pull requests. If you like, I can share the script we started to find out which files/file paths are not modified by any open pull request and are thus safe to be formatted.

[fghoussen]

OK, I understand your point of view: that's not easy!... Let's this PR closed

Reviewing a pull request would become more difficult I would prefer to enforce the formatting rules step-by-step in dedicated pull requests.

What about reviewing the PR as always... An when all is ready to merge, asking for a format-only commit? That would make reviews business-as-usual... But rebase of other PR maybe more tricky (only partially depending on latest merge). Anyway, you decide! :)

I can share the script we started to find out which files/file paths are not modified by any open pull request

Not sure to get where you go with this: if only one part of the sources are formatted, you do not solve any of the problems in connection with pending PRs