Skip to content

Fixes formatting of reference and pointer variables - #1930

Merged
kennyweiss merged 4 commits into
developfrom
feature/kweiss/fix-formatting
Aug 27, 2026
Merged

Fixes formatting of reference and pointer variables#1930
kennyweiss merged 4 commits into
developfrom
feature/kweiss/fix-formatting

Conversation

@kennyweiss

Copy link
Copy Markdown
Member

Summary

  • This maintenance PR updates our .clang-format
    • It ensures that we consistently place the * and & for pointers and references next to the type rather than the variable
    • It ensures that line ending are LF (instead of CRLF)
    • It updates the language to C++17
    • It updates the link/commit at the top to clang@19, instead of clang@14
    • It removes obsolete/deprecated attributes from our .clang-format
      • I verified that the associated commit has no formatting changes

Since this merge will affect all outstanding existing branches, it probably makes sense to merge this close to the upcoming August release.

  • I added a huge commit with all of the formatting changes related to the pointer/references/crlf, but will plan to redo that commit before merging.

@kennyweiss kennyweiss added this to the FY26 August release milestone Jul 29, 2026
@kennyweiss kennyweiss self-assigned this Jul 29, 2026
@kennyweiss kennyweiss added the maintenance Issues related to code maintenance label Jul 29, 2026
Comment thread src/.clang-format

@kennyweiss kennyweiss Jul 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file contains the only hand-edited changes.
Many of the changes were related to removing obsolete/deprecated features, and updating to the new ones.
Tested using clang-format --dump-config

The rest of the files in this PR are automatically generated via make style
(Note: Reviewing with "Hide whitespace" reveals that nearly every change in this PR is a whitespace change.)

Comment thread src/.clang-format
Comment on lines +53 to +55
PointerAlignment: Left

ReferenceAlignment: Left

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two control the placement of T* ptr and T& ref

The DerivePointAligment: false a few lines up says not to use the style found in the file.

@kennyweiss
kennyweiss marked this pull request as ready for review July 29, 2026 02:11
@kennyweiss
kennyweiss requested a review from HaluskaR July 29, 2026 02:11
Comment thread src/.clang-format Outdated

@jcs15c jcs15c left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! It is indeed mostly whitespace changes.

@kennyweiss
kennyweiss force-pushed the feature/kweiss/fix-formatting branch from b38d300 to f30ed92 Compare August 26, 2026 22:13
Comment thread src/.clang-format
AfterExternBlock: false
BeforeCatch: true
BeforeElse: true
# BeforeLambdaBody: true # available in clang 11

@kennyweiss kennyweiss Aug 26, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we're past clang@11, do we want to set BraceWrapping/BeforeLambdaBody: true?

If so, we'd check changes like this:

Image

I can see both sides: Lambda curly brace wrapping on a separate line would be more consistent w/ our other choices, but somehow, I've grown used to seeing lambda curly braces at the end of a line.

We can also defer this decision to a follow-up PR if we can't reach a quick consensus.

@kennyweiss
kennyweiss merged commit ff84de0 into develop Aug 27, 2026
24 checks passed
@kennyweiss
kennyweiss deleted the feature/kweiss/fix-formatting branch August 27, 2026 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Issues related to code maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants