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

Error when value is NULL #84

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Error when value is NULL #84

wants to merge 1 commit into from

Conversation

wideti
Copy link

@wideti wideti commented Mar 7, 2018

#83

When the value assigned to the key is NULL. Instead of returning an exception, I consider the value set of the 'default' for the option.

…returning an exception, I consider the value set of the 'default' for the option.
Copy link
Contributor

@NeoVance NeoVance left a comment

Choose a reason for hiding this comment

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

I don't think that an option's argument should automatically become optional because a default value is supplied for that option. See additional comments.

@@ -439,9 +439,14 @@ public function parse()
// the next token MUST be an "argument" and not another flag/option
$token = array_shift($tokens);
list($val, $type) = $this->_parseOption($token);
if ($type !== self::OPTION_TYPE_ARGUMENT)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is here because we expect that if the user is supplying the long/short flag that expects an argument, that they would also supply the argument value. Optional flags can still have a default value, but if they user just wants the default value, they don't need to supply to flag on the command line at all. Take this excerpt from man grep for example.

2.1.2 Matching Control
-e pattern
--regexp=pattern
Use pattern as the pattern. If this option is used multiple times or is combined with the -f (--file) option, search for all patterns given. (-e is specified by POSIX.)

-f file
--file=file
Obtain patterns from file, one per line. If this option is used multiple times or is combined with the -e (--regexp) option, search for all patterns given. The empty file contains zero patterns, and therefore matches nothing. (-f is specified by POSIX.)

-i
-y
--ignore-case
Ignore case distinctions, so that characters that differ only in case match each other. Although this is straightforward when letters differ in case only via lowercase-uppercase pairs, the behavior is unspecified in other situations. For example, uppercase “S” has an unusual lowercase counterpart “ſ” (Unicode character U+017F, LATIN SMALL LETTER LONG S) in many locales, and it is unspecified whether this unusual character matches “S” or “s” even though uppercasing it yields “S”. Another example: the lowercase German letter “ß” (U+00DF, LATIN SMALL LETTER SHARP S) is normally capitalized as the two-character string “SS” but it does not match “SS”, and it might not match the uppercase letter “ẞ” (U+1E9E, LATIN CAPITAL LETTER SHARP S) even though lowercasing the latter yields the former.

-y is an obsolete synonym that is provided for compatibility. (-i is specified by POSIX.)

-v
--invert-match
Invert the sense of matching, to select non-matching lines. (-v is specified by POSIX.)

--regexp/-e and --file/-f expect an argument to be supplied. This is the default behavior for all flags in Commando.

--ignore-case/-i and --invert-match/-v do not expect a value to be supplied as an argument, but this is not because they have some optional argument with a value that can be defaulted per se. It is because they are a Boolean type of flag. They are either supplied, or they are not. Their mere presence on in the command signals all that the application needs to know in order to fulfill the desired behavior.

Having a flag that is a hybrid Boolean, and can be supplied on the command line with or without an argument is something else entirely. Like --color.

--color[=WHEN]
--colour[=WHEN]
Surround the matched (non-empty) strings, matching lines, context lines, file names, line numbers, byte offsets, and separators (for fields and groups of context lines) with escape sequences to display them in color on the terminal. The colors are defined by the environment variable GREP_COLORS and default to ‘ms=01;31:mc=01;31:sl=:cx=:fn=35:ln=32:bn=32:se=36’ for bold red matched text, magenta file names, green line numbers, green byte offsets, cyan separators, and default terminal colors otherwise. The deprecated environment variable GREP_COLOR is still supported, but its setting does not have priority; it defaults to ‘01;31’ (bold red) which only covers the color for matched text. WHEN is ‘never’, ‘always’, or ‘auto’.

For something like this, I would probably want to add a property on Option to allow this behavior rather than making it the default behavior for when Option::$default is set. I think it would be better if there was another way to set a flag's argument as optional. This would allow us to have a flag like color, which has a default value of never and when supplied without arguments changes the value to auto, but can take additional argument values like always. Color kind of operates like an Enum so maybe that can be the property name. Check if Option::isEnum() etc...

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