WIP: Kill copypasted code in completions #4617
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
treapster
changed the title
treapster
changed the title
|
I completely forgot that there are also overrides of CompletionSourceFuse::filter with the same copypasta. And there are too many methods which call each other - filter and handleCommand are doing the same thing and onInput just confuses by unnecessary indirection, and sometimes onInput calls filter, other times it's vice versa.. I'll try to sort it out when i have time |
|
Even though more testing is needed, it seems like the main mess is gone, and from the first glance it looks like completions are still working. Will test with more commands in the following days. |
treapster
force-pushed
the
cleanup-completions
branch
from
bd44c2b
to
9b812d1
Compare
Also move them around to please eslint
Even though it makes them work, the general scoring logic should be definetely changed and put in one place, because currently is is still repeated in multiple places.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Aside from moving copypasted code to the base class, i also had to move updateOptions in TabHistory to please eslint, as well as suppress unused var warnings for overrides. Also, i discovered that our typescript is rather old(3.9.10 while the latest is 4.9.5) and doesn't know the override keyword. What stops us from bumping it to 4-something? It may also help with the case we bumped into here and probably some other things.
As for the patch itself, it's probably not the best solution but it removes some 100+ lines of repetition so i hope we can call it improvement, even though it's still OOP which i heard is not very liked here:)