-
Notifications
You must be signed in to change notification settings - Fork 156
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
Better Completion for ExternalAutocomplete functions #1178
Conversation
Add FullNameExternalAutocomplete
(for `module Result` added `open System.Diagnostics.Contracts.Contract`)
fix: completions with same label have same description add: filter completions by full name under fullNameExternalAutocomplete add: tests for fullNameExternalAutocomplete
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.
🎸 This looks great! Gonna let @baronfel make a review pass too.
Kind = (AVal.force glyphToCompletionKind) d.Glyph | ||
InsertText = Some code | ||
InsertText = Some(getCodeToInsert d) |
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.
nit: getCodeToInsert d
is calculate above in code
- can we reuse it here to prevent recalculating?
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.
fixed
| Some no when config.FullNameExternalAutocomplete -> | ||
// allow filter "Ceiling (System.Math)" by "System.Math.Ceiling" or "CeilingSystem.Math" | ||
sprintf "%s (%s)" d.NameInList no, d.NameInList + code | ||
| Some no -> sprintf "%s (open %s)" d.NameInList no, d.NameInList | ||
| None -> d.NameInList, d.NameInList |
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.
can you describe in a comment the logic going on here? how does the label selection and the filter text impact the user experience of searching/filtering in the IDE?
The previous implementation didn't document why decisions were made, and so we don't have any sort of record for justification for making changes.
Thanks @Tangent-90 - that helps me understand the intent quite a bit more! |
Pushed a tiny change to add more to the docs so it was clearer to me personally, but otherwise this LGTM and let's ship it! Ideally we could also add the option to the Ionide package.json so that end users have some docs for it. |
Co-authored-by: Chet Husk <[email protected]>
Co-authored-by: Chet Husk <[email protected]>
WHAT
🤖 Generated by Copilot at b053be9
This pull request adds a new feature to the F# language server, which allows the user to configure how completion items for external symbols are displayed. It also improves the completion items for hash directives and keywords, and adds tests for the new feature. The main files affected are
src/FsAutoComplete.Core/KeywordList.fs
,src/FsAutoComplete/LspHelpers.fs
,src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs
, andtest/FsAutoComplete.Tests.Lsp/CompletionTests.fs
.🤖 Generated by Copilot at b053be9
🧑💻🔧🧪
WHY
Array.map
"Before:
After:
Before:
After:
FullNameExternalAutocomplete
Before:
With
FullNameExternalAutocomplete
:HOW
🤖 Generated by Copilot at b053be9
getCodeToInsert
function and thedata
field of the completion items (link, link, link, link, link, link)label
field and adata
field to the completion items for hash directives and keywords, using the#
prefix for directives (link, link, link)fullNameExternalAutocompleteTest
function and theRange.FullNameExternalAutocomplete
field (link, link)