Comments (11)
interesting idea. but we would need to define somehow what happens when an option is unknown and what else could someone want to do when an option is unknown except showing the help?
from getopt-php.
What about having an ArgumentExceptionHandler
that user could extend or replace? If none is defined when time comes to handle the ExceptionBag
(or whatever we call it or use), then it uses a default one which shows the help text.
Or use a callback (a Closure or whatever the user wants to use) which receives the ExceptionBag
as argument. If none, show help.
from getopt-php.
Actually, there is no need to handle process errors: by default, exceptions are thrown, so that there is no BC break. If the user decides to not throw errors (I'm adding a $throwExceptions = true
parameter to method process()
), then they have to check whether there have been errors or not, just like they currently use a try/catch
block.
As illustrated in the example in the documentation, there are try/catch
blocks: this is not part of the library, it's in userland. I know that this way, if exceptions are stored instead of being immediately thrown and they are not checked later, it can cause an unexpected behavior: just like exceptions would be catched with no subsequent action (like showing help text).
So I'm just implementing this in a very simple way: when an error occurs (Unexpected or Missing only), send it to an error()
method, which will check whether the script should throw the exception or not. If not, store it.
I'm adding two public methods: hasErrors()
and getErrors()
.
Please, let me know if you think I'm doing the wrong way (I'll submit a PR soon, still need to write tests and adapt doc).
from getopt-php.
so the process method catches his own exceptions and checks an option if the exception should be thrown or just rememberd? sounds good
from getopt-php.
That's right.
Could look like this: https://github.com/Arcesilas/getopt-php/commit/ca2baca811cd43b0a95017777fe187607adedfba
from getopt-php.
It appears that exceptions may be thrown from Arguments::process()
, more precisely from Argument::setValue()
, when the value does not pass validation.
It would then require the Argument
(Operand
or Option
) to hold an instance of GetOpt
. It's easy with a factory, but requires many changes and possibly BC breaks.
Another solution is to use a static property and static public methods on GetOpt
. This way, it's possible to call the GetOpt::error()
method from within an Argument when assigning the value:
protected static $throwErrors = false;
protected static $processErrors = [];
public static function error(ArgumentException $exception)
{
// handle the error
}
The GetOpt::getErrors()
can remain non-static.
I don't really like static properties, generally, since they are stateless. But in this case, there should not be more than one instance of GetOpt
, and no more than one array of arguments is supposed to be processed at the same time.
I also wonder if a setter for $throwExceptions
is appropriate, so that both following are possible:
$getOpt = new GetOpt(null, false);
// or:
$getOpt = new GetOpt();
$getOpt->throwErrors(false);
from getopt-php.
https://github.com/Arcesilas/getopt-php/commit/ca2baca811cd43b0a95017777fe187607adedfba
looks good except some namings.
The comment about static methods I don't understand. There are only 3 static methods in the GetOpt class. There is no way to use the class statically - why we should remember the errors statically (global; over all GetOpt instances)?
Remember: the dev could create multiple getOpt parser that handle different scenarios. We had this in another issue: #142 (comment)
from getopt-php.
I would recommend to have two settings: SETTING_THROW_EXCEPTIONS
and SETTING_STOP_ON_ERROR
. When throwing exceptions stop on error is implicit enabled.
The exception from setValue has to be catched the usual way:
try {
$options->setValue($value);
return true;
} catch (ArgumentException $exception) {
$this->error($exception);
return false;
}
The setter closures in GetOpt\GetOpt::process()
need a boolean return value and the loop in GetOpt\Arguments::process()
needs something like this:
$stopOnError = $getOpt->get(GetOpt::SETTING_STOP_ON_ERROR);
$continue = true;
while (($arg = array_shift($this->arguments)) !== null) {
if ($stopOnError && !$continue) {
return false;
}
// ...
$continue = $setOption($option);
}
from getopt-php.
In what I had in mind, THROW_EXCEPTIONS
and STOP_ON_ERRORS
were exactly the same. I'm not sure it's relevant to stop on errors and not throw exceptions: it's exactly the same as using a try/catch
block but with more logic, which makes the code less readable, IMHO.
The problem I pointed out in this issue was that when an exception is thrown (which means "stop on error"), the remaining options and operands are not set. The goal of my proposition is to allow the remaining options and operands to be set, even if there are errors.
Also, I'm thinking about one question: isn't it inappropriate to not throw any exception at all when there are errors? In fact, the problem is not that an exception is raised, but that it's raised to soon.
Actually, I think the behaviour should be as so:
- process the arguments
- if any error happens, always store it
- when finishing process (i.e. the "last" line of
GetOpt::process()
method), throw an exception with all exceptions (an "aggregate" Exception which would hold all exceptions which have occured during process).
This would prevent the script from running with invalid / missing / unexpected arguments.
If the user wants to do nothing in the catch
block, that's their problem. It's not possible to forget something. Also, this way, no setting and the behaviour is not changed at all for older code (the "aggregate" exception would simply return the first exception message and code when using getMessage()
and getCode()
. We could access all exceptions with a getStack()
method, for example.
from getopt-php.
Actually, we can simply use Exception chaining.
from getopt-php.
Yes, stop on error without throwing exceptions is the same as catching a thrown exception. But some people might argue that the code looks very different:
$getOpt = createGetOpt();
$getOpt->process($argv);
$options = $getOpt->hasErrors() ? $defaultOptions : $getOpt->getOptions();
// vs.
try {
$getOpt = createGetOpt();
$getOpt->process($argv);
$options = $getOpt->getOptions();
} catch (ArgumentException $exception) {
$options = $defaultOptions;
}
If an exception is thrown to soon or to late depends on the length of arguments and the complexity to process them (validators that query a database for example). I wouldn't agree to a solution where it is not possible to stop processing when an error occurs.
The idea of not throwing immediately but after the processing with a "aggregate" exception is a 3rd method in my opinion. Maybe an option ERROR_HANDLING
with 3 versions: throw
, gather
, silent
.
The "aggregate" exception I would rather develop like this:
class ProcessingErrors extends ArgumentException
{
/** @var ArgumentException[] */
protected $errors = [];
public function __constructor($errors) {
$this->errors = $errors;
parent::__constructor('Errors occured during processing the arguments');
}
public function getErrors() {
return $errors;
}
}
from getopt-php.
Related Issues (20)
- Example is broken HOT 6
- Version 3.3 HOT 2
- Error Reporting is Atrocious HOT 7
- Add option to validate default value HOT 5
- Can help be a command? HOT 2
- Modifications needed for PHP 7.4 compliance HOT 1
- Option with missing REQUIRED_ARGUMENT consumes next option? HOT 2
- A lot of warnings in travis for newer php versions HOT 9
- Required Option HOT 1
- Cleaning up merged branches HOT 3
- Outdated CHANGELOG.md file HOT 3
- What are the version constraints? HOT 3
- Error on PHP 8.1.0 HOT 2
- Failed Operand validation should throw non-zero exit code HOT 2
- Replace gitlab-ci with a better integrated solution HOT 1
- Missing return type signature in PHP 8.1 (actuel version from packagist 4.2.0)
- Localization ca.php HOT 2
- How to setup arguments (or named operands)? HOT 1
- CommandInterface misses getHandler() method HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from getopt-php.