Skip to content

Conversation

pslldq
Copy link
Contributor

@pslldq pslldq commented Mar 31, 2025

Automatically format the code with clang-format to always have a consistent style.

Options used:

  • Indent width of 4 as of Add an .editorconfig file #1461 (comment)
  • Only move function body to the same line, if it's empty (e.g. void foo() {}, but not void foo() { bar(); })
  • Add braced below functions, but directly after conditional/loop statements (Mozilla style)
  • Compact short case labels on a single line (primarely for case A: return foo;)
  • Also move binary operator to the start of the next line (if it is broken up)
  • Either have all parameters/arguments on the same line or split it up into one line for each
  • Alignment stuff...

@bynect @fwsmit Would like to hear your thoughts about this style. Some additional desires or maybe remove some of the options?

Also feel free to point out potential ways, where the formatting really breaks the understanding. If they cannot be formatted in a better way, then they always can be excluded using comments.

TODO myself: Add some github action to check the code matches the formatting.

Sven Püschel added 6 commits March 28, 2025 18:20
Remove trailing whitespaces, except for the generated wayland code.
dunst has a mix of indentation styles. Try to improve it by making it
consistent in files with mixed tab/space indentation styles.

While this also changes the generated manpages slightly, it makes it
more consistent accross the man page and probably won't be noticed by
the reader.
Add an .editorconfig file to automatically configure supported editors.
This file was generated merely based on the existent styles and the
warnings caused while checking the code with the editorconfig-checker
tool.

As the editorconfig checker tool generates false positives when an
indentation is adjusted to match multiline arguments or similar
aesthetic adaptions, the corresponding check was disabled. This will
miss wrong indentation sizes (e.g. 4 instead of 8 spaces).
Format all source files with clang-format using the following comamnd:
clang-format -i **/*.c *.c **/*.h *.h
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 72.67194% with 1347 lines in your changes missing coverage. Please review.

Project coverage is 65.25%. Comparing base (f6ba641) to head (f739cf0).

Files with missing lines Patch % Lines
src/draw.c 45.34% 311 Missing ⚠️
src/notification.c 59.62% 172 Missing ⚠️
src/menu.c 14.28% 132 Missing ⚠️
src/dunst.c 5.75% 131 Missing ⚠️
src/rules.c 53.60% 116 Missing ⚠️
src/option_parser.c 82.47% 68 Missing ⚠️
src/settings.c 49.61% 65 Missing ⚠️
src/icon.c 67.50% 52 Missing ⚠️
test/icon-lookup.c 50.00% 46 Missing ⚠️
src/utils.c 82.00% 43 Missing ⚠️
... and 12 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1464      +/-   ##
==========================================
- Coverage   65.47%   65.25%   -0.22%     
==========================================
  Files          50       50              
  Lines        8923     9038     +115     
  Branches     1044     1048       +4     
==========================================
+ Hits         5842     5898      +56     
- Misses       3081     3140      +59     
Flag Coverage Δ
unittests 65.25% <72.67%> (-0.22%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bynect
Copy link
Member

bynect commented Mar 31, 2025

Is this alongside the editorconfig pr or supersedes it?

The config seems sensible but my problem is that such a big commit would dirty the git history

@pslldq
Copy link
Contributor Author

pslldq commented Mar 31, 2025

Is this alongside the editorconfig pr or supersedes it?

no, this is alongside the editorconfig PR. The PR is just included to see, if the formatting trips up something in the editorconfig checker.

The main purpose of the editorconfig is to avoid having to manually adjust the editor each time (use tabs/spaces, indention width, ...). It's like vim modelines, but supported by much more editors by default. Checking the editorconfig is just to prevent some basic stuff.

clang-format on the other hand precisely defines how C/C++ files should be formatted and therefore removes manual formatting and potential discussion entirely.

The config seems sensible but my problem is that such a big commit would dirty the git history

You can partially work around with a .git-blame-ignore-revs file. Or the formatting could only be applied to files changed in future git commits (with git clang-format). Latter is nice, but will hurt for the 8->4 space indentation, as only newly touched code is formatted with 4 spaces.

@pslldq
Copy link
Contributor Author

pslldq commented Apr 14, 2025

General thoughts how to proceed? As described above, there are multiple ways to handle the potential reformatting. And the still missing Github CI Action in this PR would depend on this choice.

I also wouldn't mind if this PR is closed, but in this case I want to avoid the effort of implementing these CI Actions.

@bynect
Copy link
Member

bynect commented Apr 14, 2025

For now let's use the editorconfig. I am not opposed to using autoformat but I have some things to still think through.

Also since we are going to reformat everything we may decide to use a slightly different style.

@pslldq pslldq closed this Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants