Hi Internals,
I've submitted a small PR proposing a new PCRE function that
returns a human-friendly string representation of the last error.
https://github.com/php/php-src/pull/5185
Currently there's only preg_last_error() which returns error codes,
which isn't really helpful. Most comments in the documentation are
about converting those to something more user friendly.
https://www.php.net/preg_last_error#usernotes
Besides, most extensions provide multiple functions for both
use-cases natively, such as:
- json (json_last_error / json_last_error_msg)
- socket (socket_last_error / socket_strerror)
- sqlite (sqlite_last_error / sqlite_error_string)
- ldap (ldap_errno / ldap_error)
- etc...
So I think it makes sense for this function to exist.
Any thoughts?
Thank you!
Maybe you can set all this messages as lowercase? That way we can use it
more easily. If we need the first letter in capital letters we can use
ucfirst(), because the opposite is more complicated (a strtolower()
would "break" the "JIT" message, for example).
Atenciosamente,
David Rodrigues
Em qua., 19 de fev. de 2020 às 11:37, Nico Oelgart nicoswd@gmail.com
escreveu:
Hi Internals,
I've submitted a small PR proposing a new PCRE function that
returns a human-friendly string representation of the last error.https://github.com/php/php-src/pull/5185
Currently there's only
preg_last_error()which returns error codes,
which isn't really helpful. Most comments in the documentation are
about converting those to something more user friendly.https://www.php.net/preg_last_error#usernotes
Besides, most extensions provide multiple functions for both
use-cases natively, such as:
- json (json_last_error / json_last_error_msg)
- socket (socket_last_error / socket_strerror)
- sqlite (sqlite_last_error / sqlite_error_string)
- ldap (ldap_errno / ldap_error)
- etc...
So I think it makes sense for this function to exist.
Any thoughts?
Thank you!
On Wed, Feb 19, 2020 at 4:20 PM David Rodrigues david.proweb@gmail.com
wrote:
Maybe you can set all this messages as lowercase? That way we can use it
more easily. If we need the first letter in capital letters we can use
ucfirst(), because the opposite is more complicated (astrtolower()
would "break" the "JIT" message, for example).
Hi David!
I'm not sure how this would be more useful, plus all other built-in
functions like
json_last_error_msg() don't return all-lowercase strings either.
I'd prefer it to be consistent with the rest of functions.
On Wed, Feb 19, 2020 at 4:20 PM David Rodrigues david.proweb@gmail.com
wrote:Maybe you can set all this messages as lowercase? That way we can use it
more easily. If we need the first letter in capital letters we can use
ucfirst(), because the opposite is more complicated (astrtolower()
would "break" the "JIT" message, for example).Hi David!
I'm not sure how this would be more useful, plus all other built-in
functions like
json_last_error_msg()don't return all-lowercase strings either.I'd prefer it to be consistent with the rest of functions.
I agree with David. Lowercase is best.
I am however influenced by Go which specifies that all error messages should be lowercased for the exact reason that David suggests.
Consistency is nice, but I would argue it is not great when it means not being able to adopt emerging better practices.
#jmtcw
-Mike
On Wed, Feb 19, 2020 at 4:20 PM David Rodrigues david.proweb@gmail.com
wrote:Maybe you can set all this messages as lowercase? That way we can use it
more easily.
I am however influenced by Go which specifies that all error messages
should be lowercased for the exact reason that David suggests.
Like Nico, I'm not sure what lowercase is easier for. Can either of you
expand on the reasoning / use case?
Regards,
Rowan Tommins
[IMSoP]
On Wed, Feb 19, 2020 at 4:20 PM David Rodrigues david.proweb@gmail.com
wrote:Maybe you can set all this messages as lowercase? That way we can use it
more easily.I am however influenced by Go which specifies that all error messages
should be lowercased for the exact reason that David suggests.Like Nico, I'm not sure what lowercase is easier for. Can either of you
expand on the reasoning / use case?
From https://github.com/golang/go/wiki/CodeReviewComments#error-strings
"Error strings should not be capitalized (unless beginning with proper nouns or acronyms) or end with punctuation, since they are usually printed following other context. That is, use fmt.Errorf("something bad") not fmt.Errorf("Something bad"), so that log.Printf("Reading %s: %v", filename, err) formats without a spurious capital letter mid-message. This does not apply to logging, which is implicitly line-oriented and not combined inside other messages."
-Mike
On Feb 19, 2020, at 12:10 PM, Rowan Tommins rowan.collins@gmail.com
wrote:On Wed, Feb 19, 2020 at 4:20 PM David Rodrigues <
david.proweb@gmail.com>
wrote:Maybe you can set all this messages as lowercase? That way we can use
it
more easily.On Wed, 19 Feb 2020 at 16:50, Mike Schinkel mike@newclarity.net
wrote:I am however influenced by Go which specifies that all error messages
should be lowercased for the exact reason that David suggests.Like Nico, I'm not sure what lowercase is easier for. Can either of you
expand on the reasoning / use case?From https://github.com/golang/go/wiki/CodeReviewComments#error-strings
"Error strings should not be capitalized (unless beginning with proper
nouns or acronyms) or end with punctuation, since they are usually printed
following other context. That is, use fmt.Errorf("something bad") not
fmt.Errorf("Something bad"), so that log.Printf("Reading %s: %v", filename,
err) formats without a spurious capital letter mid-message. This does not
apply to logging, which is implicitly line-oriented and not combined inside
other messages."-Mike
But that's Golang's policy, so either we change all error messages to be
lower case or we keep it consistent with what PHP currently does.
Moreover, I'm not sure I see a massive benefit in having it in lowercase
(especially just one random function which behaves not like others)
Best regards
George P. Banyard
But that's Golang's policy,
The intent was not to present someone else's policy but to instead show prior art.
so either we change all error messages to be lower case or we keep it consistent with what PHP currently does.
Moreover, I'm not sure I see a massive benefit in having it in lowercase (especially just one random function which behaves not like others)
That is certainly one position to take.
Another position is to evolve and adopt newer practices rather than be fixed by to choices made in the past that do not address concerns that have been identified since the original choices were made.
And with that, that is all I will say on this topic. Take the outcome where you will.
-Mike
From https://github.com/golang/go/wiki/CodeReviewComments#error-strings
"Error strings should not be capitalized (unless beginning with proper
nouns or acronyms) or end with punctuation, since they are usually printed
following other context. That is, use fmt.Errorf("something bad") not
fmt.Errorf("Something bad"), so that log.Printf("Reading %s: %v", filename,
err) formats without a spurious capital letter mid-message. This does not
apply to logging, which is implicitly line-oriented and not combined inside
other messages."
Thanks, I can see the reasoning, although I'm not sure I agree with it.
If you're only showing it to a programmer, worrying about the grammar of a
capital letter after a colon seems needlessly pedantic. If you're showing
it to end-users, you should probably be thinking about i18n anyway, and can
load your own English translations (and user-friendly summaries) for
whatever context you want to show them in.
Regards,
Rowan Tommins
[IMSoP]
Thanks, I can see the reasoning, although I'm not sure I agree with it.
If you're only showing it to a programmer, worrying about the grammar of a
capital letter after a colon seems needlessly pedantic. If you're showing
it to end-users, you should probably be thinking about i18n anyway, and can
load your own English translations (and user-friendly summaries) for
whatever context you want to show them in.
Not for organizations that have made the decision not to increase the cost of development because they have made the business decision to only support English.
#justsaying
-Mike
On Wed, Feb 19, 2020 at 6:36 PM Rowan Tommins rowan.collins@gmail.com
wrote:
From https://github.com/golang/go/wiki/CodeReviewComments#error-strings
"Error strings should not be capitalized (unless beginning with proper
nouns or acronyms) or end with punctuation, since they are usually
printed
following other context. That is, use fmt.Errorf("something bad") not
fmt.Errorf("Something bad"), so that log.Printf("Reading %s: %v",
filename,
err) formats without a spurious capital letter mid-message. This does not
apply to logging, which is implicitly line-oriented and not combined
inside
other messages."Thanks, I can see the reasoning, although I'm not sure I agree with it.
If you're only showing it to a programmer, worrying about the grammar of a
capital letter after a colon seems needlessly pedantic. If you're showing
it to end-users, you should probably be thinking about i18n anyway, and can
load your own English translations (and user-friendly summaries) for
whatever context you want to show them in.
FWIW, it is our established stance that all error messages must be
capitalized. Lower-case first character is only permitted if it is part of
a function name, or similar cases.
Regards,
Nikita
FWIW, it is our established stance that all error messages must be
capitalized. Lower-case first character is only permitted if it is part of
a function name, or similar cases.
Fair enough.
-Mike
FWIW, it is our established stance that all error messages must be
capitalized. Lower-case first character is only permitted if it is part of
a function name, or similar cases.
Thanks for clearing this up, Nikita.
Given that, I think this lower vs uppercase debate should be a
different discussion held separately if there's real interest in
changing the current stance on this.
Besides that, any other thoughts on the PR?