Improved synopsis rendering. Improved operands rendering. Reduced clutter. Reduced the number of assumptions in rendering. Removed duplicates. Fixed failing tests. Seriously, why do you use PHP_EOL in message and then suddenly compare to plain \n in test?

Hi,

As proposed in #112 I've added the ability to specify custom validation error messages. I've introduced a new Validator class, and gathered the validation related features in a new WithValidator trait. Kept Argument::hasValidation() for backward compatibility (because it is public...), and marked it as deprecated. No BC change has been introduced, tests and CS ok. Updated documentation and Changelog accordingly.

Note: I think the Invalid exception should be thrown in the Validation, rather in the Operand::setValue() and Option::setValue() methods. The custom message could be defined in the validation when sending the exception. Unfortunately it would make backward compatibility harder to maintain (we would have to handle validation which return false, which means much code just for that). Maybe in 4.0...

For the next major release, because method names should always start with a verb. There may be other classes in need of similar refactoring but this seemed like a good place to start.

The problem is that newer versions are not working on the old php versions - we are staying somehow at a point where it would be the easiest solution to just split up the development with a v3 branch and a new v4 that is only compatible with php 7.1 (I guess will be the matching point where all libraries work on all new versions).

Another option would be to upgrade the packages only in travis (split the jobs to use different versions (with composer require phpunit:8.* for example).

Two dependencies are at fault: PHPUnit and PHP Code Sniffer.

PHPUnit starts spitting deprecated issues with 7.2 and PHP Code Sniffer with 7.4. I guess it is worth giving it a try with the latest versions of both dependencies in 7.4 and see how it reacts, not sure if the source will have to be edited to do so though. What do you think of doing this in a follow up PR to unblock this one?

Originally posted by @PierrickVoulet in https://github.com/getopt-php/getopt-php/pull/162#issuecomment-657585452

My app can read an INI-style text file for default settings, as well as parsing command line parameters through GetOpt. The order of precedence I want is "default value"->"INI value"->"CL value". However, the option for which config file to parse is a command line parameter only. In trying to reconcile this logic, I configure GetOpt with all the commands and options, call GetOpt->process() to populate the values, then read the "config-file" option. I pass that value to a routine which reads the INI and sets the value with GetOpt->getOption($key, TRUE)->setValue($value). This works fine, but leaves the INI values in charge. To reinstate the command line parameters, I call GetOpt->process() again.

I've found that with a command with an operand, the second call to process() assigns the name of the command as the operand's value. For example, on the initial call to process(), I have this command object:

    DisqusImporter\Commands\CreateTablesCommand::__set_state(array(
       'config' => NULL,
       'log' => NULL,
       'name' => 'create-tables',
       'shortDescription' => 'Creates the database schema',
       'longDescription' => 'Runs the SQL necessary to create the database schema.  Requires the name of the file containing the SQL commands.',
       'handler' => 'DisqusImporter\\Commands\\CreateTablesCommand::handle',
       'options' =>
      array (
      ),
       'optionMapping' =>
      array (
      ),
       'operands' =>
      array (
        0 =>
        GetOpt\Operand::__set_state(array(
           'required' => true,
           'description' => NULL,
           'default' => NULL,
           'validation' => NULL,
           'name' => 'sqlfile',
           'multiple' => false,
           'value' => 'init.sql',
           'option' => NULL,
           'validationMessage' => NULL,
        )),
      ),
    )),

After reading the INI, it is the same. After the second call to process(), this is the object:

    DisqusImporter\Commands\CreateTablesCommand::__set_state(array(
       'config' => NULL,
       'log' => NULL,
       'name' => 'create-tables',
       'shortDescription' => 'Creates the database schema',
       'longDescription' => 'Runs the SQL necessary to create the database schema.  Requires the name of the file containing the SQL commands.',
       'handler' => 'DisqusImporter\\Commands\\CreateTablesCommand::handle',
       'options' =>
      array (
      ),
       'optionMapping' =>
      array (
      ),
       'operands' =>
      array (
        0 =>
        GetOpt\Operand::__set_state(array(
           'required' => true,
           'description' => NULL,
           'default' => NULL,
           'validation' => NULL,
           'name' => 'sqlfile',
           'multiple' => false,
           'value' => 'create-tables',
           'option' => NULL,
           'validationMessage' => NULL,
        )),
      ),
    )),

I believe this is a side-effect of the array_merge() in GetOpt::getOperands(), but I haven't traced down there yet. I understand a work-around could be to just discard the original GetOpt object and instantiate a new one, but that seems clumsy.

I am not sure which is the POSIX standard:

command operand --help: --help parsed as option

It would be good if the above example could work.

It is not that hard to implement it, however the CommandLineParser must loop through all the provided options. I can send a PR for it.

But I would like to know whether not supporting this is a decision made or just implemented this way.

I'm currently encountering a weird exception from GetOpt. I defined an option --help (or -h) but every time I insert that option, it throws me an exception of type GetOpt\ArgumentException\Unexpected with the message:

Option 'help' is unknown in file /var/www/melior/Packages/Library/ulrichsg/getopt-php/src/GetOpt.php on line 161

When I debug the GetOpt instance right before the process() call, I can see all my defined options and commands but none of them are actually available:

GetOpt\GetOpt (10) (
    protected 'additionalOperands' -> array (0) []
    protected 'additionalOptions' -> array (0) []
    protected 'command' -> null
    protected 'commands' -> array (2) [
        'melior:core:cache:flush' => GetOpt\Command (7) (
            protected 'handler' -> string (38) "Melior\Core\Commands\CacheFlushCommand"
            protected 'longDescription' -> string (30) "Flushes the application caches"
            protected 'name' -> string (23) "melior:core:cache:flush"
            protected 'operands' -> array (0) []
            protected 'optionMapping' -> array (2) [
                'f' => GetOpt\Option (6) (
                    private 'argument' -> GetOpt\Argument (7) (
                        protected 'default' -> null
                        protected 'multiple' -> boolean false
                        protected 'name' -> string (3) "arg"
                        protected 'option' -> GetOpt\Option (6) RECURSION
                        protected 'validation' -> null
                        protected 'validationMessage' -> null
                        protected 'value' -> null
                    )
                    private 'description' -> string (22) "Enables force-flushing"
                    private 'long' -> string (5) "force"
                    private 'mode' -> string (6) ":noArg"
                    private 'short' -> string (1) "f"
                    private 'value' -> null
                )
                'force' => GetOpt\Option (6) (
                    private 'argument' -> GetOpt\Argument (7) (
                        protected 'default' -> null
                        protected 'multiple' -> boolean false
                        protected 'name' -> string (3) "arg"
                        protected 'option' -> GetOpt\Option (6) RECURSION
                        protected 'validation' -> null
                        protected 'validationMessage' -> null
                        protected 'value' -> null
                    )
                    private 'description' -> string (22) "Enables force-flushing"
                    private 'long' -> string (5) "force"
                    private 'mode' -> string (6) ":noArg"
                    private 'short' -> string (1) "f"
                    private 'value' -> null
                )
            ]
            protected 'options' -> array (1) [
                0 => GetOpt\Option (6) (
                    private 'argument' -> GetOpt\Argument (7) (
                        protected 'default' -> null
                        protected 'multiple' -> boolean false
                        protected 'name' -> string (3) "arg"
                        protected 'option' -> GetOpt\Option (6) RECURSION
                        protected 'validation' -> null
                        protected 'validationMessage' -> null
                        protected 'value' -> null
                    )
                    private 'description' -> string (22) "Enables force-flushing"
                    private 'long' -> string (5) "force"
                    private 'mode' -> string (6) ":noArg"
                    private 'short' -> string (1) "f"
                    private 'value' -> null
                )
            ]
            protected 'shortDescription' -> string (30) "Flushes the application caches"
        )
    ]
    protected 'help' -> null
    protected 'operands' -> array (0) []
    protected 'operandsCount' -> integer 0
    protected 'optionMapping' -> array (2) [
        'h' => GetOpt\Option (6) (
            private 'argument' -> GetOpt\Argument (7) (
                protected 'default' -> null
                protected 'multiple' -> boolean false
                protected 'name' -> string (10) "noArgument"
                protected 'option' -> GetOpt\Option (6) RECURSION
                protected 'validation' -> null
                protected 'validationMessage' -> null
                protected 'value' -> null
            )
            private 'description' -> string (47) "Displays help for the CLI or the given command."
            private 'long' -> string (4) "help"
            private 'mode' -> string (6) ":noArg"
            private 'short' -> string (1) "h"
            private 'value' -> null
        )
        'help' => GetOpt\Option (6) (
            private 'argument' -> GetOpt\Argument (7) (
                protected 'default' -> null
                protected 'multiple' -> boolean false
                protected 'name' -> string (10) "noArgument"
                protected 'option' -> GetOpt\Option (6) RECURSION
                protected 'validation' -> null
                protected 'validationMessage' -> null
                protected 'value' -> null
            )
            private 'description' -> string (47) "Displays help for the CLI or the given command."
            private 'long' -> string (4) "help"
            private 'mode' -> string (6) ":noArg"
            private 'short' -> string (1) "h"
            private 'value' -> null
        )
    ]
    protected 'options' -> array (1) [
        0 => GetOpt\Option (6) (
            private 'argument' -> GetOpt\Argument (7) (
                protected 'default' -> null
                protected 'multiple' -> boolean false
                protected 'name' -> string (10) "noArgument"
                protected 'option' -> GetOpt\Option (6) RECURSION
                protected 'validation' -> null
                protected 'validationMessage' -> null
                protected 'value' -> null
            )
            private 'description' -> string (47) "Displays help for the CLI or the given command."
            private 'long' -> string (4) "help"
            private 'mode' -> string (6) ":noArg"
            private 'short' -> string (1) "h"
            private 'value' -> null
        )
    ]
    protected 'settings' -> array (3) [
        'strictOptions' => boolean true
        'strictOperands' => boolean false
        'scriptName' => string (8) "./melior"
    ]
)

The terminal always only shows up this notice:

Usage: ./melior [operands]

Is this somehow a bug or am I missing something that breaks my implementation? If further information is needed, I'll provide it as soon as possible. Thanks in advance for your help, Max

I'd like to see a way to enable explicit options, meaning either option "x" can be passed as an option, but option "y" would not be allowed or vice-versa (with one possibly being the default).
A real-world example might be: script.php --read myfile.txt or script.php --write myfile.txt "test data" The idea is that both options cannot coexist ahd either an Exception would be thrown or it would default to one. It could even be expanded to multiple arguments or operands.

Any thoughts?

Hi,

I am trying to use your library in a new project and have used it successfully before. This time around, I am getting an obscure error In GetOpt.php line 288: "Illegal offset type in isset or empty"

As you can tell, this error is useless. I even tried debugging your code, and well, lets just say that its a good thing that you maintain this thing. For Example, WTF? Seriously.

    public function process(GetOpt $getopt, callable $setOption, callable $setCommand, callable $addOperand)
    {
        while (($arg = array_shift($this->arguments)) !== null) {
            if ($this->isMeta($arg)) {
                // everything from here are operands
                foreach ($this->arguments as $argument) {
                    $addOperand($argument);
                }
                break;
            }

            if ($this->isValue($arg)) {
                $operands = $getopt->getOperands();

                if (empty($operands) && $command = $getopt->getCommand($arg)) {
                    $setCommand($command);
                } else {
                    $addOperand($arg);
                }
            }

            if ($this->isLongOption($arg)) {
                $setOption($this->longName($arg), function (Option $option = null) use ($arg) {
                    return $this->value($arg, null, $option);
                });
                continue;
            }

            // the only left is short options
            foreach ($this->shortNames($arg) as $name) {
                $requestedValue = false;
                $setOption($name, function (Option $option = null) use ($arg, $name, &$requestedValue) {
                    $requestedValue = true;
                    return $this->value($arg, $name, $option);
                });

                if ($requestedValue) {
                    // when there is a value it was the last option
                    break;
                }
            }
        }
        return true;
    }

This function is an incomprehensible mess. There is a reason things like continue & break are discouraged in just about every language along with multiple return statements and goto. I am not going to tell you how to style your code, but please ensure it produces a usable error when something goes wrong so others don't have to look at it.

Best Wishes.

GetOpt doesn't properly support option-arguments that begin with a hyphen. Specifically, it only handles them as expected when they're part of the same word/argument as the option itself:

% composer require -q ulrichsg/getopt-php

% cat test.php
<?php

require_once __DIR__ . '/vendor/autoload.php';

$getopt = new \GetOpt\GetOpt([
  (new \GetOpt\Option('f', 'foo', \GetOpt\GetOpt::REQUIRED_ARGUMENT)),
]);

try {
  $getopt->parse();
  echo $getopt->getOption('foo'), "\n";
} catch ( \Exception $e ) {
  echo $e->getMessage(), "\n";
}

% php test.php -f-1
-1
% php test.php -f -1
Option 'foo' must have a value

% php test.php --foo=-1
-1
% php test.php --foo -1
Option 'foo' must have a value

Not sure if this is intended to catch mistakes or if it's just an oversight, but it's contrary to the way POSIX/GNU-style argument parsers normally work, and it breaks many common use cases like negative numbers ~~and - as a short-hand for stdin/stdout~~. (Edit: Actually it does handle - correctly.)

If it is deliberate, could there be an option to disable it?

Hi,

According to https://secure.php.net/supported-versions.php and https://secure.php.net/eol.php PHP 5.4 is dead since 2 years and 7 months and 5.5 since 1 year, 8 months.

Concerning 5.6 and 7.0, they only get security fixes and until the end of this year only.

At the same time, PHPUnit 4.8 is no longer supported, as for PHPUnit 5 for 2 months. PHPUnit 6, which is the oldest version supported (for 1 year from now) only supports PHP >= 7.0

Maybe it could be worth considering dropping PHP 5.x versions and upgrading dependencies...? A branch for a future version 4 release could be started, which would support PHP >= 7.1 (with PHPUnit 7 and PHP CS 3.2) ? When possible, features could be backported to 3.x (for PHP 5.4 - 5.6), which branch would still be supported for a while, even when 4.0 will be released.

What do you think?

Note: I have some free time...

Throwing exceptions immediately when an argument is unexpected or missing when processing arguments list prevent from using valuable options like --help or --verbose, as they have not been processed and therefore are not available when handling the exception.

To observe, just take the example file provided in the documentation and just make this change:

    try {
        $getOpt->process();
    } catch (unexpected | Missing $exception) {
        // catch missing exceptions if help is requested
        if (!$getOpt->getOption('help')) {
            throw $exception;
        } else {
            echo 'Help option provided'.PHP_EOL;
        }
    }

Now call the script with: php example.php -p --help The exception will always be thrown and the text Help option provided will not be displayed, despite we provided the help option.

Note that this does not happen if we pass the help option first: php example.php --help -p

To prevent this from happening, exceptions should be stored somewhere (Symfony uses an ExceptionBag if I'm correct). After processing the arguments list is done, the exceptions can then be checked and handled appropriately. This allows to use options to adjust error handling (verbose mode for example).

I may provide with some PR if you're interested, to have something concrete to discuss.