[QGIS-Developer] pre-commit for code-formatting

[Denis Rouzaud]

Hi Stefanos,

Thanks for the feedback.

Le jeu. 5 déc. 2024 à 10:54, Stefanos Natsis via QGIS-Developer <
qgis-developer at lists.osgeo.org> a écrit :

> Thank you Denis for all this work and the heads up for setting things up!
>
> I have a few notes/questions about things I've encountered so far:
>
> 1. There are cases where the formatting is not applied. For example in PR
> https://github.com/qgis/QGIS/pull/59382, after merging master and
> pushing, clang-format fails in the CI
>

seems good now? Do you have a pointer to a failing workflow?
I fixed an invalid regex in prepare_commit 2 days ago, so hopefully
everything is ok now.

>
> 2. Is there a way one can manually run pre-commit for all modified files
> and not just files staged for commit?
>

yes, either globally:
pre-commit run --all-files

or with a path
git ls-files -- src/core/geometry | xargs pre-commit run --files


>
> 3. clang-format seems to try to squeeze parameters in a single line, even
> if one has explicitly used new lines for readability.
> For example, it will do this:
>
> ```cpp
> -  else if ( isOnMesh && ( mWidgetActionDigitizing->zValueSourceType() ==
> QgsMeshEditDigitizingAction::InMeshUseMeshZWidgetOtherwise ||
> -                          mWidgetActionDigitizing->zValueSourceType() ==
> QgsMeshEditDigitizingAction::InMeshUseMeshTerrainOtherwise ) )
> +  else if ( isOnMesh && ( mWidgetActionDigitizing->zValueSourceType() ==
> QgsMeshEditDigitizingAction::InMeshUseMeshZWidgetOtherwise ||
> mWidgetActionDigitizing->zValueSourceType() ==
> QgsMeshEditDigitizingAction::InMeshUseMeshTerrainOtherwise ) )
> ```
> This is a big hit on readability when there are  parentheses in the
> parameters.
>

I agree, I'll try to see if I can improve this.

>
> 4. It seems like with astyle we had something equivalent to
> SpacesInAngles=Leave, so both static_cast<int> and static_cast< int > was
> valid. Now the spaces are gone and while it's fine for small types, it is
> harder to read for bigger types in longer statements.
>

you must choose with or without apparently in clang-format

>
> 5. Similar for SpacesInSquareBrackets and SpacesInContainerLiterals
>

same


As a side note, there's an online configurator where you can play a bit:
https://zed0.co.uk/clang-format-configurator/
You can load the QGIS config (you need to drop the ObjC part to make it
valid).

Cheers,
Denis
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/qgis-developer/attachments/20241205/4e7a5428/attachment.htm>