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

add_argument should return the same name exist argument #224

Open
lingerer opened this issue Oct 18, 2022 · 7 comments
Open

add_argument should return the same name exist argument #224

lingerer opened this issue Oct 18, 2022 · 7 comments

Comments

@lingerer
Copy link

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?

@skrobinson
Copy link
Contributor

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

@lingerer
Copy link
Author

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.

@skrobinson
Copy link
Contributor

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?

@lingerer
Copy link
Author

lingerer commented Oct 18, 2022

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

@skrobinson
Copy link
Contributor

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");

@lingerer
Copy link
Author

lingerer commented Jan 7, 2023

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
Copy link
Author

lingerer commented Jan 7, 2024

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.

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

No branches or pull requests

2 participants