Skip to content

Refine .clang-format#1113

Merged
wolfpld merged 1 commit into
wolfpld:masterfrom
siliceum:refine-clang-format
Jul 30, 2025
Merged

Refine .clang-format#1113
wolfpld merged 1 commit into
wolfpld:masterfrom
siliceum:refine-clang-format

Conversation

@Lectem
Copy link
Copy Markdown
Collaborator

@Lectem Lectem commented Jul 27, 2025

I tried to refine a bit the .clang-format file that was introduced in #835 .

There were and still are a few issues:

Known caveats:
- could not manage to keep small enum on a single line

This is still broken, the tool has bugs

- you put 2 spaces in ifdefs (instead of the 4 elsewhere), haven't managed to replicate it

Fixed

- it's not possible to remove the spaces around binary operators 

Still not possible.


I also fixed a few other things such as lambdas, spaces with templates, ...


I uploaded a branch with the files fully formatted with this new file here: 6ef5558

Things that I noticed and still aren't really good:

  • short enum are still not working (even if not commented in config)
  • Spaces around operators (and assignment ones in for loops)
  • Arrays do not break before braces
  • Spaces in parens where we do not really expect them (function signatures, operator priority disambiguation, ...)
  • Preprocessor indentation may look weird (though correct) when whole files are hidden behind a big #if
    • 6ef5558#diff-1405072143d8fb79c9a359bd0374a286b3c7a7660f493348da6c60acbd616065L25
    • This can in some cases be limited a bit for example by removing unneeded #if 6ef5558#diff-0050b814bc7135be0784c57ca198df3787ff137d2bbcdc929048032fdd426464L1
    • This may not be desirable, especially in the public headers
      # define TracyGpuContext \
      tracy::GetGpuCtx().ptr = (tracy::GpuCtx*)tracy::tracy_malloc( sizeof( tracy::GpuCtx ) ); \
      new( tracy::GetGpuCtx().ptr ) tracy::GpuCtx;
      # define TracyGpuContextName( name, size ) tracy::GetGpuCtx().ptr->Name( name, size );
      # if defined TRACY_HAS_CALLSTACK && defined TRACY_CALLSTACK
      # define TracyGpuNamedZone( varname, name, active ) \
      static constexpr tracy::SourceLocationData TracyConcat( __tracy_gpu_source_location, TracyLine ){ name, TracyFunction, TracyFile, (uint32_t)TracyLine, 0 }; \
      tracy::GpuCtxScope varname( &TracyConcat( __tracy_gpu_source_location, TracyLine ), TRACY_CALLSTACK, active );
      # define TracyGpuNamedZoneC( varname, name, color, active ) \
      static constexpr tracy::SourceLocationData TracyConcat( __tracy_gpu_source_location, TracyLine ){ name, TracyFunction, TracyFile, (uint32_t)TracyLine, color }; \
      tracy::GpuCtxScope varname( &TracyConcat( __tracy_gpu_source_location, TracyLine ), TRACY_CALLSTACK, active );
      # define TracyGpuZone( name ) TracyGpuNamedZoneS( ___tracy_gpu_zone, name, TRACY_CALLSTACK, true )
      # define TracyGpuZoneC( name, color ) TracyGpuNamedZoneCS( ___tracy_gpu_zone, name, color, TRACY_CALLSTACK, true )
      # define TracyGpuZoneTransient( varname, name, active ) tracy::GpuCtxScope varname( TracyLine, TracyFile, strlen( TracyFile ), TracyFunction, strlen( TracyFunction ), name, strlen( name ), TRACY_CALLSTACK, active );
      # else
      # define TracyGpuNamedZone( varname, name, active ) \
      static constexpr tracy::SourceLocationData TracyConcat( __tracy_gpu_source_location, TracyLine ){ name, TracyFunction, TracyFile, (uint32_t)TracyLine, 0 }; \
      tracy::GpuCtxScope varname( &TracyConcat( __tracy_gpu_source_location, TracyLine ), active );
      # define TracyGpuNamedZoneC( varname, name, color, active ) \
      static constexpr tracy::SourceLocationData TracyConcat( __tracy_gpu_source_location, TracyLine ){ name, TracyFunction, TracyFile, (uint32_t)TracyLine, color }; \
      tracy::GpuCtxScope varname( &TracyConcat( __tracy_gpu_source_location, TracyLine ), active );
      # define TracyGpuZone( name ) TracyGpuNamedZone( ___tracy_gpu_zone, name, true )
      # define TracyGpuZoneC( name, color ) TracyGpuNamedZoneC( ___tracy_gpu_zone, name, color, true )
      # define TracyGpuZoneTransient( varname, name, active ) tracy::GpuCtxScope varname( TracyLine, TracyFile, strlen( TracyFile ), TracyFunction, strlen( TracyFunction ), name, strlen( name ), active );
      # endif
      # define TracyGpuCollect tracy::GetGpuCtx().ptr->Collect();
  • Reorders includes, which may be problematic

Overall, while the output diff is big, there seem to be a lot of valid formatting fixes.
Would it be possible to consider applying (progressively) clang-format on whole files from the profiler and public/common folders?
Note that it is possible to disable formatting locally using // clang-format off/// clang-format on

The reason I ask this is that (imho), even though the formatting doesn't always look great, it would be very helpful to be able to format the whole document as some editors do not support partial formatting (as mentioned in #958). Worse VS20XX does pick and apply .clang-format by default on the whole file. (It can be disabled but then you lose the ability to format using the clang-format).

This would make the onboarding easier (and perhaps a bit less formatting errors in PRs, of which I'm very much guilty, sorry.)

Thoughts ? (We can shift this discussion to an issue if you prefer, as it is a bit orthogonal to the changes applied to the config file here)

@wolfpld
Copy link
Copy Markdown
Owner

wolfpld commented Jul 28, 2025

Things that I noticed and still aren't really good:

The GitHub UI is unusable here. All I can get from these links is some random location in the diff that is jumping multiple times into other random locations. I can't see what you are referencing.

Would it be possible to consider applying (progressively) clang-format on whole files from the profiler and public/common folders?

No. It wants to touch too much in places where I have specifically chosen to go with the styling choice I went.

Note that it is possible to disable formatting locally using // clang-format off/// clang-format on

I don't want tool-specific markup in the code, especially markup that is only required because the tool is bad at what it does.

The reason I ask this is that (imho), even though the formatting doesn't always look great, it would be very helpful to be able to format the whole document as some editors do not support partial formatting (as mentioned in #958).

The only reason I have considered including .clang-format in the repository was specifically because the formatting could be applied only to selected code fragments.

Worse VS20XX does pick and apply .clang-format by default on the whole file. (It can be disabled but then you lose the ability to format using the clang-format).

I don't think adapting yourself to the shape of a bad tool is a good choice, ever.

@Lectem
Copy link
Copy Markdown
Collaborator Author

Lectem commented Jul 30, 2025

Thanks for the answer! (I think this will serve as reference for anybody thinking of this in the future)

As for the changes in this PR, I also pushed a commit that shows the differences between the old .clang-format and the one in this PR here: siliceum@facd86b.
Overall it seems close to the source code we currently have.

@wolfpld wolfpld merged commit 0a9f3ac into wolfpld:master Jul 30, 2025
6 checks passed
@Lectem Lectem deleted the refine-clang-format branch July 31, 2025 07:45
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.

2 participants