glsmac icon indicating copy to clipboard operation
glsmac copied to clipboard

Style specified in .clang-format differs from style of existing code in project

Open ridleyjack opened this issue 1 year ago • 2 comments

Environment:

  • Windows 10 Home
  • Visual Studio Community 2022
  • clang-format version 19.1.1

Issue: Running clang-format on my machine changes the style of existing code. Options not specified in the project's .clang-format default to the LLVM style preset. The LLVM preset options differ from what appears to be the style used in the project in some cases.

Steps to Reproduce:

  1. Use Visual Studio IDE on Windows w/ formatting configured to be done via clang-format.
  2. Open src/engine/Engine.cpp (or most other large files)
  3. Format Document (ctrl-K, ctrl-D) by default
  4. Observe imports are now sorted and some lines are removed.

Examples:

  1. Comment Alignment: Image Can be resolved by adding AlignTrailingComments: Leave

  2. Sorting includes and namespace comment: Image #include <string> unrelated (added so it would compile) Can be resolved by adding: FixNamespaceComments: false SortIncludes: false

  3. Removing empty line: Image Can be resolved by adding: EmptyLineAfterAccessModifier: Leave

These are not all of the differences and finding the "correct" settings for some of them is a lot less fun.

Detailed Explanation: The project's .clang-format file contains the line BasedOnStyle: LLVM. As a result default values from LLVM will be used for any options not set in the .clang-format file.

The default values for the LLVM preset can be obtained by running the following command: clang-format -style=llvm -dump-config > .clang-format

For example my version of clang-format shows the option and default value SortIncludes: CaseSensitive on line 215 of the .clang-format file obtained above.

The project's .clang-format file does not specify a 'SortIncludes" option so it defaults to the CaseSensitive option set by LLVM. This is why adding SortIncludes: false resolves the difference in Example 2.

It's unclear why the formatting style in the project's existing code does not match the specified style. Most of it appears to be written in CLion. CLion might be specifying rules in the command line when it calls clang-format or applying its own formatting after running clang-format.

Resolving This may not even be an issue, it depends on how much consistency is desired for code style across the project.

I can try to modify the .clang-format file so it's more restrictive and specifies something that's the same as or very similar to what currently exists in the project. Wouldn't be able to modify Intellij-GLSMAC.xml myself though.

ridleyjack avatar Mar 11 '25 04:03 ridleyjack

Modified issue for clarification. Would also be nice to know if anyone is able to or not able to reproduce the issue on their IDE of choice (or through command line if that's how you run clang-format). Thx.

ridleyjack avatar Mar 12 '25 22:03 ridleyjack

I also noticed that when working on other computers. I don't know a good way to fix, this behavior is IDE-dependent. For intellij-based IDEs (i.e. Clion that I'm using) it may be more accurate to import tools/code_style/Intellij-GLSMAC.xml, it has something that .clang-format doesn't, but that's not cross-platform.

I don't want to spend time on this currently, however if you manage to fix .clang-format and make PR that would be welcome (in this case also delete tools/code_style/Intellij-GLSMAC.xml).

Also maybe you could think about universal way to keep enforce style of .gls.js scripts. I don't know if .clang-format can be used for them too. Some challenge is that .gls.js is not real JavaScript and it has some new operators such as :~ or :+ that JS formatter won't understand.

afwbkbc avatar Apr 26 '25 10:04 afwbkbc

Closed for now following the discussion on Discord.

The current focus appears to be on getting scripting ready for modders, and my suggestions would shift attention away from that. I'll close this PR and the related issue for now, and if it becomes relevant later—or if I have more time—I may revisit it. Others are, of course, welcome to pick it up if they feel it's appropriate.

ridleyjack avatar Jun 26 '25 05:06 ridleyjack