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.

Sign up for GitHub

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

Jump to bottom

Create a clang-format config that describes PCL Style Guide #2106

Closed
taketwo opened this issue Nov 30, 2017 · 5 comments
Closed

Create a clang-format config that describes PCL Style Guide #2106

taketwo opened this issue Nov 30, 2017 · 5 comments
Labels
good first issue Skills/areas of expertise needed to tackle the issue help wanted

Comments

@taketwo
Copy link
Member

taketwo commented Nov 30, 2017

Maybe makes sense to use clang-5, it has a number of new settings.

Should we aim already for 5 even though xenial is shipping 3.8?

I don't think it matters what is shipped with a certain distro. This is auxiliary tooling only for devs and they can make this extra step of adding a PPA and installing 5.0 alongside with their standard clang.

Plus, in my dream world we have all the code base reformatted and a special Travis task that runs each pull request through formatter and fails if code does not match, outputting a diff that a contributor can apply to his code without any need to install whatever version of clang we happen to be using on Travis.
@taketwo taketwo mentioned this issue Nov 30, 2017
Merged
@hrnr
Copy link
Contributor

hrnr commented Nov 30, 2017

Also clang-format-diff.py might be interesting for you. I allows to start incrementally using clang-format with new commits. Can be setup as git hook.

This is only useful if you don't have files that use completely different conventions. Then it leads to awful mixing of styles, but even then might be useful for manual use.

@taketwo
Copy link
Member Author

taketwo commented Nov 30, 2017 •
edited
Loading

Thanks for the hint, this looks extremely useful. With this we can get very close to my dream world, and reformatting of all existing code is not needed.

@SergioRAgostinho So what about making a format specification which matches the style guide as close as possible, then replacing the style guide with this spec, and formatting all new code with it on Travis?

@SergioRAgostinho
Copy link
Member

I don't think it matters what is shipped with a certain distro. This is auxiliary tooling only for devs and they can make this extra step of adding a PPA and installing 5.0 alongside with their standard clang.

Ok makes sense. Ultimately we want the configuration file to conform to the style guide and if earlier versions don't have enough features to do it exactly as desired, it's also not useful.

So what about making a format specification which matches the style guide as close as possible, then replacing the style guide.

I would defer the decision till results of the first task are available for comparison. If we change the style spec we would need to deprecate the ones which are already in place.

@jasjuang
Copy link
Contributor

jasjuang commented Dec 1, 2017

This is great!!! Enforcing clang-format will be a huge huge improvement!

@SergioRAgostinho SergioRAgostinho added the good first issue Skills/areas of expertise needed to tackle the issue label Dec 8, 2017
@JamesGiller
Copy link
Contributor

I'm working on it.

JamesGiller added a commit to JamesGiller/pcl that referenced this issue Jul 12, 2018
@JamesGiller
Create a clang-format config for PCL style.
db32ab4 
The clang-format config enforces spacing and line-break guidelines from
the PCL style guide, but not guidelines such as all cap constants or
parenthesised return values etc.

Unfortunately, clang-format has limitations that mean a couple of the
style guidelines cannot be enforced, specifically:
1) Members affected by access qualifiers cannot be indented by one more level
2) Return type of functions will go on own separate line, not even the
   same line as template parameters as in one example from
   section 2.3 of the style guide

resolves PointCloudLibrary#2106
@JamesGiller JamesGiller mentioned this issue Jul 12, 2018
Closed
@kunaltyagi kunaltyagi mentioned this issue Jul 8, 2019
Merged
4 tasks
@taketwo taketwo closed this as completed Jul 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Skills/areas of expertise needed to tackle the issue help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Create a clang-format config for PCL style. JamesGiller/pcl
5 participants
@taketwo @hrnr @JamesGiller @SergioRAgostinho @jasjuang