git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 0/3] clang-format: fix rules to make the CI job cleaner

From
Karthik Nayak <karthik.188@gmail.com>
Date
Oct 12, 2024, 01:49 UTC
Message-ID
<cover.1728697428.git.karthik.188@gmail.com>
In-Reply-To
<CAOLa=ZRvFBhageS65uE5enzLBz7H_CAvvnEcPsi_QAi0exRx2w@mail.gmail.com>

The clang-format CI job is currently cluttered due to too many errors being reported. See some of the examples here:

* https://gitlab.com/gitlab-org/git/-/jobs/7854601948
* https://gitlab.com/gitlab-org/git/-/jobs/7843131109
So modify the clang-format with the following changes:
1. Modify the penalties for linebreaks to be more considerate towards
readability. The commit goes into detail explaining how/why.  
2. Don't align expressions after linebreaks to ensure that we instead rely on
'ContinuationIndentWidth'. This fix is rather small and ensures that instead of
trying to align wrapped expressions, we follow the indentation width.
3. Align the macro definitions. This is something we follow to keep the macros
readable. 

I will still keep monitoring the jobs from time to time to ensure we can fine tune more as needed, if someone see's something odd, do keep me in the loop.

Thanks
Changes over the previous version:
1. I figured that we can keep the column limit to the previous 80 while still
having some breathing room by tweaking the penalties associated with line breaks.
Also CC'in Johannes here, since this builds on top of his changes.
Karthik Nayak (3):
  clang-format: re-adjust line break penalties
  clang-format: align consecutive macro definitions
  clang-format: don't align expressions after linebreaks
 .clang-format | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)
Range-diff against v2:
1:  e22ffbe0f6 ! 1:  74bbd2f9db clang-format: change column limit to 96 characters
    @@ Metadata
     Author: Karthik Nayak <karthik.188@gmail.com>
     
      ## Commit message ##
    -    clang-format: change column limit to 96 characters
    +    clang-format: re-adjust line break penalties
     
    -    The current value for the column limit is set to 80. While this is as
    -    expected, we often prefer readability over this strict limit. This means
    -    it is common to find code which extends over 80 characters. So let's
    -    change the column limit to be 96 instead. This provides some slack so we
    -    can ensure readability takes preference over the 80 character hard
    -    limit.
    +    In 42efde4c29 (clang-format: adjust line break penalties, 2017-09-29) we
    +    adjusted the line break penalties to really fine tune what we care about
    +    while doing line breaks. Modify some of those to be more inline with
    +    what we care about in the Git project now.
    +
    +    We need to understand that the values set to penalties in
    +    '.clang-format' are relative to each other and do not hold any absolute
    +    value. The penalty arguments take an 'Unsigned' value, so we have some
    +    liberty over the values we can set.
    +
    +    First, in that commit, we decided, that under no circumstances do we
    +    want to exceed 80 characters. This seems a bit too strict. We do
    +    overshoot this limit from time to time to prioritize readability. So
    +    let's reduce the value for 'PenaltyExcessCharacter' to 10. This means we
    +    that we add a penalty of 10 for each character that exceeds the column
    +    limit. By itself this is enough to restrict to column limit. Tuning
    +    other penalties in relation to this is what is important.
    +
    +    The penalty `PenaltyBreakAssignment` talks about the penalty for
    +    breaking an assignment operator on to the next line. In our project, we
    +    are okay with this, so giving a value of 5, which is below the value for
    +    'PenaltyExcessCharacter' ensures that in the end, even 1 character over
    +    the column limit is not worth keeping an assignment on the same line.
    +
    +    Similarly set the penalty for breaking before the first call parameter
    +    'PenaltyBreakBeforeFirstCallParameter' and the penalty for breaking
    +    comments 'PenaltyBreakComment' and the penalty for breaking string
    +    literals 'PenaltyBreakString' also to 5.
    +
    +    Finally, we really care about not breaking the return type into its own
    +    line and we really care about not breaking before an open parenthesis.
    +    This avoids weird formatting like:
    +
    +       static const struct strbuf *
    +              a_really_really_large_function_name(struct strbuf resolved,
    +              const char *path, int flags)
    +
    +    or
    +
    +       static const struct strbuf *a_really_really_large_function_name(
    +                struct strbuf resolved, const char *path, int flags)
    +
    +    to instead have something more readable like:
    +
    +       static const struct strbuf *a_really_really_large_function_name(struct strbuf resolved,
    +              const char *path, int flags)
    +
    +    This is done by bumping the values of 'PenaltyReturnTypeOnItsOwnLine'
    +    and 'PenaltyBreakOpenParenthesis' to 300. This is so that we can allow a
    +    few characters above the 80 column limit to make code more readable.
     
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## .clang-format ##
    -@@ .clang-format: UseTab: Always
    - TabWidth: 8
    - IndentWidth: 8
    - ContinuationIndentWidth: 8
    --ColumnLimit: 80
    -+
    -+# While we recommend keeping column limit to 80, we want to also provide
    -+# some slack to maintain readability.
    -+ColumnLimit: 96
    +@@ .clang-format: KeepEmptyLinesAtTheStartOfBlocks: false
    + 
    + # Penalties
    + # This decides what order things should be done if a line is too long
    +-PenaltyBreakAssignment: 10
    +-PenaltyBreakBeforeFirstCallParameter: 30
    +-PenaltyBreakComment: 10
    ++PenaltyBreakAssignment: 5
    ++PenaltyBreakBeforeFirstCallParameter: 5
    ++PenaltyBreakComment: 5
    + PenaltyBreakFirstLessLess: 0
    +-PenaltyBreakString: 10
    +-PenaltyExcessCharacter: 100
    +-PenaltyReturnTypeOnItsOwnLine: 60
    ++PenaltyBreakOpenParenthesis: 300
    ++PenaltyBreakString: 5
    ++PenaltyExcessCharacter: 10
    ++PenaltyReturnTypeOnItsOwnLine: 300
      
    - # C Language specifics
    - Language: Cpp
    + # Don't sort #include's
    + SortIncludes: false
3:  6ebcd2690e = 2:  1586d53769 clang-format: align consecutive macro definitions
2:  b55d5d2c14 ! 3:  36a53299c1 clang-format: don't align expressions after linebreaks
    @@ Commit message
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## .clang-format ##
    -@@ .clang-format: AlignConsecutiveDeclarations: false
    +@@ .clang-format: AlignConsecutiveMacros: true
      #   int cccccccc;
      AlignEscapedNewlines: Left
      
-- 
2.47.0
Previous: Toon ClaesNext: Karthik Nayak
Message 9 of 34 in “clang-format: fix rules to make the CI job cleaner”
  1. 0/3 clang-format: fix rules to make the CI job cleanerKarthik Nayak, Oct 9, 2024
  2. 1/3 clang-format: don't enforce the column limitKarthik Nayak, Oct 9, 2024
  3. Justin ToblerOct 9, 2024
  4. Junio C HamanoOct 9, 2024
  5. karthik nayakOct 10, 2024
  6. 2/3 clang-format: don't align expressions after linebreaksKarthik Nayak, Oct 9, 2024
  7. 3/3 clang-format: align consecutive macro definitionsKarthik Nayak, Oct 9, 2024
  8. Toon ClaesOct 10, 2024
  9. 0/3 clang-format: fix rules to make the CI job cleanerKarthik Nayak, Oct 12, 2024
  10. 1/3 clang-format: re-adjust line break penaltiesKarthik Nayak, Oct 12, 2024
  11. Toon ClaesOct 14, 2024
  12. Taylor BlauOct 14, 2024
  13. karthik nayakOct 15, 2024
  14. karthik nayakOct 15, 2024
  15. Kyle LippincottOct 14, 2024
  16. karthik nayakOct 15, 2024
  17. Kyle LippincottOct 16, 2024
  18. karthik nayakOct 16, 2024
  19. 2/3 clang-format: align consecutive macro definitionsKarthik Nayak, Oct 12, 2024
  20. Kyle LippincottOct 14, 2024
  21. karthik nayakOct 15, 2024
  22. 3/3 clang-format: don't align expressions after linebreaksKarthik Nayak, Oct 12, 2024
  23. Kyle LippincottOct 14, 2024
  24. karthik nayakOct 15, 2024
  25. 0/2 Subject: clang-format: fix rules to make the CI job cleanerKarthik Nayak, Oct 18, 2024
  26. 1/2 clang-format: re-adjust line break penaltiesKarthik Nayak, Oct 18, 2024
  27. Justin ToblerOct 25, 2024
  28. karthik nayakOct 25, 2024
  29. 2/2 clang-format: align consecutive macro definitionsKarthik Nayak, Oct 18, 2024
  30. Taylor BlauOct 18, 2024
  31. karthik nayakOct 20, 2024
  32. Taylor BlauOct 21, 2024
  33. karthik nayakOct 22, 2024
  34. Taylor BlauOct 22, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.