[WIP] tab completion

[Markcial]

Added files matcher, added parameters for the commands, fixed couple of bugs

TODO :

• Basic completion support
• Add a huge set of tests for all the matchers
• Add reflection with parameter completion for dynamic or static class matchers, methods, closures, etc.

[bobthecow]

This is now addTabCompletionMatchers for consistency with existing things, and consistency with what the words mean :)

[Markcial]

Sorry my bad. I tried to add matchers via config and threw an error. I will double check the calls and maybe a test for that.

[bobthecow]

No worries. I'll never say no to more test coverage.

[bobthecow]

Actually, do you mind splitting this commit? I'm not entirely sure what the fixes are, so it'd be helpful to have a commit (or commits) just for them, with a descriptive commit message.

[Markcial]

sure, no problem. Maybe today you will have the patch and the upgrades on a separate commits.

[bobthecow]

Let's leave off the dynamic stuff, and get this first set of fixes and improvements merged?

[Markcial]

Are you able to merge until certain commit? I would like to have the PR open so i can do more commits whenever possible, i have a busy week, but maybe for the next week i will be able to solve some of the TODOs still not done.

[bobthecow]

Yeah, I can do that. Is there a specific point where I should merge to? Do you feel like the things that are here are ready?

[Markcial]

If you don't mind use this commit as merge point Markcial@442e86b the other one after has a still premature version of a file matcher, i am not convinced yet about pushing it to master, maybe a little bit more later, thanks.

[bobthecow]

Sounds good. Commits up to 442e86b has been merged into develop.

[dlussky]

Don't know where to ask, maybe here is the best place)
What is the point of using constants with token names in AbstractMatcher.php?
You are still using it mostly as method arguments, so why don't you rely on original tokens?

self::tokenIs($prevToken, T_NEW)
// is cleaner than
self::tokenIs($prevToken, self::T_NEW)

[Markcial]

Is a work in progress, the code in this feature heavily relies on code subject to change, it will possibly not be the same after some more code iterations.

Anyways, i will take note about the suggestion and try to remember the reason why i did the token matcher this way, maybe it was not necessary and can be refactored as you are saying, i am currently unable to explain it, i will need some time to grasp again the old code and keep adding more support for some more missing features.

Once said this, thanks.

[dlussky]

Thank for the reply, Markcial!) No pressure, it was just curiosity, because I was going to write custom Matcher and was confused about the way you are using tokens, so I thought that there was something I didn't see)

[Markcial]

Just give me a couple of days and i will be able to help, maybe even i will be able to do a little bit of docs for the matchers. I am currently a little bit busy with my main work, but i expect to get my open sourced tickets done somewhere in the current and next month.

As far as i can remember, i did the matchers this way because some php versions use some variables, and another ones are possibly missing in another PHP versions, one clear example is the nekutodallin token, that has been renamed to double semicolon or something like that, so if you "hardcode" the token you will have a little bit more consistency, but that is a thing that might need more testing between versioning or even a better solution.

Attached there is a sample between how the constant names changes between php versions.

Anyways, if you get along with a better solution i will fully embrace it, of course :)

[dlussky]

Oh! I see, didn't know they really changed numeric values behind that tokens, but I don't see why you need to hardcode any numeric or string values. For example:

//before:
    self::tokenIs($firstToken, self::T_STRING)
//...
    public static function tokenIs($token, $which){
        return token_name($token[0]) === $which; }

//after:
    self::tokenIs($firstToken, T_STRING)
//...
    public static function tokenIs($token, $which){
        return $token[0] === $which;}

On different versions both $token[0] and the value behind T_STRING constant will be syncronized, as i understand.

[bobthecow]

I don't really have a preference WRT tokens. If the token constants (not the constant values, but the names themselves) are consistent and backwards compatible from 5.3 to 7.0, we probably can just use those ones, no?

[dlussky]

As i understand, all constants from 5.3 is preserved, only aliases were added (like T_PAAMAYIM_NEKUDOTAYIM => T_DOUBLE_COLON) and values behind constants were changed.
*also, new tokens were added, corresponding to the new features added in 5.4-5.6

[Markcial]

some constants are only from newer versions for example the variadic token, the yield token etc...

But anyways it should be handled gracefully even without the constants, so the warning must be caught.

Well @dlussky , to test if it works without storing the variables, simply replace all the self::{TOKEN_NAME} by token_name({TOKEN_NAME}) and if all the tests pass, then it is ok edit or with the changes you proposed too.

If you need it quick, you can do the tests by your own, if you can wait a week or so, i will do it by myself, whatever you choose it is ok

[dlussky]

@Markcial, It is just a minor aesthetic issue, so don't worry, I will look into it this weekend and send you a PR if there will be a solution)

[theofidry]

Fix for #292 I guess; @Markcial any update on that one?

[Markcial]

I am waiting for newer features like public eval api.

@dlussky any successful result using constants?

[theofidry]

@Markcial is there any plan to get them soon? Just asking as this issue has been pending for over a year.

In the meantime if you have some workarounds... It's tedious to have to type the FQCN each time...

[neronmoon]

Any progress?