argparse icon indicating copy to clipboard operation
argparse copied to clipboard

add_argument should return the same name exist argument

Open lingerer opened this issue 3 years ago • 7 comments

add_argument now with auto argument = m_optional_arguments.emplace(std::cend(m_optional_arguments), m_prefix_chars, array_of_sv{f_args...}); and [] will return the exist argumint in m_argument_map and throw exception when missing.

In most cases it should no allow the same name argument right? Why not return the exist argument in m_argument_map instead?

lingerer avatar Oct 18 '22 08:10 lingerer

Please correct me if I misunderstand. Are you reporting that the following code will compile, but running the compiled binary will almost always give a confusing error or other unexpected result?

#include <argparse/argparse.hpp>
#include <iostream>

int main(int argc, char *argv[]) {
  argparse::ArgumentParser program("program_name");

  program.add_argument("file");
  program.add_argument("file");

  program.parse_args(argc, argv);

  std::cout << program.get("file") << std::endl;
  return 0;
}
$ program
terminate called after throwing an instance of 'std::runtime_error'
  what():  file: 1 argument(s) expected. 0 provided.
Aborted

$ program first_file.txt
terminate called after throwing an instance of 'std::runtime_error'
  what():  file: 1 argument(s) expected. 0 provided.
Aborted

$ program first_file.txt second_file.txt
second_file.txt

skrobinson avatar Oct 18 '22 13:10 skrobinson

A little bit different case `

argparse::ArgumentParser test("test", "1.0", argparse::default_arguments::none);

test.add_argument("--vel").default_value(0.0).scan<'f',double>;
test.add_argument("--vel").default_value(0.0).scan<'f',double>;

    test.parse_args(argc,argv);
    std::cout<<test.get<double>("--vel")<<std::endl;

` This will work but the m_optional_arguments will contain 2 arguments. Compare another case,in one function:

test.add_argument("--vel").default_value(0.0); then

test["--vel"].scan<'f',double>;

the risk is "--vel" may not exist in function 2,so a modified code like

test.add_argument("--vel").default_value(0.0).scan<'f',double>;

is a more natural way.

lingerer avatar Oct 18 '22 14:10 lingerer

I think I understand. You want

  program.add_argument("--file");
  program.add_argument("--file").default_value("badger-sett-001.txt");

to not create a new, second Argument, but still return an Argument (in this case the first instance) for function chaining?

skrobinson avatar Oct 18 '22 15:10 skrobinson

That's right. It's more natural. Add new argument will make a confusing usage() result.

lingerer avatar Oct 18 '22 15:10 lingerer

I've thought more about this and I'm not a fan of two meanings for a single function. Your proposal would have add_argument take on the meaning: "add a new Argument or return the existing one with that name".

PR #245 does not directly address this, but could be used to make a stand-alone function to add or return an existing Argument.

argparse::Argument &add_or_get(argparse::ArgumentParser parser, const std::string arg) {
  argparse::Argument new_arg;
  try {
    new_arg = parser.at(arg);
  } catch () {
    new_arg = parser.add_argument(arg);
  }
  return &new_arg;
}


// use example
add_or_get(program, "--file");
add_or_get(program, "--file").default_value("badger-sett-001.txt");

skrobinson avatar Jan 05 '23 20:01 skrobinson

I think it's not about two meanings. What's the point about adding dunplicate argument? if all the arguments are added in one function that's fine.But if arguments could be added by others , I guess that code add_argument means He/She/AnyGender will need that argument and just too lazy to add a test code.

lingerer avatar Jan 07 '23 02:01 lingerer

I've thought more about this and I'm not a fan of two meanings for a single function. Your proposal would have add_argument take on the meaning: "add a new Argument or return the existing one with that name".

PR #245 does not directly address this, but could be used to make a stand-alone function to add or return an existing Argument.

argparse::Argument &add_or_get(argparse::ArgumentParser parser, const std::string arg) {
  argparse::Argument new_arg;
  try {
    new_arg = parser.at(arg);
  } catch () {
    new_arg = parser.add_argument(arg);
  }
  return &new_arg;
}


// use example
add_or_get(program, "--file");
add_or_get(program, "--file").default_value("badger-sett-001.txt");

the add_or_get looks won't work here.

lingerer avatar Jan 07 '24 15:01 lingerer