Add clang-format checks to CI

[pitbuster]

@TurboGit I know this will be some effort, but having consistent format should ease contributions: I was bitten by this, because the project has some formatting rules, which IDEs or language servers will pick, but are not enforced, so a lot of unwanted changes that people have to revert.

I see two options for landing this:

1. Have the PR open while we (I guess mostly me since I am the one proposing) reformat files bit by bit (one module at a time, for example).
2. Land this with a reduced number of files to check and we incrementally fix the formatting while we add more files to the whitelist.

Also, probably the formatting rules themselves will cause a bit of discussion, but they can be changed as we reach consensus :)

[wpferguson]

I'm not sure this is a good thing. There is already a style guide that's followed.

Is the purpose of this to make it work with IDEs? I don't think most of the developers use one, so changing the code style for IDEs would probably hamper development more than it would help.

What would help development is refactoring some of the HUGE files into smaller chunks.

[fcs-ts]

There is already a style guide that's followed.

No, there is already a format, but it's not being followed, I have had several unwanted formatting changes on previous PRs

so changing the code style for IDEs would probably hamper development more than it would help.

This is not changing anything, just taking the already existing formatting rules and enforcing them on PRs. I am sorry if the description made it sound different.

What would help development is refactoring some of the HUGE files into smaller chunks.

Of course, but automatic formatting enforcement is usually the baseline to make contributions easier :)

[parafin]

I think you’re missing the point. clang format file is not used by most (all) developers, so it’s not being followed or kept up-to-date. It needs to be either removed completely, or updated with current code style. It is not a ground truth to be enforced.

That being said, I’m not sure if current code style is described anywhere, except for @TurboGit comments on PRs. That should be the first step - formulating it somewhere.

[wpferguson]

I’m not sure if current code style is described anywhere

https://github.com/darktable-org/darktable/wiki/Developer's-guide

[fcs-ts]

It is not a ground truth to be enforced.

Yes, I thought the code style would resemble more closely the current clang file. I guess I'll ask some help to @TurboGit to bring the file up to date then :)

[parafin]

I’m not sure if current code style is described anywhere

https://github.com/darktable-org/darktable/wiki/Developer's-guide

That does not describe it fully. For example Pascal asks for function parameters to be one per line.

[pitbuster]

I just realized that I used another account to reply to comments (that is, @fcs-ts is me :sweat_smile:).

I took a look at the .clang-format file and it seems to cover the "function parameters in their own lines rule". I guess there are other rules that have come up in reviews, but I guess we should wait for Pascal to be back to move this forward.

[TurboGit]

@pitbuster : In the style there is at least 3 very important rules to me:

1. Never ever reformat the SQL code as this was a long and boring manual reformatting work. At least in current state most (if not all) SQL statement are readable.

    DT_DEBUG_SQLITE3_PREPARE_V2(dt_database_get(darktable.db),
                                "INSERT INTO main.color_labels (imgid, color)"
                                "  SELECT ?1, color"
                                "  FROM main.color_labels"
                                "  WHERE imgid = ?2",
                                -1, &stmt, NULL);

2. A single parameter per line for a function spec/definition.

void commit_params(struct dt_iop_module_t *self,
                   dt_iop_params_t *p1,
                   dt_dev_pixelpipe_t *pipe,
                   dt_dev_pixelpipe_iop_t *piece);

3. Not more than 90 characters per line

4. Boolean operator should be at start of expression and one per line

  if(!strcasecmp(cc, ".dt")
     || !strcasecmp(cc, ".dttags")
     || !strcasecmp(cc, ".xmp"))

The rest can be discussed on the way. You're free to gather and document the style guide.

This https://github.com/darktable-org/darktable/wiki/Developer's-guide#coding-style has not been touched since long time. Maybe you can take it over and update the style?

[pitbuster]

@TurboGit I updated the section with your feedback (and also fixed the language hints for codeblocks, so we get highlighting in the wiki :D). Next, I'll try to see how far are the current clang-format rules from the ones you stated here :)

[pitbuster]

I tweaked the rules a little bit to be closer to the points mentioned in the discussion. I also formatted src/bauhaus/bauhaus.h as a demo, so we can see if the rules look fine.

[github-actions[bot]]

This pull request has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please verify it has no conflicts with the master branch and rebase if needed. Mention it now if you need help or give permission to other people to finish your work.

[github-actions[bot]]

This pull reguest was closed because it has been inactive for 300 days since being marked as stale.