Hi all,
As I have proposed previously in an old thread... What about we name all the
tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM, T_DOLLAR_OPEN_CURLY_BRACES in the messages etc)
Some examples:
$ sapi/cli/php -r 'function ""'
Patched:
Parse error: syntax error, unexpected quoted-string, expecting identifier or
'(' in Command line code on line 1
Current:
Parse error: syntax error, unexpected T_CONSTANT_ENCAPSED_STRING, expecting
T_STRING or '(' in Command line code on line 1
$ sapi/cli/php -r 'echo ::a;'
Patched:
Parse error: syntax error, unexpected :: in Command line code on line 1
Current:
Parse error: syntax error, unexpected T_PAAMAYIM_NEKUDOTAYIM in Command line
code on line 1
Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-message
Any thoughts?
Thanks.
--
Regards,
Felipe Pena
Hi all,
As I have proposed previously in an old thread... What about we name all
the
tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM,T_DOLLAR_OPEN_CURLY_BRACESin the messages etc)Some examples:
$ sapi/cli/php -r 'function ""'
Patched:
Parse error: syntax error, unexpected quoted-string, expecting identifier
or
'(' in Command line code on line 1Current:
Parse error: syntax error, unexpected T_CONSTANT_ENCAPSED_STRING, expecting
T_STRINGor '(' in Command line code on line 1$ sapi/cli/php -r 'echo ::a;'
Patched:
Parse error: syntax error, unexpected :: in Command line code on line 1Current:
Parse error: syntax error, unexpectedT_PAAMAYIM_NEKUDOTAYIMin Command
line
code on line 1Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-messageAny thoughts?
Thanks.
--
Regards,
Felipe Pena
+1
do you think that this could cause some kind of BC?
I don't think that anybody out there parses the tokens as is, but there are
crazy people. :)
the other thing that I can imagine that the developers will be surprised
that they are getting meaningful error messages, but I think they can live
with that. :)
Tyrael
Hi,
2011/5/16 Ferenc Kovacs info@tyrael.hu
+1
do you think that this could cause some kind of BC?
I don't think that anybody out there parses the tokens as is, but there are
crazy people. :)
the other thing that I can imagine that the developers will be surprised
that they are getting meaningful error messages, but I think they can live
with that. :)Tyrael
There is no BC. The changes doesn't affects token_*() functions for example.
$ sapi/cli/php -r 'var_dump(token_name(318));'
string(6) "T_ECHO"
I added such information to the wiki page. Thanks. :)
--
Regards,
Felipe Pena
Hi Felipe,
Hi all,
As I have proposed previously in an old thread... What about we name all the
tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM,T_DOLLAR_OPEN_CURLY_BRACESin the messages etc)Some examples:
$ sapi/cli/php -r 'function ""'
Patched:
Parse error: syntax error, unexpected quoted-string, expecting identifier or
'(' in Command line code on line 1Current:
Parse error: syntax error, unexpected T_CONSTANT_ENCAPSED_STRING, expecting
T_STRINGor '(' in Command line code on line 1$ sapi/cli/php -r 'echo ::a;'
Patched:
Parse error: syntax error, unexpected :: in Command line code on line 1Current:
Parse error: syntax error, unexpectedT_PAAMAYIM_NEKUDOTAYIMin Command line
code on line 1Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-messageAny thoughts?
That's great, it will save millions of headaches.
Best,
Thanks.
--
Regards,
Felipe Pena
+1
Regards
Pierrick
Hi all,
As I have proposed previously in an old thread... What about we name all
the
tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM,T_DOLLAR_OPEN_CURLY_BRACESin the messages etc)Some examples:
$ sapi/cli/php -r 'function ""'
Patched:
Parse error: syntax error, unexpected quoted-string, expecting identifier
or
'(' in Command line code on line 1Current:
Parse error: syntax error, unexpected T_CONSTANT_ENCAPSED_STRING, expecting
T_STRINGor '(' in Command line code on line 1$ sapi/cli/php -r 'echo ::a;'
Patched:
Parse error: syntax error, unexpected :: in Command line code on line 1Current:
Parse error: syntax error, unexpectedT_PAAMAYIM_NEKUDOTAYIMin Command
line
code on line 1Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-messageAny thoughts?
Thanks.
--
Regards,
Felipe Pena
Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-message
Just for completeness, here's the lengthy thread
we had in October 2010 about this:
http://news.php.net/php.internals/49978
FWIW, I like the Easter Egg :)
Greetings,
Florian
I remember error messages have been improved since last decade, but
they were engine ones.
Parser ones have never been touched, so to improve them, I +1 your
idea Felipe :)
Julien
Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-messageJust for completeness, here's the lengthy thread
we had in October 2010 about this:http://news.php.net/php.internals/49978
FWIW, I like the Easter Egg :)
Greetings,
Florian
2011/5/16 Felipe Pena felipensp@gmail.com
Hi all,
As I have proposed previously in an old thread... What about we name all
the tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM,T_DOLLAR_OPEN_CURLY_BRACESin the messages etc)
[...]
Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-message
Any thoughts?
Thanks.
So... In case of no objections, I'll commit it in trunk and 5_4 branch, Is
it okay? :)
--
Regards,
Felipe Pena
Hi Felipe,
Read the archives. We discussed T_PAAMAYIM_NEKUDOTAYIM extensively and agreed to keep it as a historical landmark. Let's not have the discussion yet again within the same year. Google it just to see the # of mentions.
What we could do as a compromise between history and improving many of the parse error messages is:
Parse error: syntax error, unexpected :: (T_PAAMAYIM_NEKUDOTAYIM) in Command line code on line 1
Generally speaking I'm OK with the other changes but I don't know if any extensions or scripts depend on them (probably not).
I suggest to give it another couple of days to see if anyone has any valid concerns on breakage this could cause.
Andi
-----Original Message-----
From: Felipe Pena [mailto:felipensp@gmail.com]
Sent: Tuesday, May 17, 2011 7:18 AM
To: internals
Subject: [PHP-DEV] Re: [RFC] Improved parser error message2011/5/16 Felipe Pena felipensp@gmail.com
Hi all,
As I have proposed previously in an old thread... What about we name
all the tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM,T_DOLLAR_OPEN_CURLY_BRACESin the
messages
etc)[...]
Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-message
Any thoughts?
Thanks.
So... In case of no objections, I'll commit it in trunk and 5_4 branch, Is it okay? :)
--
Regards,
Felipe Pena
Em Tue, 17 May 2011 17:01:26 +0100, Andi Gutmans andi@zend.com escreveu:
Read the archives. We discussed
T_PAAMAYIM_NEKUDOTAYIMextensively and
agreed to keep it as a historical landmark. Let's not have the
discussion yet again within the same year. Google it just to see the #
of mentions.What we could do as a compromise between history and improving many of
the parse error messages is:
Parse error: syntax error, unexpected :: (T_PAAMAYIM_NEKUDOTAYIM) in
Command line code on line 1
This proposal is not about changing the name of the token, only the
message that's shown to the user. The token name will remain as is in
zend_language_parser.y et al. If I recall correctly, there was not
objection raised to changing the message in the last discussion.
Anyway, +1 to the change from you.
--
Gustavo Lopes
Em Tue, 17 May 2011 17:07:09 +0100, Gustavo Lopes glopes@nebm.ist.utl.pt
escreveu:
[...]
Anyway, +1 to the change from you.
Sorry, it should read "from me".
--
Gustavo Lopes
Hi Felipe,
Read the archives. We discussed
T_PAAMAYIM_NEKUDOTAYIMextensively and
agreed to keep it as a historical landmark. Let's not have the discussion
yet again within the same year. Google it just to see the # of mentions.
Maybe you don't remember, but Felipe participated in that discussion, to be
exact he proposed this exact feature there:
internals@lists.php.net/msg48173.html" rel="nofollow" target="_blank">http://www.mail-archive.com/internals@lists.php.net/msg48173.html
after Felipe's mail, nobody objected about replacing the constants with
meaningful names, as it was the original idea, but nobody found a clean way
to implement it before.
What we could do as a compromise between history and improving many of the
parse error messages is:
Parse error: syntax error, unexpected :: (T_PAAMAYIM_NEKUDOTAYIM) in
Command line code on line 1
maybe it's a little bit redundant, but it can be a good transition.
Generally speaking I'm OK with the other changes but I don't know if any
extensions or scripts depend on them (probably not).
I suggest to give it another couple of days to see if anyone has any valid
concerns on breakage this could cause.
as Felipe mentioned, his patch doesn't modify the token names(so the token_
functions aren't affected by this change), only the rendering of the error
messages.
Tyrael
2011/5/16 Felipe Pena felipensp@gmail.com
Hi all,
As I have proposed previously in an old thread... What about we name all
the tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM,T_DOLLAR_OPEN_CURLY_BRACESin the messages etc)[...]
Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-message
Any thoughts?
Thanks.
So... In case of no objections, I'll commit it in trunk and 5_4 branch, Is
it okay? :)
Go for it.
Derick
+1 with Andi's improvement.
Gustavo - I realize it's not about changing the token's name, but nobody (=very few) looks at the token names. The point is to keep the tradition of also exposing this specific token name to the user, but still making it clear that what was expected was :: - without the need to head to Google Translate...
Zeev
-----Original Message-----
From: Felipe Pena [mailto:felipensp@gmail.com]
Sent: Tuesday, May 17, 2011 5:18 PM
To: internals
Subject: [PHP-DEV] Re: [RFC] Improved parser error message2011/5/16 Felipe Pena felipensp@gmail.com
Hi all,
As I have proposed previously in an old thread... What about we name
all the tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM,T_DOLLAR_OPEN_CURLY_BRACESin the
messages
etc)[...]
Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-message
Any thoughts?
Thanks.
So... In case of no objections, I'll commit it in trunk and 5_4 branch, Is it okay?
:)--
Regards,
Felipe Pena
Hi!
I think we need to keep token name in the message, since it makes it
easier to understand what parser expected if you need to debug the
parser (as opposed to your code). So I think we need to have both the
human-readable name and the token name, as Andi suggested.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
On Tue, May 17, 2011 at 6:49 PM, Stas Malyshev smalyshev@sugarcrm.comwrote:
Hi!
I think we need to keep token name in the message, since it makes it easier
to understand what parser expected if you need to debug the parser (as
opposed to your code). So I think we need to have both the human-readable
name and the token name, as Andi suggested.
on the sidenote, I don't think that many people out there would start
debuging the php src if they are presented with an error by the engine.
but I'm fine with Andi's suggestion.
Tyrael
On Tue, May 17, 2011 at 6:49 PM, Stas Malyshevsmalyshev@sugarcrm.comwrote:
Hi!
I think we need to keep token name in the message, since it makes it easier
to understand what parser expected if you need to debug the parser (as
opposed to your code). So I think we need to have both the human-readable
name and the token name, as Andi suggested.on the sidenote, I don't think that many people out there would start
debuging the php src if they are presented with an error by the engine.
but I'm fine with Andi's suggestion.
While that is true, I did run across some code a while back that used
the tokenizer extension in conjunction with the error messages. If we
decouple the error messages completely from the tokens there would be no
way to write that same code. An edge case, but I don't think it hurts to
keep the token name there.
-Rasmus
Em Tue, 17 May 2011 17:49:53 +0100, Stas Malyshev smalyshev@sugarcrm.com
escreveu:
I think we need to keep token name in the message, since it makes it
easier to understand what parser expected if you need to debug the
parser (as opposed to your code). So I think we need to have both the
human-readable name and the token name, as Andi suggested.
I think this alternative has very little value for debugging the parser.
There are no repeated labels for the tokens, so even if you don't know the
token name by heart, you can look it up in 10 seconds.
Keeping the token name will only perpetuate the confusion it has caused. I
think it ought to be dropped, but if no consensus can be reached, this
would be better than the status quo.
--
Gustavo Lopes
2011/5/18 Gustavo Lopes glopes@nebm.ist.utl.pt:
Em Tue, 17 May 2011 17:49:53 +0100, Stas Malyshev smalyshev@sugarcrm.com
escreveu:I think we need to keep token name in the message, since it makes it
easier to understand what parser expected if you need to debug the parser
(as opposed to your code). So I think we need to have both the
human-readable name and the token name, as Andi suggested.I think this alternative has very little value for debugging the parser.
There are no repeated labels for the tokens, so even if you don't know the
token name by heart, you can look it up in 10 seconds.Keeping the token name will only perpetuate the confusion it has caused. I
think it ought to be dropped, but if no consensus can be reached, this would
be better than the status quo.--
Gustavo Lopes--
So, following some suggestion I made a patch wich displays such messages:
$ sapi/cli/php -r 'class '
Current:
Parse error: syntax error, unexpected $end, expecting T_STRING
Patched:
Parse error: syntax error, unexpected end of file, expecting
'identifier' (T_STRING)
$ sapi/cli/php -r 'class abc foo'
Current:
Parse error: syntax error, unexpected T_STRING, expecting '{' in
Command line code on line 1
Patched:
Parse error: syntax error, unexpected 'foo' (T_STRING), expecting '{'
in Command line code on line 1
As can be noticed, I added the actual scanned string in the
"unexpected" part. This might be useful for finding really which makes
the parser error. (It was a bit tricky though :D)
Any thoughts?
https://wiki.php.net/rfc/improved-parser-error-message
--
Regards,
Felipe Pena
Thanks for the great work. More should be done on the front of helping
newcomers solve trivial issues imo.
$ sapi/cli/php -r 'class '
Current:
Parse error: syntax error, unexpected $end, expectingT_STRINGPatched:
Parse error: syntax error, unexpected end of file, expecting
'identifier' (T_STRING)
identifier should not be quoted. The quotes are fine for "expecting '{'"
and "unexpected 'foo'" because those are text literals, but this implies
you were supposed to actually type "class identifier", which is
obviously not the intended message.
$ sapi/cli/php -r 'class abc foo'
Current:
Parse error: syntax error, unexpected T_STRING, expecting '{' in
Command line code on line 1Patched:
Parse error: syntax error, unexpected 'foo' (T_STRING), expecting '{'
in Command line code on line 1As can be noticed, I added the actual scanned string in the
"unexpected" part. This might be useful for finding really which makes
the parser error. (It was a bit tricky though :D)
Good stuff, maybe "unexpected string 'foo' (T_STRING)" or "unexpected
string (T_STRING) 'foo'" would be more clear, but no big deal really.
Cheers
--
Jordi Boggiano
@seldaek - http://nelm.io/jordi
Thanks for the great work. More should be done on the front of helping
newcomers solve trivial issues imo.$ sapi/cli/php -r 'class '
Current:
Parse error: syntax error, unexpected $end, expectingT_STRINGPatched:
Parse error: syntax error, unexpected end of file, expecting
'identifier' (T_STRING)identifier should not be quoted. The quotes are fine for "expecting '{'"
and "unexpected 'foo'" because those are text literals, but this implies
you were supposed to actually type "class identifier", which is
obviously not the intended message.$ sapi/cli/php -r 'class abc foo'
Current:
Parse error: syntax error, unexpected T_STRING, expecting '{' in
Command line code on line 1Patched:
Parse error: syntax error, unexpected 'foo' (T_STRING), expecting '{'
in Command line code on line 1As can be noticed, I added the actual scanned string in the
"unexpected" part. This might be useful for finding really which makes
the parser error. (It was a bit tricky though :D)Good stuff, maybe "unexpected string 'foo' (T_STRING)" or "unexpected
string (T_STRING) 'foo'" would be more clear, but no big deal really.Cheers
--
Jordi Boggiano
@seldaek - http://nelm.io/jordi--
Good work Felipe! the $end was very ambiguous :-)
Hi!
As can be noticed, I added the actual scanned string in the
"unexpected" part. This might be useful for finding really which makes
the parser error. (It was a bit tricky though :D)Any thoughts?
https://wiki.php.net/rfc/improved-parser-error-message
Looks cool. Any tests we'd need to fix for that? Except for that, I
think it's all good.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Patched:
Parse error: syntax error, unexpected end of file, expecting
'identifier' (T_STRING)
At least for developers who do not know how a compiler works, I think
that we should think about removing the information duplication of
"parse error" and "syntax error". Just "Syntax error: ..." should be
enough.
--
Sebastian Bergmann Co-Founder and Principal Consultant
http://sebastian-bergmann.de/ http://thePHP.cc/