Rename PhpToken::getAll()?

php.internals

Nikita Popov

5 years ago
Hi internals, Cross-posting from https://bugs.php.net/bug.php?id=80328 to a place where someone might see it... The PhpToken::getAll() API introduced in PHP 8.0 is of course inspired by the existing token_get_all() function. But arguably, the name is not that great. The reporter suggests to rename it into PhpToken::tokenize() instead, which seems like a sensible suggestion to me. Any thoughts on doing that rename? It's pretty late in the release cycle, but if we want to rename something, this is our last chance :) Regards, Nikita PS: Because I know someone is going to suggest it: No, we will not introduce a separate PhpLexer class at this point in time.

Hans Henrik Bergan

5 years ago
+1 for ::tokenize()

Sebastian Bergmann

5 years ago
Am 07.11.2020 um 10:09 schrieb Nikita Popov:
> The reporter suggests to rename it into PhpToken::tokenize() instead, which > seems like a sensible suggestion to me.
+1

Gabriel Caruso

5 years ago
On Sat, Nov 7, 2020, 10:10 Nikita Popov <nikita.ppv@gmail.com> wrote:
> Hi internals, > > Cross-posting from https://bugs.php.net/bug.php?id=80328 to a place where > someone might see it... > > The PhpToken::getAll() API introduced in PHP 8.0 is of course inspired by > the existing token_get_all() function. But arguably, the name is not that > great. > > The reporter suggests to rename it into PhpToken::tokenize() instead, which > seems like a sensible suggestion to me. > > Any thoughts on doing that rename? It's pretty late in the release cycle, > but if we want to rename something, this is our last chance :) > > Regards, > Nikita > > PS: Because I know someone is going to suggest it: No, we will not > introduce a separate PhpLexer class at this point in time. >
Agreed, and as one of PHP 8.0 RM, I see no problem on doing this renaming, just make sure to document it. Naming stuff is hard, but it's never too late to improve it :)

Sara Golemon

5 years ago
On Sat, Nov 7, 2020 at 3:09 AM Nikita Popov <nikita.ppv@gmail.com> wrote:
> The PhpToken::getAll() API introduced in PHP 8.0 is of course inspired by > the existing token_get_all() function. But arguably, the name is not that > great. > > The reporter suggests to rename it into PhpToken::tokenize() instead, which > seems like a sensible suggestion to me. > > Any thoughts on doing that rename? It's pretty late in the release cycle, > but if we want to rename something, this is our last chance :) > >
Do I love a rename 3 weeks from GA? No. Is there any good reason to object? Not really. The change is super minor and the functionality is new and as-yet undocumented. Go for it. The reasoning in favor is sound. -Sara