-
-
Notifications
You must be signed in to change notification settings - Fork 357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix 13825. Create a new sorter for the completion engine . #17473
base: Pharo13
Are you sure you want to change the base?
Fix 13825. Create a new sorter for the completion engine . #17473
Conversation
Thanks for opening this pull request! Now continious integration (CI) will build Pharo with your change and run all tests. This might fail due to many reasons! Please check if your PR breaks the build or makes tests fail. Feel free to add comments to the PR. After this, before your PR can be merged it needs one or more reviews. Do not hesitate to ask people (on the Mailinglist or Discord) to help! When the CI shows no problems and there are positive reviews, your PR will be merged. |
Before merging this pull-request, please note that it does not changes the default strategy selected in the Pharo settings. Edit: @omarabedelkader changed the default sorter to be the |
src/NECompletion/NoSorter.class.st
Outdated
@@ -0,0 +1,20 @@ | |||
" | |||
I'm a class that do nothing in the suggestion list. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that does nothing
src/NECompletion/NoSorter.class.st
Outdated
" | ||
I'm a class that do nothing in the suggestion list. | ||
I return the list created from the compltion engine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
completion engine
{ #category : 'accessing' } | ||
CoCompletionEngineTest >> tearDown [ | ||
super tearDown. | ||
CompletionContext sorterClass: oldSort. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please tab!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@omarabedelkader Because you used 4 spaces instead of 1 tab
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right !
i think now it's fixed.
src/NECompletion/SizeSorter.class.st
Outdated
@@ -0,0 +1,20 @@ | |||
" | |||
I'm a class that do sorting based on the size of the context itself. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does sorting
In order to fix the issue , we first implement the Sorter logic already existant (Alpahbetic and ReverseAlphabetic ) and we did also implement a new Sorter logic based on the size of the completion token.
with @ValentinBourcier @olekscode