whoenig / libMultiRobotPlanning

Library with search algorithms for task and path planning for multi robot/agent systems
MIT License
839 stars 220 forks source link

Ran `clang-format` target which properly formatted all files. #4

Closed kylevedder closed 5 years ago

kylevedder commented 5 years ago

Ran the clang-format target defined in the CMakeLists.txt file in order to get consistent formatting of the source files.

whoenig commented 5 years ago

Which clang-format version are you using? I also ran clang-format before submitting, so I am surprised that this caused so many changes. To my knowledge, clang-format's output is version-dependent. Here is the version I am using:

$clang-format --version
clang-format version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)
kylevedder commented 5 years ago

On my machine:

$ clang-format --version
clang-format version 6.0.0-1ubuntu2 (tags/RELEASE_600/final)

The .clang-format file also impacts formatting:

$ md5sum .clang-format 
359c1a97a35266ac1249a19ff451686d  .clang-format

I did not realize that the output was so heavily version dependent.

kylevedder commented 5 years ago

I ran make clang-format on this branch on an Ubuntu 16.04 install which has the same clang-format version as what you have. Unfortunately, there is some disagreement between the two clang-format versions related to bin packing function arguments:

$ git diff
diff --git a/example/cbs_ta.cpp b/example/cbs_ta.cpp
index f23bf6f..4f8b454 100644
--- a/example/cbs_ta.cpp
+++ b/example/cbs_ta.cpp
@@ -272,9 +272,8 @@ class Environment {
     for (size_t i = 0; i < startStates.size(); ++i) {
       for (const auto& goal : goals[i]) {
         m_assignment.setCost(
-            i, goal,
-            m_heuristic.getValue(Location(startStates[i].x, startStates[i].y),
-                                 goal));
+            i, goal, m_heuristic.getValue(
+                         Location(startStates[i].x, startStates[i].y), goal));
         m_goals.insert(goal);
       }
     }
diff --git a/example/ecbs_ta.cpp b/example/ecbs_ta.cpp
index 2c19080..57ef177 100644
--- a/example/ecbs_ta.cpp
+++ b/example/ecbs_ta.cpp
@@ -272,9 +272,8 @@ class Environment {
     for (size_t i = 0; i < startStates.size(); ++i) {
       for (const auto& goal : goals[i]) {
         m_assignment.setCost(
-            i, goal,
-            m_heuristic.getValue(Location(startStates[i].x, startStates[i].y),
-                                 goal));
+            i, goal, m_heuristic.getValue(
+                         Location(startStates[i].x, startStates[i].y), goal));
         m_goals.insert(goal);
       }
     }

However, a fix to this is to disable bin packing of arguments, a style I'm actually a fan of personally anyway because I think it makes function arguments for invocations that are longer than a line easier to read. To see what this looks like, you can update the following two lines in .clang-format:

BinPackArguments: false
BinPackParameters: false