Skip to content

Conversation

SimonCropp
Copy link
Contributor

So usages of TryGetOptionArgumentList are problematic

  • some usages use the first arg and ignore the rest
  • some usages check if there is only one and dont use the first if there is more than one
  • some usages assume there is 1 and will throw if that is not correct

so this PR adds TryGetOptionArgument. which will formalize the usages. it will:

  • return false if there is no arg
  • return false if there is no items in the arg
  • throw if there is more than 1

@SimonCropp SimonCropp marked this pull request as draft July 10, 2025 12:20
Comment on lines +31 to +36
if (!options.TryGetOptionArgumentList(optionName, out string[]? arguments) ||
arguments.Length == 0)
{
argument = null;
return false;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would return false only if TryGetOptionArgumentList returned false. Otherwise, throw if arguments.Length != 1, and I would probably throw Unreachable, since validation should have happened early and by the point we are calling this, we should be expecting us to be in a valid state.

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

Successfully merging this pull request may close these issues.

2 participants