Skip to content
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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Oct 20, 2023
Merged

Better Completion for ExternalAutocomplete functions #1178

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
932a30b
Try fix insert "Array.map" become "``Array.map``"
ijklam Oct 18, 2023
c16da84
fix format
ijklam Oct 18, 2023
b7837ea
Fix: Completion insert wrong open statement
ijklam Oct 18, 2023
537faf9
fix
ijklam Oct 18, 2023
b053be9
fix and improve
ijklam Oct 19, 2023
0462309
fix
ijklam Oct 20, 2023
c17a9d0
minor refactor: extract completion item to function and expand on docs
baronfel Oct 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/FsAutoComplete.Core/KeywordList.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,16 @@ module KeywordList =
let hashSymbolCompletionItems =
hashDirectives
|> Seq.map (fun kv ->
let label = "#" + kv.Key

{ CompletionItem.Create(kv.Key) with
Data = Some(Newtonsoft.Json.Linq.JValue(label))
Kind = Some CompletionItemKind.Keyword
InsertText = Some kv.Key
FilterText = Some kv.Key
SortText = Some kv.Key
Documentation = Some(Documentation.String kv.Value)
Label = "#" + kv.Key })
Label = label })
|> Seq.toArray

let allKeywords: string list =
Expand All @@ -58,6 +61,7 @@ module KeywordList =
allKeywords
|> List.mapi (fun id k ->
{ CompletionItem.Create(k) with
Data = Some(Newtonsoft.Json.Linq.JValue(k))
Kind = Some CompletionItemKind.Keyword
InsertText = Some k
SortText = Some(sprintf "1000000%d" id)
Expand Down
5 changes: 5 additions & 0 deletions src/FsAutoComplete/LspHelpers.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,7 @@ type FSharpConfigDto =
ExcludeProjectDirectories: string[] option
KeywordsAutocomplete: bool option
ExternalAutocomplete: bool option
FullNameExternalAutocomplete: bool option
Linter: bool option
LinterConfig: string option
IndentationSize: int option
Expand Down Expand Up @@ -771,6 +772,7 @@ type FSharpConfig =
ExcludeProjectDirectories: string[]
KeywordsAutocomplete: bool
ExternalAutocomplete: bool
FullNameExternalAutocomplete: bool
Linter: bool
LinterConfig: string option
IndentationSize: int
Expand Down Expand Up @@ -816,6 +818,7 @@ type FSharpConfig =
ExcludeProjectDirectories = [||]
KeywordsAutocomplete = false
ExternalAutocomplete = false
FullNameExternalAutocomplete = false
IndentationSize = 4
Linter = false
LinterConfig = None
Expand Down Expand Up @@ -861,6 +864,7 @@ type FSharpConfig =
ExcludeProjectDirectories = defaultArg dto.ExcludeProjectDirectories [||]
KeywordsAutocomplete = defaultArg dto.KeywordsAutocomplete false
ExternalAutocomplete = defaultArg dto.ExternalAutocomplete false
FullNameExternalAutocomplete = defaultArg dto.ExternalAutocomplete false
IndentationSize = defaultArg dto.IndentationSize 4
Linter = defaultArg dto.Linter false
LinterConfig = dto.LinterConfig
Expand Down Expand Up @@ -958,6 +962,7 @@ type FSharpConfig =
ExcludeProjectDirectories = defaultArg dto.ExcludeProjectDirectories x.ExcludeProjectDirectories
KeywordsAutocomplete = defaultArg dto.KeywordsAutocomplete x.KeywordsAutocomplete
ExternalAutocomplete = defaultArg dto.ExternalAutocomplete x.ExternalAutocomplete
FullNameExternalAutocomplete = defaultArg dto.FullNameExternalAutocomplete x.FullNameExternalAutocomplete
IndentationSize = defaultArg dto.IndentationSize x.IndentationSize
Linter = defaultArg dto.Linter x.Linter
LinterConfig = dto.LinterConfig
Expand Down
2 changes: 2 additions & 0 deletions src/FsAutoComplete/LspHelpers.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ type FSharpConfigDto =
ExcludeProjectDirectories: string[] option
KeywordsAutocomplete: bool option
ExternalAutocomplete: bool option
FullNameExternalAutocomplete: bool option
Linter: bool option
LinterConfig: string option
IndentationSize: int option
Expand Down Expand Up @@ -363,6 +364,7 @@ type FSharpConfig =
ExcludeProjectDirectories: string[]
KeywordsAutocomplete: bool
ExternalAutocomplete: bool
FullNameExternalAutocomplete: bool
Linter: bool
LinterConfig: string option
IndentationSize: int
Expand Down
47 changes: 25 additions & 22 deletions src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2575,6 +2575,11 @@ type AdaptiveFSharpLspServer
getCompletions forceGetOpenFileTypeCheckResults
| _ -> getCompletions forceGetOpenFileTypeCheckResultsStale

let getCodeToInsert (d: DeclarationListItem) =
match d.NamespaceToOpen with
| Some no when config.FullNameExternalAutocomplete -> sprintf "%s.%s" no d.NameInCode
| _ -> d.NameInCode

match!
retryAsyncOption
(TimeSpan.FromMilliseconds(15.))
Expand All @@ -2592,7 +2597,7 @@ type AdaptiveFSharpLspServer
transact (fun () ->
HashMap.OfList(
[ for d in decls do
d.NameInList, (d, pos, filePath, volatileFile.Source.GetLine, typeCheckResults.GetAST) ]
d.FullName, (d, pos, filePath, volatileFile.Source.GetLine, typeCheckResults.GetAST) ]
)
|> autoCompleteItems.UpdateTo)
|> ignore<bool>
Expand All @@ -2602,26 +2607,22 @@ type AdaptiveFSharpLspServer
let items =
decls
|> Array.mapi (fun id d ->
let code =
if
System.Text.RegularExpressions.Regex.IsMatch(d.NameInList, """^[a-zA-Z][a-zA-Z0-9']+$""")
then
d.NameInList
elif d.NamespaceToOpen.IsSome then
d.NameInList
else
FSharpKeywords.NormalizeIdentifierBackticks d.NameInList

let label =
let code = getCodeToInsert d

let label, filterText =
match d.NamespaceToOpen with
| Some no -> sprintf "%s (open %s)" d.NameInList no
| None -> d.NameInList
| 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
Copy link
Contributor

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.


{ CompletionItem.Create(d.NameInList) with
Data = Some(JValue(d.FullName))
Kind = (AVal.force glyphToCompletionKind) d.Glyph
InsertText = Some code
InsertText = Some(getCodeToInsert d)
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

SortText = Some(sprintf "%06d" id)
FilterText = Some d.NameInList
FilterText = Some filterText
Label = label })

let its =
Expand Down Expand Up @@ -2650,6 +2651,8 @@ type AdaptiveFSharpLspServer
}

override __.CompletionItemResolve(ci: CompletionItem) =
let config = AVal.force config

let mapHelpText (ci: CompletionItem) (text: HelpText) =
match text with
| HelpText.Simple(symbolName, text) ->
Expand Down Expand Up @@ -2708,8 +2711,8 @@ type AdaptiveFSharpLspServer

let n =
match getAutoCompleteNamespacesByDeclName sym |> AVal.force with
| None -> None
| Some s -> Some s
| Some s when not config.FullNameExternalAutocomplete -> Some s
| _ -> None

CoreResponse.Res(HelpText.Full(sym, tip, n))

Expand All @@ -2725,10 +2728,10 @@ type AdaptiveFSharpLspServer
)

return!
match ci.InsertText with
| None -> LspResult.internalError "No InsertText"
| Some insertText ->
helpText insertText
match ci.Data with
| None -> LspResult.internalError "No FullName"
| Some fullName ->
helpText (fullName.ToString())
|> Result.ofCoreResponse
|> Result.bimap
(function
Expand Down
Loading