New PCRE function

  108665
February 19, 2020 14:36 nicoswd@gmail.com (Nico Oelgart)
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!
  108669
February 19, 2020 15:20 david.proweb@gmail.com (David Rodrigues)
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! >
  108671
February 19, 2020 15:33 nicoswd@gmail.com (Nico Oelgart)
On Wed, Feb 19, 2020 at 4:20 PM David Rodrigues 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 (a `strtolower()` > 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.
  108674
February 19, 2020 16:50 mike@newclarity.net (Mike Schinkel)
> On Feb 19, 2020, at 10:33 AM, Nico Oelgart <nicoswd@gmail.com> wrote: > > On Wed, Feb 19, 2020 at 4:20 PM David Rodrigues 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 (a `strtolower()` >> 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
  108677
February 19, 2020 17:10 rowan.collins@gmail.com (Rowan Tommins)
> > On Wed, Feb 19, 2020 at 4:20 PM David Rodrigues 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? Regards, -- Rowan Tommins [IMSoP]
  108678
February 19, 2020 17:15 mike@newclarity.net (Mike Schinkel)
> On Feb 19, 2020, at 12:10 PM, Rowan Tommins collins@gmail.com> wrote: > >>> On Wed, Feb 19, 2020 at 4:20 PM David Rodrigues 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
  108679
February 19, 2020 17:27 george.banyard@gmail.com ("G. P. B.")
On Wed, 19 Feb 2020 at 18:15, Mike Schinkel <mike@newclarity.net> wrote:

> > On Feb 19, 2020, at 12:10 PM, Rowan Tommins 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 > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php
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
  108684
February 19, 2020 18:03 mike@newclarity.net (Mike Schinkel)
> On Feb 19, 2020, at 12:27 PM, G. P. B. banyard@gmail.com> wrote: > > 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
  108680
February 19, 2020 17:35 rowan.collins@gmail.com (Rowan Tommins)
On Wed, 19 Feb 2020 at 17:15, Mike Schinkel <mike@newclarity.net> 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. Regards, -- Rowan Tommins [IMSoP]
  108682
February 19, 2020 17:52 mike@newclarity.net (Mike Schinkel)
> On Feb 19, 2020, at 12:35 PM, Rowan Tommins collins@gmail.com> wrote: > > 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
  108695
February 20, 2020 09:11 nikita.ppv@gmail.com (Nikita Popov)
On Wed, Feb 19, 2020 at 6:36 PM Rowan Tommins collins@gmail.com>
wrote:

> On Wed, 19 Feb 2020 at 17:15, Mike Schinkel <mike@newclarity.net> 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
  108709
February 21, 2020 00:49 mike@newclarity.net (Mike Schinkel)
> On Feb 20, 2020, at 4:11 AM, Nikita Popov ppv@gmail.com> wrote: > > 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
  108712
February 21, 2020 09:02 nicoswd@gmail.com (Nico Oelgart)
On Thu, Feb 20, 2020 at 10:12 AM Nikita Popov ppv@gmail.com> wrote:

> 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?