search

norlab-ulaval / libpointmatcher

An Iterative Closest Point (ICP) library for 2D and 3D mapping in Robotics
BSD 3-Clause "New" or "Revised" License
1.61k stars 544 forks source link

Add clang format #277

Open PhiBabin opened 6 years ago

PhiBabin commented 6 years ago

It would be easier to submit pull request, if the indentation/format rules were enforced by an automatic tools like clang-format.

pomerlef commented 6 years ago

Sounds like a good idea.

simonpierredeschenes commented 5 years ago

I don't know if this is what we want. When we will add the formater, the first commit will contain a huge amount of lines changed, which will ruin git blame in almost all files. This must be taken into account before adding it.

pomerlef commented 5 years ago

Alright, should we move that feature in v2.0.0 then?

simonpierredeschenes commented 5 years ago

I think it would more reasonable, since history is less important between major releases.

stephanemagnenat commented 5 years ago

A note about that. We had a similar discussion in aseba. Beside breaking the history, automatic formatting also breaks some special formatting done on purpose by the programmer (for example to define a matrix or highlight the specifics of an equation). For new code, a // clang-format off/on comment allows to disable automatic formatting, and thus it is acceptable. But for old code, I do not see the benefit. My advise is to apply it to new and changed code only.

pomerlef commented 5 years ago

Specifications:

stephanemagnenat commented 5 years ago

This file might be a useful starting point for the formatting rules, as both aseba and libpointmatcher kind of follow "my" style.

PhiBabin commented 5 years ago

I try 2 weeks ago to create a clang-format which cover the libpointmatcher style. It seems libpointmatcher follows aseba style. We could add a git commit hook to the project, which run clang format on the modified files when creating a commit. I suggest to add this feature after #282 is merged, since boost::list_ofare not correctly handle by clang-format.

YoshuaNava commented 4 years ago

Hi all,

Would it be possible to re-consider integrating a formatter? Both to old and new code. It would simplify debugging, reviewing pull requests, and contributing (if you have a fork), without spending too much time on style discussions and fixing syntax.

pomerlef commented 4 years ago

I fine with the idea. Could you prepare a PR?

YoshuaNava commented 4 years ago

Yes. I second @PhiBabin proposal to use clang format, which is easy to run from CLI, with modern IDEs (for example, VSCode), is also used by other ASL projects, and gives quite a bit of flexibility.

I would divide the contribution in two PRs