Add clang-format file & target

[tmontaigu]

This moves the .clang-format file of the qLASIO plugin to the root of the project to use it.

This also add two custom CMake targets "format" and "check-format" that work on the source files of explicitly listed CMake targets - format will format the files - check-format will check that files are correctly formatted (intended for the CI)

This would be the mechanism used to format and check the formatting, for now it applied only to a few files so that we can see the style and change the .clang-format options until it suits what we want

[bramburn]

I'll review those this coming weekend

On Tue, Jul 23, 2024, 10:50 Daniel Girardeau-Montaut < @.***> wrote:

@.**** commented on this pull request.

In libs/CCAppCommon/include/CCAppCommon.h https://github.com/CloudCompare/CloudCompare/pull/2041#discussion_r1687514622 :

-//# # -//# CLOUDCOMPARE # -//# # -//# This program is free software; you can redistribute it and/or modify # -//# it under the terms of the GNU General Public License as published by # -//# the Free Software Foundation; version 2 or later of the License. # -//# # -//# This program is distributed in the hope that it will be useful, # -//# but WITHOUT ANY WARRANTY; without even the implied warranty of # -//# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # -//# GNU General Public License for more details. # -//# # -//# COPYRIGHT: CloudCompare project # -//# # -//########################################################################## +// ##########################################################################

Any way to fix this shift in the header? I believe @bramburn https://github.com/bramburn had the same issue on his PR (#2026 https://github.com/CloudCompare/CloudCompare/pull/2026). By the way, maybe some things should be shared between the 2?

In libs/CCAppCommon/include/CCAppCommon.h https://github.com/CloudCompare/CloudCompare/pull/2041#discussion_r1687516960 :

-//# # -//# CLOUDCOMPARE # -//# # -//# This program is free software; you can redistribute it and/or modify # -//# it under the terms of the GNU General Public License as published by # -//# the Free Software Foundation; version 2 or later of the License. # -//# # -//# This program is distributed in the hope that it will be useful, # -//# but WITHOUT ANY WARRANTY; without even the implied warranty of # -//# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # -//# GNU General Public License for more details. # -//# # -//# COPYRIGHT: CloudCompare project # -//# # -//########################################################################## +// ##########################################################################

That's the only things that strikes me, and it's not a big deal. It 'makes sense' in a way as it looks like comment lines...

— Reply to this email directly, view it on GitHub https://github.com/CloudCompare/CloudCompare/pull/2041#pullrequestreview-2193112621, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACUTT3NNZFWDZM7B4ZUK5X3ZNX4MTAVCNFSM6AAAAABLJGNXDGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCOJTGEYTENRSGE . You are receiving this because you were mentioned.Message ID: @.***>

[dgirardeau]

Ah, the CI doesn't like it on Windows 😓

[tmontaigu]

It's because I'm listing targets that we want to be formatted (to avoid checking the format of external dependencies we pull via submodules)

And so I can check if a target exists, if it exists then I add its sources to the list of files to format (so that we can handle optional targets like plugins)

The problem is that if the target has optional features (thus optionnal source files) then these files are only added to the list of files to format if the corresponding option is enabled.

And for example on my machine I don't enable the game pad support option for the CCComon lib so running the format target does not format them, but on the CI that feature is enabled and so these files's formatting is checked and is incorrect.

This should only be a problem at the start, so I should just format all cpp/h files on my local clone, push them as part of the commit, then if one for example modifies the gamepad support file, then, it likely enabled the feature to build and test its changes, so the format target also knows about these files

But that still makes me question a bit whether we should use targets to find files to format, or if we should use file(GLOB) (even with GLOB, I think we would need to list which directories contains sources we want to format, to avoid checking format of external dependencies

[tmontaigu]

So this seems to be ok now, a few things to note:

• I think apart from standard plugins every .h/.cpp is now formatted
• Once merged it means we would need to use somewhat the same clang-format version as the one we choose in the CI (I don't know how often it breaks, but for for example v18 and v20 have a few changes
• If there are mid/huge branches that are not yet merged we should probably wait as this PR will likely create conflicts

[dgirardeau]

Yes, I believe there is at least one PR that could be merged quickly (https://github.com/CloudCompare/CloudCompare/pull/2169). I'll try to help fasten the process this week-end.

[dgirardeau]

@tmontaigu I think we are good now, I don't see any other PR that could be merged soon.

[dgirardeau]

Ok, I've tested on my side (with Visual Studio 2017 and a 'clang' plugin, but sadly it doesn't work very well 😅

I'll need to investigate a little bit more. Or maybe simplify the .clang file?

[tmontaigu]

What did you try and what where the problems ?

apparently clang-format is supported by visual studio https://devblogs.microsoft.com/cppblog/clangformat-support-in-visual-studio-2017-15-7-preview-1/

Otherwise there is a cmake target format that formats all files

[dgirardeau]

I simply opened and saved the mainwindow.cpp file and it made a lot of changes 😅

[tmontaigu]

Probably because it did not use clang to format, I'll try to check, but there is probably a setting to tell visual studio to format with clang format

[dgirardeau]

I forgot to tell that the formatting was also very weird. So definitely not working properly with my setup. I'll try to investigate alternatives.

[dgirardeau]

Ok, I've updated clang-format to use a much more recent version. And I'm only using the 'format' project.

I now have a single discrepancy with your PR:

I prefer your formatting though 😅

[tmontaigu]

Normally you should be able to configure Visual Studio to use clang-format and then clang-format should pick-up the .clang_format file so that formatting is applied on save (at least it works on my CLion and neovim)

I think here the formatting looks broken in both cases, I tried to add a ; to close the ccMesh_extendd_call1 and after that the formatting looks correct

[tmontaigu]

I don't know if there is still a plan to go forward with this, but maybe we should first migrate to qt6

[dgirardeau]

I'm still bugged by the small glitches I have on my side... Even though there are not that many.

I don't know how long the migration to Qt 6 will take. Maybe it's safer to move on with this first.