Value passed to a flag option is ignored (but gives no errors)

[andoalon]

I realized that there is a corner case when parsing flag options: if the command-line contains a value for the flag, it will get ignored, which can be confusing:

bool wait_for_input = false;

(lyra::cli()
  |= lyra::opt(wait_for_input)
  ["--wait-for-input"]).parse(lyra::args({ argv[0], "--wait-for-input=no" }));

// wait_for_input == true
// Although we specified '=no'

The reason for that seems to be located in opt::parse:

if (m_ref->isFlag())
{
  // remove 'token' without checking whether it contains
  // a value with a delimiter, i.e. --option=myvalue
  // vvvvvvvvvvvvvvvvvvvvvvvv
  remainingTokens.pop(token);
  auto flagRef
    = static_cast<detail::BoundFlagRefBase *>(m_ref.get());
  auto result = flagRef->setFlag(true);

I initially tried to make flags optionally accept values, so that if no value is provided, true is used, and the value otherwise: --wait-for-input -> true --wait-for-input=yes -> true --wait-for-input=no -> false

The code looked something like this:

auto const& argToken = remainingTokens.value();
auto result = (argToken.type == detail::token_type::unknown)
  ? flagRef->setFlag(true)
  : flagRef->setValue(argToken.name);

Unfortunately, this doesn't work in practice:

• If a value is provided (--wait-for-input=yes or --wait-for-input no) it works ✅
• If no value is provided (--wait-for-input), it seems to work...
• ...unless there are more tokens afterwards, in which case they get interpreted as values for the flag, and it fails
  --wait-for-input --other-option=3 -> Unable to convert '--other-option' to destination type ❌

Therefore, my suggestion would be to not accept values for flag options at all, but make sure that no value is provided with a delimiter:

if (m_ref->isFlag())
{
  // don't pop the token just yet
  auto flagRef
    = static_cast<detail::BoundFlagRefBase *>(m_ref.get());

  if (remainingTokens.has_option_prefix() && remainingTokens.has_value_delimiter()) // from token_iterator::value()
  {
    // --option=x, -o=x
    auto const& argToken = remainingTokens.value();

    return parse_result::runtimeError(
      { parser_result_type::no_match, remainingTokens },
      "Flag option '" + token.name + "' contains value '" + argToken.name + "'. Flag options cannot contain values ");
  }
  remainingTokens.pop(token); // now we can pop the token

  auto result = flagRef->setFlag(true);
  // ...

While we are at it, we might also add some extra validation that checks that no value_choices are provided for flags (choices don't make sense for them, as far as I know):

if (value_choices)
{
  return parse_result::logicError(
    { parser_result_type::no_match, remainingTokens },
    "Flag options cannot contain choices. Token: '" + token.name + '\'');
}

Please let me know if I misunderstood something, and give me feedback if you like the idea so that I can create a pull request. Thank you for the awesome library!

[grafikrobot]

Sorry for the really late reply. All good ideas. I'm thinking about the best direction. And thanks for offering to help. But I'm going to go ahead and implemented something int he next few days to resolve it. Since it would be unfair of me to ask you to do the work after such a long time.

[andoalon]

Nice! Let me know if I can help, I'm still 100% open for collaborating/discussing (I don't think it's unfair for you to ask for help), I was just unsure what the best direction was. Thank you for your work 😊