dart-lang / args

A command-line argument parsing library for Dart.
https://pub.dev/packages/args
BSD 3-Clause "New" or "Revised" License
213 stars 63 forks source link

Setting the mandatory parameter of the addOption function to true has no effect. #255

Open Fralacticus opened 1 year ago

Fralacticus commented 1 year ago

I see that setting the mandatory parameter of the addOption function to true has no effect. In the example below, the arguments array contains none of the 3 mandatories options, and the program throws no exception during parsing.

import 'dart:io';
import 'package:args/args.dart';

void main(List<String> arguments) {
  final ArgParser parser = ArgParser()
    ..addOption('action', abbr: "a", mandatory: true, allowed: ['c1', 'c2', 'c3', 'c4', 'c5', 'c6', 'c7', 'c8', 'c9', 'd'])
    ..addOption("fichier_entree", mandatory: true, abbr: "e", help: "Chemin du fichier d'entrée.")
    ..addOption("fichier_sortie", mandatory: true, abbr: "s", help: "Chemin du fichier de sortie");

  arguments = ["garbage"];

  ArgResults results;
  try {
    results = parser.parse(arguments);
  }catch(error) {
    stderr.writeln(error);
    exit(64);
  }
}
slightfoot commented 8 months ago

This was behaviour introduced in version 2.4.2 by #245 that just tripped me up also. Add callback: (_){}, to our addOption and it will work around it.

alexeyinkin commented 3 months ago

Add callback: (_){}

This:

  1. Will break --help functionality, if any.
  2. Is undocumented, just happens to work, and may stop working in the future.

A manual check seems more reliable. When you make sure no --help is passed, do this:

for (final option in parser.options.values) {
  if (option.mandatory && !results.wasParsed(option.name)) {
    throw ArgParserException('Option ${option.name} is mandatory.');
  }
}
kroikie commented 1 week ago

I think this is actually working as intended. In the OP example the arguments are parsed but not used. From this comment it seems like mandatory is only enforced when the field is used.

If this is correct, @munificent can we have this added to the documentation for mandatory?