catchorg / Clara

A simple to use, composable, command line parser for C++ 11 and beyond
Boost Software License 1.0
650 stars 67 forks source link

Accept const char*[] argv in Args constructor #56

Closed ronen closed 6 years ago

ronen commented 6 years ago

This makes it easier to run tests using string literals.

Currently this fails:

const char *v[] = { "arg0", "arg1" };
Args(2, v);          #   error: invalid conversion from 'const char**' to 'char**' 

and this warns (fails if you fail on warnings):

char *v[] = { "arg0", "arg1" };  # warning: ISO C++ forbids converting a string constant to 'char*' 
Args(2, v);         

And so the only workable solution is to use const_cast

codecov[bot] commented 6 years ago

Codecov Report

Merging #56 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #56   +/-   ##
=======================================
  Coverage   93.69%   93.69%           
=======================================
  Files           2        2           
  Lines         682      682           
=======================================
  Hits          639      639           
  Misses         43       43
Impacted Files Coverage Δ
include/clara.hpp 91.86% <ø> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5f13d59...3ccdcf3. Read the comment docs.

philsquared commented 6 years ago

Thanks for this. I've been meaning to get to it anyway. However, because we need to be able to pass the args in as they come from main(), the standard signature of which uses non-const char arrays, this needs to be provided as an overload, rather than just changing the signature (in fact, some time ago - in the previous version of Clara IIRC - I did have the const in there, but it broke too many people).

So I've now added it as an overload (char const** and char**, to make them easier to interoperate).

https://github.com/catchorg/Clara/commit/caff01f4e38222bf905ea3db49748bf824835edf

horenmar commented 6 years ago

Actually IIRC you should be able to do char const * const * and accept std main arguments that way.

philsquared commented 6 years ago

that's not been my experience

philsquared commented 6 years ago

With just:

Args( int argc, char const** argv ) { /* ... */ }
char* args[] = { (char*)"TestApp", (char*)"hello" };
auto result = cli.parse( Args( 2, args ) );
ClaraTests.cpp:423:34: error: no matching constructor for initialization of 'clara::detail::Args'
        auto result = cli.parse( Args( 2, args ) );
philsquared commented 6 years ago

wait - missed a const - adding that in makes it work - yet it hasn't done previously. Need to check back...

philsquared commented 6 years ago

I can't find any record of the const* const* version being reported as a problem - and it works on all the compilers we claim to support - so I must have dreamt it! In which case I'm certainly in favour of using that signature as the simplest (and most expressive) option! Thanks.

ronen commented 6 years ago

cool thx