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

Define coding style and a clang-format config #386

Open
rsmmr opened this issue May 29, 2019 · 4 comments

Comments

Projects
None yet
2 participants
@rsmmr
Copy link
Member

commented May 29, 2019

We should develop a summary of the C++ side coding style along with a corresponding clang-format config (In fact, that config can be the style documentation for things can it can express).

Note that clang-format doesn't support (at least) two formatting bits yet that the Zeek code uses:

  • Inserting a space after a logical not (!). This has actually just been added to clang-format and will be part of version 9 (option SpaceAfterLogicalNot, see commit)

  • Inserting spaces around conditions in if etc. There's a patch that adds a corresponding option coming with the install-clang script. install-clang script incorporates that patch into a mostly statically linked version of clang-format; which we could make available as a binary ("mostly" because it keeps linking against some shared low-level system libraries)

@timwoj

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

If you're building Clang via homebrew, there's a way to insert extra patches into existing recipes: https://www.ralfebert.de/snippets/brew-apply-patch-to-package-formula/. That might be better than building a static version of Clang to pass around, since most (all?) of us work on macOS.

As for the actual formatting, did you have a config already created when you were working on the patch?

@rsmmr

This comment has been minimized.

Copy link
Member Author

commented May 31, 2019

@timwoj

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2019

At a first glance, clang-format also doesn't support Whitesmith. There's a very old patch open against LLVM for it, but it hasn't been looked at since 2016. See https://reviews.llvm.org/D6833.

@rsmmr

This comment has been minimized.

Copy link
Member Author

commented Jun 3, 2019

Ah, I missed that. For some reason I thought clang-format would support Whitesmith. Sounds like another patch will be needed; maybe that old one will work, or at least can be a starting point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.