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.

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

Support for enumerations #12

Closed
Maktm opened this issue Jun 2, 2017 · 9 comments · Fixed by #15
Closed

Support for enumerations #12

Maktm opened this issue Jun 2, 2017 · 9 comments · Fixed by #15
Assignees
Milestone

Comments

@Maktm
Copy link

Maktm commented Jun 2, 2017

I tried the following code to see whether CLI11 supports the use of enumerations in add_set and got a series of errors. Is the code wrong is support for enumerations planned/not going to be added?

#include <iostream>
#include <string>

#include <CLI/CLI.hpp>

enum Level : std::int32_t
{
  High,
  Medium,
  Low
};

int main(int argc, char* argv[])
{
  CLI::App app{"App description"};
  try
  {
    Level level;
    app.add_set("-l,--level", level, {Level::High, Level::Medium, Level::Low}, "Level settings");
    app.parse(argc, argv);
  }
  catch (CLI::Error const& e)
  {
    app.exit(e);
  }
  return EXIT_SUCCESS;
}

Errors:

1>f:\github\cli11\include\cli\typetools.hpp(108): error C2440: '=': cannot convert from 'std::string' to 'Level'
1>f:\github\cli11\include\cli\typetools.hpp(108): note: No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
1>f:\github\cli11\include\cli\app.hpp(326): note: see reference to function template instantiation 'bool CLI::detail::lexical_cast<T,0>(std::string,T &)' being compiled
1>        with
1>        [
1>            T=Level
1>        ]
1>f:\code\visual studio 2017\projects\cmdapp\cmdapp\main.cpp(20): note: see reference to function template instantiation 'CLI::Option *CLI::App::add_set<Level>(std::string,T &,std::set<Level,std::less<_Kty>,std::allocator<_Kty>>,std::string,bool)' being compiled
1>        with
1>        [
1>            T=Level,
1>            _Kty=Level
1>        ]
1>Done building project "cmdapp.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========
@henryiii
Copy link
Collaborator

henryiii commented Jun 2, 2017

I'm curious, what do you expect for the interface? -l 0 or -l High? The second can't work since C++ does not have reflection, so it can't know that High == "0". The first one should have worked, though.

I'll have to see why the SFINAE is selecting the string version for a enum that just wraps an int, though, that should not be happening.

std::is_enum needs to be added along with std::is_int. Would have thought is_int would include enums.

@henryiii
Copy link
Collaborator

henryiii commented Jun 2, 2017

By the way, there is no need to wrap the first couple of lines with try, unless you catch CLI::Error instead of CLI::ParseError. The CLI::ConstructionErrors are not ParseErrors. (Catching CLI::Error instead is perfectly valid) (fixed in example)

@henryiii henryiii added this to the v1.1 milestone Jun 2, 2017
@henryiii henryiii self-assigned this Jun 2, 2017
@Maktm
Copy link
Author

Maktm commented Jun 2, 2017

I'm curious, what do you expect for the interface? -l 0 or -l High?

The first version (i.e. -l 0) and it would be useful if the help message outputted something like this:

Options: -l,--level enum/Level in {High=0, Medium=1, Low=2} Level Settings

@henryiii
Copy link
Collaborator

henryiii commented Jun 4, 2017

C++ can't capture the names of enumerations. See this question. You can manually set that in your help string, though.

@henryiii
Copy link
Collaborator

henryiii commented Jun 5, 2017

Current implementation:

 Level level;
 app.add_set("-l,--level", level, {High, Medium, Low}, "Level settings")
     ->set_type_name("enum/Level in {High=0, Medium=1, Low=2}");
henrys-mbp:build henryiii$ ./examples/enum -h
Usage: ./examples/enum [OPTIONS]
Options:
  -h,--help                   Print this help message and exit
  -l,--level enum/Level in {High=0, Medium=1, Low=2}
                              Level settings

May think about this a little more. Still need to add a few tests.

@lambdafu
Copy link
Contributor

This issue is closed, but I think it would be nice if we could support enums with string identifiers instead integers. I am currently using add_set together with stream operators, but that is false sharing (the stream operators may have a different purpose elsewhere), and more work than it should be (with a weird way to report conversion error). Maybe a map could be used instead?


namespace rang {
std::istream& operator>>(std::istream& in, rang::control& when) {
  std::string label;
  in >> label;
  if (label == "always")
    when = rang::control::Force;
  else if (label == "never")
    when = rang::control::Off;
  else if (label == "auto")
    when = rang::control::Auto;
  else {
    // We are not pushing back what we couldn't parse.  Seems to be OK.
    in.setstate(std::ios_base::failbit);
  }
  return in;
}

std::ostream& operator<<(std::ostream& in, const rang::control& when) {
  switch (when) {
    case rang::control::Force:
      return in << "always";
    case rang::control::Off:
      return in << "never";
    default:
      return in << "auto";
  }
}
}  // namespace rang

class GlobalOptions {
 public:
  rang::control color{rang::control::Auto};
} options;

CLI::App app;
app.add_set("--color", options.color,
            {rang::control::Auto, rang::control::Force, rang::control::Off},
            "colorize the output (auto, always, or never)")
    ->set_type_name("WHEN");

app.set_callback([&options]() { rang::setControlMode(options.color); });

@henryiii henryiii reopened this May 21, 2018
@henryiii
Copy link
Collaborator

I'd prefer (I think) a custom class in CLI11 that would support this. Something like CLI::Set with CLI::SetOption items. Maybe could even roll in case insensitivity too.

@lambdafu
Copy link
Contributor

I also tried overloading literal_cast, but it wasn't effective, probably because I didn't define it early enough or so.

@henryiii
Copy link
Collaborator

Okay, with IsMember mapping and built-in support for enums, I think this is finally part of CLI11, headed for 1.8!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants