lexical-lsp / lexical

Lexical is a next-generation elixir language server
781 stars 77 forks source link

New completion sorting approach #643

Closed Moosieus closed 3 months ago

Moosieus commented 3 months ago

Here's a brief completion sorting proposal I've made up:

"00": local variables "01": local declarations (declared within the immediate module or use) "10": remote declarations (from existing invocations of alias, require, and import) "11": global declarations (def, do/end, if/do/else/end, SpecialForms, etc) "20": auto declarations (short for auto-import added by the LSP)

variables = variables. Made relatively easy by Elixir's scoping rules. declarations = functions, attributes, atoms(?), and all classes of macros (macros, guards, structs, protocols, etc).

This is mainly inspired by the way TypeScript (tsserver) handles its completion suggestions, which are taxonomized and sorted as:

LocalDeclarationPriority: "10" as SortText,
LocationPriority: "11" as SortText,
OptionalMember: "12" as SortText,
MemberDeclaredBySpreadAssignment: "13" as SortText,
SuggestedClassMembers: "14" as SortText,
GlobalsOrKeywords: "15" as SortText,
AutoImportSuggestions: "16" as SortText,
ClassMemberSnippets: "17" as SortText,
JavascriptIdentifiers: "18" as SortT

(source)

I like what TS does here b/c:

I think the approach works well for Elixir and Lexical b/c:

Some personal notes:

The API might look something like:

Builder.snippet(env, insert_text, sort_scope, [lsp: "fields"])

where sort_scope is one of:

defp sort_scope(:local_variables), do: "00"
defp sort_scope(:local_declarations), do: "01"
defp sort_scope(:remote_declarations), do: "10"
defp sort_scope(:global_declarations), do: "11"
defp sort_scope(:auto_declarations), do: "20"

defp sport_scope(unknown) do
  raise(ArgumentError, "invalid sort_scope: #{inspect(unknown)}")
end
scohen commented 3 months ago

I really agree with the thinking here, I'm often miffed that the function/variable right next to what I'm doing is sorted beneath the function. I do think scopes need to be defined somewhere concrete, rather than as atoms. For example, Make a Builder.SortScope module that includes the functions local_variable(), local_declaration(), remote_declaration(), etc.

A side note, is it easy, from the builder to see what's local and remote? I don't think this is the case.

Moosieus commented 3 months ago

I'd set sort-scopes in the same locations boosts are done now, that's where the necessary context is accessible atm.

I sorta consider it an abstraction leak, but I think it's a surmountable one.