clang-format for src/analysis

[3nids]

Here are the changes brought to allow using clang-format alongside with our sip code:

• includes in TypeHeaderCode is now achieved by using SIP_TYPEHEADER_INCLUDE
• %MethodCode and other directives are now commented (I think I would move them to using a macro SIP_MethodCode when the whole code base has already been moved to clang-format)
• SIP_SKIP should be placed before the definition of a method in the header (otherwise it's moved to next line by clang-format)

[3nids]

This is ready for review and I would like to avoid waiting too long for a merge since rebasing is not funny :)

[github-actions]

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit c6e6e3c)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit c6e6e3c)

[troopa81]

includes in TypeHeaderCode is now achieved by using SIP_TYPEHEADER_INCLUDE

Not the case for all files it seems, see qgsprocessingparameter for instance. Is it intentionnal ?

%MethodCode and other directives are now commented (I think I would move them to using a macro SIP_MethodCode when the whole code base has already been moved to clang-format)

That mean that we would have a macro also for MappedType/ConvertFromTypeCode/End/ConvertToTypeCode see qgsattributes. ?

I'm not sure about commenting them either as it is IMHO a little bit error prone. Comment followed with % would become SIP instruction and other comment would stay a comment.

can we not disable clang-format around given lines like with clang-tidy?

[3nids]

Not the case for all files it seems, see qgsprocessingparameter for instance. Is it intentionnal ?

in this case, the whole TypeHeaderCode is manually generated. This can be switched indeed (I'd prefer leaving this for a follow-up).

That mean that we would have a macro also for MappedType/ConvertFromTypeCode/End/ConvertToTypeCode see qgsattributes. ?

indeed, probably a macro

I'm not sure about commenting them either as it is IMHO a little bit error prone. Comment followed with % would become SIP instruction and other comment would stay a comment.

indeed, that's why I plan to move these as macros. But for the sake of simplicity while we still have astyle running on some part of the code, I would keep the comments for now.

can we not disable clang-format around given lines like with clang-tidy?

no, at least I failed at it. It's not really the formatting of these lines which are problematic but rather what happens after. The following method after a directive are not correctly formatted. In general clang-format seems a bit more picky than astyle and it's not super tolerant with our macros and injected sip code.

[troopa81]

no, at least I failed at it.

This doesn't work ?

[3nids]

No. It works for the line in question, but messes up the following

[troopa81]

I'm not in favor of altering sip code either with comment or macros, it makes the whole thing even less readable IMHO. Most of sip code (all?) should not live in header file, it should be in different file included on the fly by sipify, but that's another story.

I won't block this PR but I'd rather wait for another contributor opinion.

[3nids]

I'm not in favor of altering sip code either with comment or macros, it makes the whole thing even less readable IMHO

We already had to do this for %ConvertToTypeCode (e.g.

QGIS/src/core/qgstemporalcontroller.h

Line 46 in 390ac14

SIP_CONVERT_TO_SUBCLASS_CODE

)
So basically the same, but I get your point of altering the code.

Most of sip code (all?) should not live in header file

I tend to find it useful to read the expected behavior in a single place, but that's debatable.

I won't block this PR but I'd rather wait for another contributor opinion.

Thanks for the feedback, let's wait for more opinions :)

[nyalldawson]

I'm +1 to this -- I think the choices here look like good pragmatic solutions to a problem which does not have an ideal answer.

Regarding:

Most of sip code (all?) should not live in header file, it should be in different file included on the fly by sipify, but that's another story.

I'm personally in favour of this code living in the headers. It makes it really obvious when you're editing c++ classes what extra logic is involved in Python. If we siloed it away in a different place then it's very prone to bit rotting/getting out of sync.