Make sure all internal lists do not rely on keywords order#117
Make sure all internal lists do not rely on keywords order#117greg0ire merged 2 commits intodoctrine:1.4.xfrom
Conversation
mvorisek
force-pushed
the
sort_kws_lists
branch
3 times, most recently
from
962aa45 to
951d50f
Compare
goetas
left a comment
goetas
left a comment
There was a problem hiding this comment.
Not a fan of it but is ok from my pow
greg0ire
left a comment
greg0ire
left a comment
There was a problem hiding this comment.
There are 2 mixed concerns in this PR, one of them wasn't in the original PR. There should at least be 2 commits, each with meaningful commit messages that don't just tell us what you did, but why you did it. If you want things to go fast, I suggest you create several PRs, one for each commit, with good commit messages and explanations, rather than pinging us in the middle of the night, only one hour after creating your PR. And by "suggest", I of course mean "require".
mvorisek
changed the title
| return $res; | ||
| } | ||
|
|
||
| public function testInternalKeywordListsAreSortedForEasierMaintenance(): void |
There was a problem hiding this comment.
i think that this should be a code style check rather than an unit test.
There was a problem hiding this comment.
If you know how, then great and definitely tell me, if not, let's move with this PR. In #106 I need to add many keywords and I want to make sure the git diff is minimal for now and long term.
mvorisek
changed the title
3 participants
as requested in #107 (review)
Into 1.4.x as this is needed for #106 bugfix where the lists are massively updated based on exact SQL specs.
We keep the lists alpha sorted for easier maintenance but resort them by length in the tokenizer for longest match.
The lists should not be defined sorted by length, as with comming bugfix PR, the sorting by length will no longer be needed, as there is nothing like "multi-word reserved keyword".