Wirena / clang-format-configurator-v2

Interactive clang-format configuration tool
https://clang-format-configurator.site/
MIT License
92 stars 3 forks source link

AlignConsecutiveAssignments allows named option groups in clang-format 18 #52

Open detly opened 1 month ago

detly commented 1 month ago

In clang-format 18, AlignConsecutiveAssignments allows both a nested dict or an enumerated type, with values:

Two related issues show with the current site:

  1. These options are not selectable with the UI. But they can be achieved just by setting the individual options that correspond to each group.
  2. Uploading a configuration file containing these grouped settings results in an error:

    Something went wrong:

    Clang-format returned non zero code
    <command-line>:17:7: error: unknown key '9'
       "9": 'v',
          ^~~
    Error parsing -style: Invalid argument

Clicking on "Config File" again shows that AlignConsecutiveAssignments has been changed into:

AlignConsecutiveAssignments:
  "0": C
  "1": o
  "2": n
  "3": s
  "4": e
  "5": c
  "6": u
  "7": t
  "8": i
  "9": v
  "10": e
  Enabled: false
  AcrossEmptyLines: false
  AcrossComments: false
  AlignCompound: false
  AlignFunctionPointers: false
  PadOperators: true
detly commented 1 month ago

Oh, it's the same for AlignConsecutiveDeclarations and AlignConsecutiveMacros.

Wirena commented 1 month ago

@detly thanks for your bug report! Yeah, legacy values for all AlignConsecutive* options are not supported by this configurator, it's mentioned in the readme file.

I've made this popup error message to notify users about this limitation, but it looks like this message does not appear when BasedOnStyle option is set, I can confirm that. Could you please provide original config file used in your example, just in case?

image

I'll fix the error message not appearing. As for support for these legacy values, I doubt I can add them to the main UI page, there's too much shitcode and architectural design flaws. Finding which combination of nested values corresponds to legacy value and offering user to convert it in the Config File UI page looks more realistic solution to me

detly commented 1 month ago

Yeah, legacy values for all AlignConsecutive* options are not supported by this configurator, it's mentioned in the readme file.

Ah sorry, I thought they were new values, not legacy ones. I might change those in my own project then. You can disregard that part of it then.

However:

but it looks like this message does not appear when BasedOnStyle option is set

Indeed, I have WebKit for that:

BasedOnStyle: WebKit
Language: Cpp

ColumnLimit: 88

SortIncludes: CaseInsensitive

AlignAfterOpenBracket: BlockIndent

BreakBeforeBraces: Custom
BraceWrapping:
    AfterCaseLabel: true
    AfterControlStatement: true
    AfterEnum: true
    AfterFunction: true
    AfterStruct: true
    AfterUnion: true
    BeforeElse: true
    BeforeWhile: true

AlignConsecutiveAssignments: Consecutive
AlignConsecutiveDeclarations: Consecutive
AlignConsecutiveMacros: Consecutive
AlignEscapedNewlines: Left
AllowAllArgumentsOnNextLine: false
AllowAllParametersOfDeclarationOnNextLine: false
AllowShortEnumsOnASingleLine: false
AllowShortFunctionsOnASingleLine: Empty
BinPackArguments: false
BinPackParameters: false
BreakBeforeBinaryOperators: NonAssignment
IndentCaseLabels: true
IndentPPDirectives: BeforeHash
InsertBraces: true
KeepEmptyLinesAtEOF: false
KeepEmptyLinesAtTheStartOfBlocks: false
PointerAlignment: Middle
QualifierAlignment: Left
ReflowComments: true
RemoveSemicolon: true
SpaceAfterCStyleCast: true

PenaltyBreakAssignment: 0
PenaltyReturnTypeOnItsOwnLine: 0
Wirena commented 1 month ago

@detly I've fixed error message not appearing, it would be great if you could double check that it works

I'll add conversion from legacy option later and let you know

detly commented 1 month ago

Can confirm that an error message appears now if I use the legacy values:

Something went wrong:

Legacy values like 'None', 'Consecutive', etc for 'AlignConsecutive*' options are not supported for versions >=15.0
Wirena commented 1 month ago

@detly Thanks for confirmation.

Here's clang-format code that maps legacy values to modern nested options, if you're interested