quickjs-ng / quickjs

QuickJS, the Next Generation: a mighty JavaScript engine
https://quickjs-ng.github.io/quickjs/
MIT License
1.15k stars 102 forks source link

Add clang-format / tidy? #96

Open saghul opened 1 year ago

saghul commented 1 year ago

Shall we create a clang-format config that roughly matches the current code, and enforce it with a linting CI?

bnoordhuis commented 1 year ago

Yes, good idea.

bnoordhuis commented 1 year ago

I cobbled together a .clang-format that tries to hew close to the existing style but, even disregarding whitespace changes, it results in massive churn:

$ clang-format -i quickjs*.[ch] lib*.[ch] # clang-format 15
$ git diff --shortstat
 14 files changed, 18373 insertions(+), 18470 deletions(-)
$ git diff --shortstat -w
 13 files changed, 8272 insertions(+), 8369 deletions(-)
```yaml Language: Cpp ColumnLimit: 80 IndentWidth: 4 MaxEmptyLinesToKeep: 1 PointerAlignment: Right AlignAfterOpenBracket: Align AlignTrailingComments: true AlignConsecutiveAssignments: Enabled: true AcrossEmptyLines: true AcrossComments: true AlignCompound: true PadOperators: true AlignConsecutiveBitFields: Enabled: true AcrossEmptyLines: true AcrossComments: true AlignCompound: true PadOperators: true AlignConsecutiveDeclarations: Enabled: true AcrossEmptyLines: true AcrossComments: true AlignCompound: true PadOperators: true AlignConsecutiveMacros: Enabled: true AcrossEmptyLines: true AcrossComments: true AlignCompound: true PadOperators: true AllowShortFunctionsOnASingleLine: false AttributeMacros: - __exception - __js_printf_like - __maybe_unused BinPackArguments: true BinPackParameters: true BreakBeforeBraces: Custom BraceWrapping: AfterCaseLabel: false AfterClass: false AfterControlStatement: Never AfterEnum: false AfterFunction: true AfterNamespace: false AfterObjCDeclaration: false AfterStruct: false AfterUnion: false AfterExternBlock: false BeforeCatch: false BeforeElse: false BeforeLambdaBody: false BeforeWhile: false IndentBraces: false SplitEmptyFunction: true SplitEmptyRecord: true SplitEmptyNamespace: true SpaceBeforeParensOptions: AfterControlStatements: true AfterForeachMacros: true AfterFunctionDefinitionName: true AfterFunctionDeclarationName: true AfterIfMacros: true ```
saghul commented 1 year ago

I'm generally not a fan of such changes but since pretty much have one file maybe it's a good time to do it now that it's early?

bnoordhuis commented 1 year ago

I don't disagree but I suspect the diff can be minimized with some further tweaking, I just ran out of time.

chqrlie commented 9 months ago

Let me investigate if I can tweak a clang-format config file to match our style or find a more appropriate tool. We should have a coding-rules.md file to explain DOs and DON'Ts for patches to maintain consistency, which I consider very important.