-
Notifications
You must be signed in to change notification settings - Fork 755
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
Improved the order of completions provided for resource access paths #41663
Merged
KavinduZoysa
merged 4 commits into
ballerina-platform:master
from
nipunayf:fix-path-resource
Nov 24, 2023
Merged
Improved the order of completions provided for resource access paths #41663
KavinduZoysa
merged 4 commits into
ballerina-platform:master
from
nipunayf:fix-path-resource
Nov 24, 2023
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
nipunayf
added
the
Team/LanguageServer
Language Server Implementation related issues. #Compiler
label
Nov 8, 2023
...rg/ballerinalang/langserver/completions/providers/context/InvocationNodeContextProvider.java
Outdated
Show resolved
Hide resolved
nipunayf
force-pushed
the
fix-path-resource
branch
3 times, most recently
from
November 20, 2023 11:32
892e9a2
to
b88e2eb
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #41663 +/- ##
============================================
+ Coverage 76.70% 76.73% +0.03%
- Complexity 52716 52724 +8
============================================
Files 2878 2878
Lines 198708 198735 +27
Branches 25825 25832 +7
============================================
+ Hits 152410 152501 +91
+ Misses 37882 37804 -78
- Partials 8416 8430 +14 ☔ View full report in Codecov by Sentry. |
...a/org/ballerinalang/langserver/completions/providers/context/AsyncSendActionNodeContext.java
Outdated
Show resolved
Hide resolved
...rinalang/langserver/completions/providers/context/ClientResourceAccessActionNodeContext.java
Outdated
Show resolved
Hide resolved
nipunayf
force-pushed
the
fix-path-resource
branch
from
November 23, 2023 15:23
3e710ba
to
29edffe
Compare
nipunayf
force-pushed
the
fix-path-resource
branch
from
November 24, 2023 03:47
29edffe
to
09e995d
Compare
KavinduZoysa
approved these changes
Nov 24, 2023
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.
Purpose
When multiple resource access methods share the same
sortText
, the VS Code plugin sorts them alphabetically, thus dividing completions with named and computed resource segments into two sets. This arrangement reduces the developer experience, with computed resource segments moved to the bottom.Fixes #41636
Approach
The current approach assigns a unique
sortText
to each completion item based on the position generated by the LS, ensuring that the order is not disrupted by the VS Code plugin.Samples
Grouped completions with both named resource segments and computed resource segments are now as shown below.
Check List