Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format codes with clang-format tool? #550

Closed
dota17 opened this issue Mar 9, 2020 · 3 comments
Closed

Format codes with clang-format tool? #550

dota17 opened this issue Mar 9, 2020 · 3 comments

Comments

@dota17
Copy link
Member

dota17 commented Mar 9, 2020

hi, hawicz @hawicz

The code formof json-c is a little messy, like json_tokener.c. Spaces and tabs are used mixedly and the code style is not unified. Maybe code formatting is a good choice. I submitted the .clang-format( clang-format configuration file ) and a formatted demo in https://github.com/dota17/json-c/pull/58/files.

If you are insterested in it. I am willing to submit the .clang-format file. And we could also add some other optimization options or formats you want. The other options are introduced in clang.llvm.org.

We can format the code by clang-format -i xxxx.c -style=file. And if a contributor want to add some codes, he can format the modified file by using .clang-format simply. Even, we can added formatting-code script in CI as entrance guard to force contributor formatting the codes, like jsoncpp.

@hawicz
Copy link
Member

hawicz commented Mar 10, 2020

Several things:

  • The churn this will generate sucks, but consistency would be nice.
    ** Perhaps it makes sense to rewrite all the commits in a parallel history tree with consistent formatting applied retroactively, so line-by-line attestation of changes shows something reasonably accurate? hmm...
  • I prefer tabs instead of spaces, at least for indentation.
    ** Probably the UseTab: ForIndentation option, though I'm not quite clear as to what it'll do for "continuations"
    ** If it uses tabs to the start of the continued line, and then spaces from there, that'd be great.
  • Braces should be on the following lines of function and struct definitions (I like being able to visually match up the starting and ending braces)
    ** Braces within a function, such as after if/while/switch/etc... should go on the next line too, but I can go either way with that.
    ** Probably BreakBeforeBraces: Allman
  • "char foo", not "char foo". PointerAlignment: Right (i.e. the LLVM format default)
  • Maximum line lengths are just suggestions, and long lines that end up split by an automatic formatted should be manually adjusted if it's more readable with different splitting.
    ** Let's use ColumnLimit: 100 here
  • AlignEscapedNewlines: Left
  • AllowShortCaseLabelsOnASingleLine : true
  • IndentWidth: 4 (does it even matter UseTab?)
  • ReflowComments: false
  • TabWidth: 4 (does it even matter with UseTab?)

@dota17
Copy link
Member Author

dota17 commented Mar 10, 2020

This change is really not comfortable indeed. Many files will be formatted. But the advantage is that the code will be cleaner and developers can read/update code easily. At the same time, contributors could optimize the code in a uniform format. I will add the optimization options according to your requirements.

@hawicz
Copy link
Member

hawicz commented Mar 11, 2020

Never mind about rewriting all commit history. I just tried doing that, and it ends up being more of a mess that it's worth. git blame has a -w option anyway, which will mostly ignore formatting changes.

@dota17 dota17 closed this as completed Apr 2, 2020
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

No branches or pull requests

2 participants