colcon / colcon-cmake

Extension for colcon to support CMake packages
http://colcon.readthedocs.io
Apache License 2.0
16 stars 25 forks source link

Document --cmake-args needs a quoted space before an argument with a dash on windows #7

Closed sloretz closed 6 years ago

sloretz commented 6 years ago

The help text for --cmake-args now says to quote a space if using cmd on windows.

Arguments for 'cmake' packages:
  --cmake-args [* [* ...]]
                        Arbitrary arguments which are passed to all CMake
                        projects. Args that start with "-" must be prefixed
                        with a space. If using bash then use
                                --cmake-args \ -Dvar=val
                        If using Windows cmd then use
                                --cmake-args " -Dvar=val"
Arguments for 'cmake' packages:
  --ctest-args [* [* ...]]
                        Arbitrary arguments which are passed to all CTest
                        projects. Args that start with "-" must be prefixed
                        with a space. If using bash then use
                                --ctest-args \ -L label
                        If using Windows cmd then use
                                --ctest-args " -L" label

connects to colcon/colcon-cmake#6

mikaelarguedas commented 6 years ago

Does the same apply to ctest-args and other greedy arguments ?

sloretz commented 6 years ago

@mikaelarguedas The same applies. I added documentation to --ctest-args in this PR and in colcon/colcon-ros#3 --ament-cmake-args and --catkin-cmake-args

mikaelarguedas commented 6 years ago

Sounds good, I commented on another ticket with a suggestion to keep it simple and avoid calling out specific shells

sloretz commented 6 years ago

@mikaelarguedas ff9db414af20957c70e052cdba911caeea70547c

  --cmake-args [* [* ...]]
                        Arbitrary arguments which are passed to all CMake
                        projects. Args that start with "-" must be prefixed
                        with a space.
                                --cmake-args " -Dvar=val"
  --ctest-args [* [* ...]]
                        Arbitrary arguments which are passed to all CTest
                        projects. Args that start with "-" must be prefixed
                        with a space.
                                --ctest-args " -L" label
mikaelarguedas commented 6 years ago

@sloretz looks good to me, can you please open a similar PR on colcon-core for pytest-args ? https://github.com/colcon/colcon-core/blob/bdf0516c9d4cd0004f7a5d7ba4e4e9fd75b3b92c/colcon_core/task/python/test/pytest.py#L38-L40

dirk-thomas commented 6 years ago

The current help text is getting pretty long (4 lines). Let me try to suggest a shorter version which contains the improvements of this patch...

mikaelarguedas commented 6 years ago

The current help text is getting pretty long (4 lines)

It seems that this PR is decreasing the length of the help text rather than increasing it, though I agree that it would benefit from being even shorter :+1:

dirk-thomas commented 6 years ago

How about the following which I picked for these reasons:

  --cmake-args [* [* ...]]
                        Pass arguments to all CMake projects. Every arg
                        starting with a dash must be prefixed by a space, e.g.
                            --cmake-args " -Dvar=val"
dirk-thomas commented 6 years ago

With all the variations (CMake, CTest, Ament, Pytest) and their different lengths I came up with this wrapping instead which works consistently across all cases:

  --cmake-args [* [* ...]]
                        Pass arguments to all CMake projects. Every arg
                        starting with a dash must be prefixed by a space,
                        e.g. --cmake-args " -Dvar=val"
dirk-thomas commented 6 years ago

After some offline iteration I committed a slightly more condensed version in 5cd23c6f562ea3767939bec6dc91e1034671b44e.