new PHP Parser
niko.sams at gmail.com
Sun Jun 8 16:09:28 UTC 2008
Thanks for the first review - i'm glad you like it.
> /me looks forward to do Drupal development on KDevelop4
> with PHP autocompletion :)
yep - that would be cool!
(though still a long way to go)
>> PAAMAYIM_NEKUDOTAYIM ("::")
> Er, pretty strange name for a token?
It's Hebrew and used in PHP - see
A bit hard to work with (I copy&paste the name all the time)
>> IS_EQUAL ("=="), (...), EQUAL ("=")
> Might just be personal taste, but I think it would be good for readability to
> make those two a *bit* more easier to tell apart. Like, renaming the
> assignment operators from EQUAL to ASSIGN.
Changed as you suggested.
>> parameter=functionCallParameterListElement @ COMMA | 0
>> -> functionCallParameterList ;;
> (and similar rules)
> I found it a good idea to not have any rules that may be empty (except for
> pseudo data processing rules), as that makes stuff more predictable and
> slightly easier to debug. Most importantly, it prevents you from having the
> optional zero both in the caller rule and in the called rule.
you mean I should remove the "| 0" from the rule, for example:
#parameters=functionCallParameterListElement @ COMMA ->
LPAREN (parameterList=functionCallParameterList | 0) RPAREN -> foo;
#parameters=functionCallParameterListElement @ COMMA | 0 ->
LPAREN parameterList=functionCallParameterList RPAREN -> foo;
> Also, "parameter" should be a list here ("#parameter") if I get that right.
>> ParserState _MState;
> Maybe I'm missing something, but what's the point of having those variables
> duplicated? I think you should either drop _MState or the two separate ones.
> Please do also care for consistency - if you keep _MState, it should probably
> be named m_state like the other member variables that you defined.
(I do now understand how this works :D)
> All minor nitpicking, of course. I guess I could go on with pointless style
> issues all day, but let's instead just enjoy a fine new parser :D
I'm always happy about constructive criticism!
More information about the KDevelop-devel