chore: Remove trailing commas in braced initializers to help clang-format - #3274
chore: Remove trailing commas in braced initializers to help clang-format#3274mirelle7 wants to merge 1 commit into
Conversation
PR Summary by QodoRemove trailing commas from braced initializers for clang-format
AI Description
High-Level Assessment
Files changed (19)
|
Code Review by Qodo
1. Generals builds fail on border color initialization
|
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Source/Common/Bezier/BezierSegment.cpp | Removes trailing commas from local Coord3D initializers without changing their values. |
| Core/Tools/Launcher/BFISH.cpp | Removes trailing commas from nested Blowfish S-box rows while preserving every constant and array boundary. |
| Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp | Cleans up the Surfaces initializer while retaining the valid outer trailing comma in TheShellHookNames. |
| GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp | Mirrors the safe initializer cleanup and outer-array comma handling in the Zero Hour variant. |
| Generals/Code/Tools/WorldBuilder/src/TeamGeneric.cpp | Removes inner row-ending commas without changing control ID pairs or the terminating sentinel. |
| GeneralsMD/Code/Tools/WorldBuilder/src/TeamGeneric.cpp | Applies the equivalent non-behavioral initializer cleanup to the Zero Hour WorldBuilder table. |
Reviews (5): Last reviewed commit: "chore: Remove trailing commas that break..." | Re-trigger Greptile
|
Code review by qodo was updated up to the latest commit f502b38 |
|
If the second commit is necessary to avoid the same issue, doesn't that mean there are many more instances of this issue? I'd expect all cases that match these queries to be the same: |
|
,\s*//.\n\s} not needed. added ,\s*\n\s*} |
|
What is the modern C++ convention on using trailing comma's on the last line? C#'s Stylecop enforced it (SA1413 - Use trailing comma in multi-line initializers). Python, Javascript and Typescript have similar rules that can be enabled. There are pro- and cons for using trailing comma's, but removing shouldn't be based on formatting only |
69e3fca to
f6dc518
Compare
Scoped to comment-free array/struct literals (BorderColors, TeamGeneric, BezierSegment, GameMemoryInitPools, BFISH, Properties, Scripts) where clang-format explodes each element onto its own line without this. Enums are left alone; EnumTrailingComma: Remove in .clang-format handles those automatically instead of needing manual edits.
f6dc518 to
55c7d6b
Compare
|
I'm adding looking into "EnumTrailingComma" in the clang format PR. |
| 0x83260376U,0x6295CFA9U,0x11C81968U,0x4E734A41U,0xB3472DCAU,0x7B14A94AU,0x1B510052U,0x9A532915U, | ||
| 0xD60F573FU,0xBC9BC6E4U,0x2B60A476U,0x81E67400U,0x08BA6FB5U,0x571BE91FU,0xF296EC6BU,0x2A0DD915U, | ||
| 0xB6636521U,0xE7B9F9B6U,0xFF34052EU,0xC5855664U,0x53B02D5DU,0xA99F8FA1U,0x08BA4799U,0x6E85076AU, | ||
| 0xB6636521U,0xE7B9F9B6U,0xFF34052EU,0xC5855664U,0x53B02D5DU,0xA99F8FA1U,0x08BA4799U,0x6E85076AU |
Looks like its still in the proposal stages for c++ std But google and LLVM coding standards allow it |
|
I think the main reason why trailing comma exists is for unrolling type of macros (a macro that unrolls a list). It cannot omit the last trailing comma, because the macro is not that clever. |
In preparation for the clang-format PR. This removes all trailing commas inside braced initializers to avoid unwanted multi-line formatting.