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

Wrong text when no arguments supplied #234

Open
rnwhitehead opened this issue Nov 8, 2022 · 9 comments
Open

Wrong text when no arguments supplied #234

rnwhitehead opened this issue Nov 8, 2022 · 9 comments

Comments

@rnwhitehead
Copy link

If I require 3 positional arguments, but none are supplied, it always says "1 argument(s) expected. 0 provided.", rather than "3 arguments(s) expected..."
It doesn't matter if you add ".required()" or not.
This is confusing for the user. It would be better to simply not print this line, in fact - the help output is sufficient; prefixing it with this incorrect information is just confusing..

@skrobinson
Copy link
Contributor

A short-term fix would be to use a different pattern.

// confusing errors
program.add_argument("first name");
program.add_argument("middle name");
program.add_argument("last name");

// versus

// gives correct count of missing arguments
program.add_argument("names").nargs(3);

This should give a more meaningful error to your users. But, yes, it may not semantically fit with your use.

Please leave this Issue open. This has come up before and we should address it.

@skrobinson
Copy link
Contributor

@rnwhitehead I looked at this again and I'm now not sure it's as bad as I first believed.

The "confusing errors" pattern gives what(): first name: 1 argument(s) expected. 0 provided. Which is correct for the way I laid out the arguments.

Can you give a simplified example of how you are using positional arguments?

@rnwhitehead
Copy link
Author

rnwhitehead commented Nov 11, 2022

I don't see why it's right: it needs 3 positional parameters, why does it say "1 argument(s) expected"? It should say "3 positional arguments expected" or if that can't be done, "Insufficient positional arguments provided", or simply don't say anything - which is what most commands and apps do on Linux:

$ apt
apt 2.0.9 (amd64)
Usage: apt [options] command

apt is a commandline package manager and ...
...

@p-ranav
Copy link
Owner

p-ranav commented Nov 11, 2022

why does it say "1 argument(s) expected"?

It says first name: 1 argument(s) expected... as in.. 1 argument expected for first_name but zero was provided. It's the first point of failure.

If the program specified

program.add_argument("first name").nargs(3);

then you'd see first name: 3 argument(s) expected. 0 provided.

@marzer
Copy link
Contributor

marzer commented Nov 11, 2022

That is confusing from a user perspective, though. IMO the "N arguments expected" should be the sum of all nargs for all positional arguments, because anything else is ambiguous. The way it is specified in code shouldn't really matter here; it's about what makes sense to the user trying to interpret the error message.

@p-ranav
Copy link
Owner

p-ranav commented Nov 11, 2022

IMO the "N arguments expected" should be the sum of all nargs for all positional arguments, because anything else is ambiguous.

How do you think it should it be presented for nargs that are ranges?

Consider this example:

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

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

  program.add_argument("first_name");
  program.add_argument("middle_name");
  program.add_argument("last_name").nargs(1, 3);

  try
    {
      program.parse_args(argc, argv);
    }
  catch (const std::exception &err)
    {
      std::cerr << err.what() << "\n";
      std::cerr << program;
      return 1;
    }

  std::cout << program.get("first_name") << "\n";
  std::cout << program.get("middle_name") << "\n";
  std::cout << program.get("last_name") << "\n";

  return 0;
}

with usage:

foo:bar$ ./program Foo Bar

argparse will currently report this error:

foo:bar$ ./main Foo Bar
last_name: 1 to 3 argument(s) expected. 0 provided.
Usage: program_name [-h] first_name middle_name last_name

Positional arguments:
  first_name   	
  middle_name  	
  last_name    	[nargs=1..3] 

Optional arguments:
  -h, --help   	shows help message and exits 
  -v, --version	prints version information and exits 

Should it instead sum all the nargs and say 3 to 6 positional arguments expected?

That's sounds more ambiguous than simply reporting that for the current positional argument last_name: 1 to 3 argument(s) expected. 0 provided.

@marzer
Copy link
Contributor

marzer commented Nov 11, 2022

Should it instead sum all the nargs and say 3 to 6 positional arguments expected?

Yea, I think so. It seems clearer to me, though of course that's a matter of taste.

More fundamentally you could argue that an application with multiple positional arguments with varying amounts of nargs is just a crap design altogether, but I guess that's beside the point 😅

@rnwhitehead
Copy link
Author

rnwhitehead commented Nov 11, 2022

Honestly, the rest of the help output is so clear, you could simply not output this line, it isn't really helping.
If there really is something wrong with the first positional argument (e.g. it's a string when it must be a number) then a detailed error about that argument makes sense: but saying that the first positional argument was expected to have count 1 when it had count zero is a really confusing way to say "you didn't provide any arguments".
It reminds me of an application I had for downloading pictures off a digital camera: it would report something like "length of cameras vector is zero", which is a really solution-oriented way of saying "please connect the camera and make sure it is turned on".

@skrobinson
Copy link
Contributor

@rnwhitehead It sounds like the default error message is not appropriate for your users. So catch the exception and throw your own error message.

  try {
    program.parse_args(arguments);
  } catch ( const std::runtime_error &e ) {
    if ( e.what() == std::string{"first name: 1 argument(s) expected. 0 provided."} ) {
      throw std::runtime_error("a full name (at least three words) is required");
    }
    if ( e.what() == std::string{"middle name: 1 argument(s) expected. 0 provided."} ) {
      throw std::runtime_error("only a first name was given, a full name is required");
    }
    if ( e.what() == std::string{"last name: 1 argument(s) expected. 0 provided."} ) {
      throw std::runtime_error("almost there, three names are required");
    }
  }

Rather than viewing the argparse error message as the mandatory user message, think of it as a starting point for your custom messages. In my own projects, I've used a similar pattern to replace the generic missing positional argument error with a reminder that a configuration file path is required.

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

4 participants