<div dir="ltr"><div>Hi Stefanos,</div><div><br></div><div>Thanks for the feedback.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le jeu. 5 déc. 2024 à 10:54, Stefanos Natsis via QGIS-Developer <<a href="mailto:qgis-developer@lists.osgeo.org">qgis-developer@lists.osgeo.org</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Thank you Denis for all this work and the heads up for setting things up!</div><div><br></div><div>I have a few notes/questions about things I've encountered so far:</div><div><br></div><div>1. There are cases where the formatting is not applied. For example in PR <a href="https://github.com/qgis/QGIS/pull/59382" target="_blank">https://github.com/qgis/QGIS/pull/59382</a>, after merging master and pushing, clang-format fails in the CI</div></div></blockquote><div><br></div><div>seems good now? Do you have a pointer to a failing workflow? </div><div>I fixed an invalid regex in prepare_commit 2 days ago, so hopefully everything is ok now.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>2. Is there a way one can manually run pre-commit for all modified files and not just files staged for commit?</div></div></blockquote><div><br></div><div>yes, either globally:</div><div>pre-commit run --all-files</div><div><br></div><div>or with a path</div><div>git ls-files -- src/core/geometry | xargs pre-commit run --files</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>3. clang-format seems to try to squeeze parameters in a single line, even if one has explicitly used new lines for readability.</div><div>For example, it will do this:</div><div><br></div><div>```cpp<br></div><div>-  else if ( isOnMesh && ( mWidgetActionDigitizing->zValueSourceType() == QgsMeshEditDigitizingAction::InMeshUseMeshZWidgetOtherwise ||<br>-                          mWidgetActionDigitizing->zValueSourceType() == QgsMeshEditDigitizingAction::InMeshUseMeshTerrainOtherwise ) )<br>+  else if ( isOnMesh && ( mWidgetActionDigitizing->zValueSourceType() == QgsMeshEditDigitizingAction::InMeshUseMeshZWidgetOtherwise || mWidgetActionDigitizing->zValueSourceType() == QgsMeshEditDigitizingAction::InMeshUseMeshTerrainOtherwise ) )</div><div>```</div><div>This is a big hit on readability when there are  parentheses in the parameters. </div></div></blockquote><div><br></div><div>I agree, I'll try to see if I can improve this.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>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.</div></div></blockquote><div><br></div><div>you must choose with or without apparently in clang-format </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>5. Similar for <span><span>SpacesInSquareBrackets</span></span> and SpacesInContainerLiterals</div></div></blockquote><div><br></div><div>same</div><div><br></div><div><br></div><div>As a side note, there's an online configurator where you can play a bit: <a href="https://zed0.co.uk/clang-format-configurator/">https://zed0.co.uk/clang-format-configurator/</a></div><div>You can load the QGIS config (you need to drop the ObjC part to make it valid).</div><div><br></div><div>Cheers,</div><div>Denis</div><div><br></div></div></div>