Added arg argument when throwing a ArgParserException.

[fa0311]

• Thanks for your contribution! Please replace this text with a description of what this PR is changing or adding and why, list any relevant issues, and review the contribution guidelines below.

Although ArgParserException inherits from FormatException, the source is always null.
This is a very small fix, but please let me know if testing is needed.

---

• I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

• See our contributor guide for general expectations for PRs.
• Larger or significant changes should be discussed in an issue before creating a PR.
• Contributions to our repos should follow the Dart style guide and use dart format.
• Most changes should add an entry to the changelog and may need to rev the pubspec package version.
• Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[fa0311]

The current FormatException messages are inconsistent, making this FormatException.source in the Pull Request difficult to handle. For example, a hyphen is prefixed to solo options, but not to regular options.

For instance:

# https://github.com/dart-lang/args/blob/e623652744c82533829f2e62b1aba1a6cf06e291/lib/src/parser.dart#L148
args: [-p, foo, install]
message: Could not find an option or flag "-p".

# https://github.com/dart-lang/args/blob/e623652744c82533829f2e62b1aba1a6cf06e291/lib/src/parser.dart#L289
args: [--path, foo, install]
message: Could not find an option named "path".

Additionally, error messages might differ from the format of the user’s input, especially when aliases or abbreviated forms are used. For example:

For instance:

# https://github.com/dart-lang/args/blob/e623652744c82533829f2e62b1aba1a6cf06e291/lib/src/parser.dart#L148
args: [-f]
message: Could not find an option or flag "-f".

# https://github.com/dart-lang/args/blob/e623652744c82533829f2e62b1aba1a6cf06e291/lib/src/parser.dart#L123
args: [-f]
message: Missing argument for "file".

Do you have any thoughts on this? I’m considering standardizing the error messages to include hyphens.

[fa0311]

This Pull Request improves the clarity of error messages. Additionally, it adds related tests.

[fa0311]

I think that super.source should contain the entire input, not just the corresponding argument.
In other words, on this Pull Request, super.source should be renamed to super.args, and the full input should be assigned to super.source.
By doing this, offset can function properly.

Here’s an example:

// input: sub --color apple
ArgParserException( 
  message: "error message",
  commands: ["sub"],
  args: '--color', // The specific argument where the error occurred
  source: ["sub", "--color", "apple"], // The actual input
  offset: 2, // The key of the value where the error occurred relative to source (in this case, a value is chosen, not an `--color`)
)

My motivation for this is to display user-friendly error messages in software that utilizes this.
However, to implement this, it would be necessary to count how many times _args.removeFirst() has been called in the Parser.

[jakemac53]

Hi! I just wanted to mention that I will circle back here next week, we had an off-site this week so I haven't had time to review, but I haven't forgotten about this PR either.

[jakemac53]

One thing I am struggling with here is what the actual user visible impact of this change is, can you show me an example of how this affects the user experience when you pass an invalid argument etc?

We probably should have some integration tests or something, but that is out of scope for this PR.

[jakemac53]

I think that super.source should contain the entire input, not just the corresponding argument.
In other words, on this Pull Request, super.source should be renamed to super.args, and the full input should be assigned to super.source.
By doing this, offset can function properly.

I was also wondering about this, but worried about how much work it would be and if it would be breaking. If you are willing to give it a shot, I would be happy to review that.

[fa0311]

One thing I am struggling with here is what the actual user visible impact of this change is, can you show me an example of how this affects the user experience when you pass an invalid argument etc?

This pull request addresses several issues, but the main user-facing change is the improvement in error messages when invalid arguments are passed. Below are some examples:

import 'package:args/args.dart';

void main() {
  try {
    final parser = ArgParser();
    final _ = parser.parse(['-f', 'bar']);
  } on FormatException catch (e) {
    print(e.message); // Could not find an option or flag "-f".
  }

  try {
    final parser = ArgParser();
    final _ = parser.parse(['--foo', 'bar']);
  } on FormatException catch (e) {
    print(e.message); // Could not find an option named "foo".
  }

  try {
    final parser = ArgParser();
    parser.addOption('input', abbr: 'i');
    final _ = parser.parse(['--input']);
  } on FormatException catch (e) {
    print(e.message); // Missing argument for "input".
  }

  try {
    final parser = ArgParser();
    parser.addOption('input', abbr: 'i');
    final _ = parser.parse(['-i']);
  } on FormatException catch (e) {
    print(e.message); // Missing argument for "input".
  }

  try {
    final parser = ArgParser();
    parser.addOption('input', abbr: 'i', aliases: ['add']);
    final _ = parser.parse(['--add']);
  } on FormatException catch (e) {
    print(e.message); // Missing argument for "input".
  }
}

With this PR, error messages will now look like this:

import 'package:args/args.dart';

void main() {
  try {
    final parser = ArgParser();
    final _ = parser.parse(['-f', 'bar']);
  } on FormatException catch (e) {
    print(e.message); // Could not find an option or flag "-f".
  }

  try {
    final parser = ArgParser();
    final _ = parser.parse(['--foo', 'bar']);
  } on FormatException catch (e) {
    print(e.message); // Could not find an option named "--foo".
  }

  try {
    final parser = ArgParser();
    parser.addOption('input', abbr: 'i');
    final _ = parser.parse(['--input']);
  } on FormatException catch (e) {
    print(e.message); // Missing argument for "--input".
  }

  try {
    final parser = ArgParser();
    parser.addOption('input', abbr: 'i');
    final _ = parser.parse(['-i']);
  } on FormatException catch (e) {
    print(e.message); // Missing argument for "-i".
  }

  try {
    final parser = ArgParser();
    parser.addOption('input', abbr: 'i', aliases: ['add']);
    final _ = parser.parse(['--add']);
  } on FormatException catch (e) {
    print(e.message); // Missing argument for "--add".
  }
}

The changes make the messages more intuitive and consistent. Here's a detailed explanation of the key improvements:

Standardizing Hyphen: In the current version, the use of hyphen is inconsistent. This PR will ensure that hyphen are uniform throughout the messages.

Example before:

try {
  final parser = ArgParser();
  final _ = parser.parse(['-f', 'bar']);
} on FormatException catch (e) {
  print(e.message); // Could not find an option or flag "-f".
}

try {
  final parser = ArgParser();
  final _ = parser.parse(['--foo', 'bar']);
} on FormatException catch (e) {
  print(e.message); // Could not find an option named "foo".
}

The second point is about unifying the argument names displayed when an error occurs.

For errors like Missing argument for "xxxxxxx"., the argument name will always be exactly what is provided to _addOption.
For errors like Option "xxxxxxx" must be a flag to be in a collapsed "-"., the message will always display exactly what the user input.

The latter approach (displaying what the user entered) is more intuitive for users, so we are adopting it consistently.

For example:

try {
  final parser = ArgParser();
  parser.addOption('input', abbr: 'i');
  final _ = parser.parse(['-i']);
} on FormatException catch (e) {
  print(e.message); // Missing argument for "input".
}

try {
  final parser = ArgParser();
  parser.addFlag('apple', abbr: 'a');
  parser.addOption('banana', abbr: 'b');
  parser.addFlag('cherry', abbr: 'c');
  final _ = parser.parse(['-abc']);
} on FormatException catch (e) {
  print(e.message); // Option "-b" must be a flag to be in a collapsed "-".
}

[fa0311]

I was also wondering about this, but worried about how much work it would be and if it would be breaking. If you are willing to give it a shot, I would be happy to review that.

I gave it a try in #285.

[jakemac53]

@natebosch did you want to take a look at this?