Hi all,
I'd like to propose adding a PREG_THROW_ON_ERROR flag to the preg_*()
functions, and gauge interest in that.
Json and ext/filter both grew an opt-in way to turn a silent error into
an exception...that is JSON_THROW_ON_ERROR and FILTER_THROW_ON_FAILURE.
PCRE is the one left out where a failing call is easy to miss. An execution
error for example (e.g. a bad UTF-8) surfaces only if
you call preg_last_error() afterwards, and a malformed pattern emits a
warning and returns false.
PREG_THROW_ON_ERROR would do for PCRE what those two do for their
functions.
Passing it to any preg_*() call makes a PCRE error throw a
Pcre\PcreException that carries the PREG_*_ERROR code and the
preg_last_error_msg() text
instead of warning or returning false/null.
And it covers both kinds: a compilation error (e.g. a malformed pattern)
and an execution error, so opting in means any PCRE failure becomes a
single catchable exception.
Also, preg_replace() and preg_filter() gain an optional $flags
parameter to accept it (the only two without one as far as I know).
Concretely, the check you'd write today:
if (preg_match($pattern, $subject, $m) === false) {
throw new RuntimeException(preg_last_error_msg());
}
...collapses to:
preg_match($pattern, $subject, $m, PREG_THROW_ON_ERROR);
First, I'd like to know whether there's interest at all and whether anyone
sees a reason not to add it.
If there is interest, a couple of design questions I'd want the list's read
on:
- Array subjcets. Currently
preg_last_error()reflects only the last
element processed, but I'd have the flag throw on the first failing
element rather than keep last-one-wins...does that seem right? - Whether
*_ON_ERRORreads better than*_ON_FAILUREgiven the
existingpreg_last_error()/PREG_*_ERRORvocabulary.
If it seems worth pursuing, I'll write it up as a proper RFC.
Thanks,
Osama
Hi all,
I'd like to propose adding a
PREG_THROW_ON_ERRORflag to thepreg_*()
functions, and gauge interest in that.Json and
ext/filterboth grew an opt-in way to turn a silent error into
an exception...that isJSON_THROW_ON_ERRORandFILTER_THROW_ON_FAILURE.
PCRE is the one left out where a failing call is easy to miss. An
execution error for example (e.g. a bad UTF-8) surfaces only if
you callpreg_last_error()afterwards, and a malformed pattern emits a
warning and returnsfalse.
PREG_THROW_ON_ERRORwould do for PCRE what those two do for their
functions.
Passing it to anypreg_*()call makes a PCRE error throw a
Pcre\PcreExceptionthat carries thePREG_*_ERRORcode and the
preg_last_error_msg()text
instead of warning or returningfalse/null.
And it covers both kinds: a compilation error (e.g. a malformed pattern)
and an execution error, so opting in means any PCRE failure becomes a
single catchable exception.
Also,preg_replace()andpreg_filter()gain an optional$flags
parameter to accept it (the only two without one as far as I know).Concretely, the check you'd write today:
if (preg_match($pattern, $subject, $m) === false) { throw new RuntimeException(preg_last_error_msg()); }...collapses to:
preg_match($pattern, $subject, $m, PREG_THROW_ON_ERROR);First, I'd like to know whether there's interest at all and whether anyone
sees a reason not to add it.If there is interest, a couple of design questions I'd want the list's
read on:
- Array subjcets. Currently
preg_last_error()reflects only the
last element processed, but I'd have the flag throw on the first
failing element rather than keep last-one-wins...does that seem right?- Whether
*_ON_ERRORreads better than*_ON_FAILUREgiven the
existingpreg_last_error()/PREG_*_ERRORvocabulary.If it seems worth pursuing, I'll write it up as a proper RFC.
Thanks,
Osama
Hi all,
I've put together a complete implementation across all the preg_*()
functions so I can open a PR if it helps the discussion.
For now I've got it throwing on the first failing element for arrays, and
went with PREG_THROW_ON_ERROR for the flag name, just so I could complete
the implementation.
Still eager to hear what people have to say about that though, and to know
whether there's interest/objection before I take it further.
Thanks,
Osama
Hi
I'd like to propose adding a
PREG_THROW_ON_ERRORflag to the
preg_*()
functions, and gauge interest in that.
I personally hate this kind of flag, because of its opt-in nature. But
given the precedent and unless and until we rebuild the regex API in a
clean and modern way, it makes sense to me as a “stop-gap” solution that
should allow to get rid of some custom userland code that wraps pcre
into explicit checks.
Passing it to any
preg_*()call makes a PCRE error throw a
Pcre\PcreExceptionthat carries thePREG_*_ERRORcode and the
preg_last_error_msg()text
Pcre\PcreException would technically be fully in line with the naming
and Throwable policy, by including the extension name as the prefix.
However the existing PCRE functions use preg_ as a prefix, it will
probably be confusing to have the two different prefixes here. Given
that, I would suggest going with an unnamespaced \PregException for now
and then only introduce a namespace when actually building a new API to
not paint us into a corner already.
The naming policy specifically allows for that:
When adding new symbols to existing extensions it is RECOMMENDED to be
consistent with existing symbols, rather than to follow the namespacing
guidelines.
and
Newly introduced extensions MUST follow the following rules, existing
extensions SHOULD follow the rules for newly introduced exceptions, but
MAY diverge for consistency with existing symbols.
- Whether
*_ON_ERRORreads better than*_ON_FAILUREgiven the
existingpreg_last_error()/PREG_*_ERRORvocabulary.
I would go with ERROR for the reasons you mentioned there.
Best regards
Tim Düsterhus
Hi Tim,
Pcre\PcreException would technically be fully in line with the naming
and Throwable policy, by including the extension name as the prefix.
However the existing PCRE functions usepreg_as a prefix, it will
probably be confusing to have the two different prefixes here. Given
that, I would suggest going with an unnamespaced \PregException for now
and then only introduce a namespace when actually building a new API to
not paint us into a corner already.The naming policy specifically allows for that:
When adding new symbols to existing extensions it is RECOMMENDED to be
consistent with existing symbols, rather than to follow the namespacing
guidelines.and
Newly introduced extensions MUST follow the following rules, existing
extensions SHOULD follow the rules for newly introduced exceptions, but
MAY diverge for consistency with existing symbols.
I'd missed that entirely...thanks for the pointer!
And I think your reasoning about \PregException makes sense, I'll
switch to that.
Thanks,
Osama