From d909272746d4881ecfdc9bc2f7b9915b8cf15650 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Sun, 5 Dec 2021 14:58:30 +0100 Subject: [PATCH] Improve completion (#747) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add keywords to completion * Fix signature request caching for call without parentheses * Don't insert parentheses for def* macros * Don't trigger missing runtime message when auto completion is enabled * Don't insert parentheses for keyword macros * Improve completion of env macros * Apply review comments * Update locals without parentheses * Apply suggestions from code review Co-authored-by: José Valim * Format Co-authored-by: José Valim --- assets/js/cell/live_editor.js | 10 +- lib/livebook/intellisense.ex | 112 ++++++++++++++++-- .../intellisense/identifier_matcher.ex | 44 +++++-- lib/livebook/runtime.ex | 2 +- lib/livebook_web/live/session_live.ex | 8 +- test/livebook/intellisense_test.exs | 50 +++++++- test/livebook_web/live/session_live_test.exs | 6 +- 7 files changed, 201 insertions(+), 31 deletions(-) diff --git a/assets/js/cell/live_editor.js b/assets/js/cell/live_editor.js index 35aa78b4f..683710fb5 100644 --- a/assets/js/cell/live_editor.js +++ b/assets/js/cell/live_editor.js @@ -250,6 +250,7 @@ class LiveEditor { return this.__asyncIntellisenseRequest("completion", { hint: lineUntilCursor, + editor_auto_completion: settings.editor_auto_completion, }) .then((response) => { const suggestions = completionItemsToSuggestions( @@ -311,8 +312,11 @@ class LiveEditor { const lineUntilCursor = lines[lineIdx].slice(0, position.column - 1); const codeUntilCursor = [...prevLines, lineUntilCursor].join("\n"); - // Remove trailing characters that don't affect the signature - const codeUntilLastStop = codeUntilCursor.replace(/[^(),]*?$/, ""); + const codeUntilLastStop = codeUntilCursor + // Remove trailing characters that don't affect the signature + .replace(/[^(),\s]*?$/, "") + // Remove whitespace before delimiter + .replace(/([(),])\s*$/, "$1"); // Cache subsequent requests for the same prefix, so that we don't // make unnecessary requests @@ -464,6 +468,8 @@ function parseItemKind(kind) { return monaco.languages.CompletionItemKind.Variable; case "field": return monaco.languages.CompletionItemKind.Field; + case "keyword": + return monaco.languages.CompletionItemKind.Keyword; default: return null; } diff --git a/lib/livebook/intellisense.ex b/lib/livebook/intellisense.ex index fdeea3d22..8d13e3030 100644 --- a/lib/livebook/intellisense.ex +++ b/lib/livebook/intellisense.ex @@ -114,13 +114,14 @@ defmodule Livebook.Intellisense do IdentifierMatcher.completion_identifiers(hint, binding, env) |> Enum.filter(&include_in_completion?/1) |> Enum.map(&format_completion_item/1) + |> Enum.concat(extra_completion_items(hint)) |> Enum.sort_by(&completion_item_priority/1) end defp include_in_completion?({:module, _module, _display_name, :hidden}), do: false defp include_in_completion?( - {:function, _module, _name, _arity, _display_name, :hidden, _signatures, _specs} + {:function, _module, _name, _arity, _type, _display_name, :hidden, _signatures, _specs} ), do: false @@ -168,7 +169,7 @@ defmodule Livebook.Intellisense do end defp format_completion_item( - {:function, module, name, arity, display_name, documentation, signatures, specs} + {:function, module, name, arity, type, display_name, documentation, signatures, specs} ), do: %{ label: "#{display_name}/#{arity}", @@ -181,11 +182,24 @@ defmodule Livebook.Intellisense do ]), insert_text: cond do - String.starts_with?(display_name, "~") -> display_name - Macro.operator?(name, arity) -> display_name - # A snippet with cursor in parentheses - arity == 0 -> "#{display_name}()" - true -> "#{display_name}($0)" + type == :macro and keyword_macro?(name) -> + "#{display_name} " + + type == :macro and env_macro?(name) -> + display_name + + String.starts_with?(display_name, "~") -> + display_name + + Macro.operator?(name, arity) -> + display_name + + arity == 0 -> + "#{display_name}()" + + true -> + # A snippet with cursor in parentheses + "#{display_name}($0)" end } @@ -207,7 +221,87 @@ defmodule Livebook.Intellisense do insert_text: name } - @ordered_kinds [:field, :variable, :module, :struct, :interface, :function, :type] + defp keyword_macro?(name) do + def? = name |> Atom.to_string() |> String.starts_with?("def") + + def? or + name in [ + # Special forms + :alias, + :case, + :cond, + :for, + :fn, + :import, + :quote, + :receive, + :require, + :try, + :with, + + # Kernel + :destructure, + :raise, + :reraise, + :if, + :unless, + :use + ] + end + + defp env_macro?(name) do + name in [:__ENV__, :__MODULE__, :__DIR__, :__STACKTRACE__, :__CALLER__] + end + + defp extra_completion_items(hint) do + items = [ + %{ + label: "do", + kind: :keyword, + detail: "do-end block", + documentation: nil, + insert_text: "do\n $0\nend" + }, + %{ + label: "true", + kind: :keyword, + detail: "boolean", + documentation: nil, + insert_text: "true" + }, + %{ + label: "false", + kind: :keyword, + detail: "boolean", + documentation: nil, + insert_text: "false" + }, + %{ + label: "nil", + kind: :keyword, + detail: "special atom", + documentation: nil, + insert_text: "nil" + }, + %{ + label: "when", + kind: :keyword, + detail: "guard operator", + documentation: nil, + insert_text: "when" + } + ] + + last_word = hint |> String.split(~r/\s/) |> List.last() + + if last_word == "" do + [] + else + Enum.filter(items, &String.starts_with?(&1.label, last_word)) + end + end + + @ordered_kinds [:keyword, :field, :variable, :module, :struct, :interface, :function, :type] defp completion_item_priority(%{kind: :struct, detail: "exception"} = completion_item) do {length(@ordered_kinds), completion_item.label} @@ -263,7 +357,7 @@ defmodule Livebook.Intellisense do end defp format_details_item( - {:function, module, name, _arity, _display_name, documentation, signatures, specs} + {:function, module, name, _arity, _type, _display_name, documentation, signatures, specs} ) do join_with_divider([ format_signatures(signatures, module) |> code(), diff --git a/lib/livebook/intellisense/identifier_matcher.ex b/lib/livebook/intellisense/identifier_matcher.ex index cc270c491..94defe41d 100644 --- a/lib/livebook/intellisense/identifier_matcher.ex +++ b/lib/livebook/intellisense/identifier_matcher.ex @@ -22,14 +22,15 @@ defmodule Livebook.Intellisense.IdentifierMatcher do {:variable, name(), value()} | {:map_field, name(), value()} | {:module, module(), display_name(), Docs.documentation()} - | {:function, module(), name(), arity(), display_name(), Docs.documentation(), - list(Docs.signature()), list(Docs.spec())} + | {:function, module(), name(), arity(), function_type(), display_name(), + Docs.documentation(), list(Docs.signature()), list(Docs.spec())} | {:type, module(), name(), arity(), Docs.documentation()} | {:module_attribute, name(), Docs.documentation()} @type name :: atom() @type display_name :: String.t() @type value :: term() + @type function_type :: :function | :macro @exact_matcher &Kernel.==/2 @prefix_matcher &String.starts_with?/2 @@ -241,10 +242,13 @@ defmodule Livebook.Intellisense.IdentifierMatcher do end defp match_sigil(hint, ctx) do - for {:function, module, name, arity, "sigil_" <> sigil_name, documentation, signatures, specs} <- + for {:function, module, name, arity, type, "sigil_" <> sigil_name, documentation, signatures, + specs} <- match_local("sigil_", %{ctx | matcher: @prefix_matcher}), ctx.matcher.(sigil_name, hint), - do: {:function, module, name, arity, "~" <> sigil_name, documentation, signatures, specs} + do: + {:function, module, name, arity, type, "~" <> sigil_name, documentation, signatures, + specs} end defp match_erlang_module(hint, ctx) do @@ -376,24 +380,26 @@ defmodule Livebook.Intellisense.IdentifierMatcher do funs = funs || exports(mod) matching_funs = - Enum.filter(funs, fn {name, _arity} -> + Enum.filter(funs, fn {name, _arity, _type} -> name = Atom.to_string(name) ctx.matcher.(name, hint) end) doc_items = - Livebook.Intellisense.Docs.lookup_module_members(mod, matching_funs, + Livebook.Intellisense.Docs.lookup_module_members( + mod, + Enum.map(matching_funs, &Tuple.delete_at(&1, 2)), kinds: [:function, :macro] ) - Enum.map(matching_funs, fn {name, arity} -> + Enum.map(matching_funs, fn {name, arity, type} -> doc_item = Enum.find(doc_items, %{documentation: nil, signatures: [], specs: []}, fn doc_item -> doc_item.name == name && doc_item.arity == arity end) - {:function, mod, name, arity, Atom.to_string(name), doc_item && doc_item.documentation, - doc_item.signatures, doc_item.specs} + {:function, mod, name, arity, type, Atom.to_string(name), + doc_item && doc_item.documentation, doc_item.signatures, doc_item.specs} end) else [] @@ -402,12 +408,19 @@ defmodule Livebook.Intellisense.IdentifierMatcher do defp exports(mod) do if Code.ensure_loaded?(mod) and function_exported?(mod, :__info__, 1) do - mod.__info__(:macros) ++ (mod.__info__(:functions) -- [__info__: 1]) + macros = mod.__info__(:macros) + functions = mod.__info__(:functions) -- [__info__: 1] + append_funs_type(macros, :macro) ++ append_funs_type(functions, :function) else - mod.module_info(:exports) -- [module_info: 0, module_info: 1] + functions = mod.module_info(:exports) -- [module_info: 0, module_info: 1] + append_funs_type(functions, :function) end end + defp append_funs_type(funs, type) do + Enum.map(funs, &Tuple.append(&1, type)) + end + defp match_module_type(mod, hint, ctx) do types = get_module_types(mod) @@ -444,7 +457,14 @@ defmodule Livebook.Intellisense.IdentifierMatcher do defp ensure_loaded?(Elixir), do: false defp ensure_loaded?(mod), do: Code.ensure_loaded?(mod) - defp imports_from_env(env), do: env.functions ++ env.macros + defp imports_from_env(env) do + Enum.map(env.functions, fn {mod, funs} -> + {mod, append_funs_type(funs, :function)} + end) ++ + Enum.map(env.macros, fn {mod, funs} -> + {mod, append_funs_type(funs, :macro)} + end) + end defp split_at_last_occurrence(string, pattern) do case :binary.matches(string, pattern) do diff --git a/lib/livebook/runtime.ex b/lib/livebook/runtime.ex index 5c8715d60..b47e083ee 100644 --- a/lib/livebook/runtime.ex +++ b/lib/livebook/runtime.ex @@ -72,7 +72,7 @@ defprotocol Livebook.Runtime do } @type completion_item_kind :: - :function | :module | :struct | :interface | :type | :variable | :field + :function | :module | :struct | :interface | :type | :variable | :field | :keyword @typedoc """ Looks up more details about an identifier found in `column` in `line`. diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index 7920a9f0b..a621d9f67 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -784,14 +784,14 @@ defmodule LivebookWeb.SessionLive do {:reply, %{"ref" => inspect(ref)}, socket} else info = - case params["type"] do - "completion" -> + cond do + params["type"] == "completion" and not params["editor_auto_completion"] -> "You need to start a runtime (or evaluate a cell) for code completion" - "format" -> + params["type"] == "format" -> "You need to start a runtime (or evaluate a cell) to enable code formatting" - _ -> + true -> nil end diff --git a/test/livebook/intellisense_test.exs b/test/livebook/intellisense_test.exs index 7731058f7..34a60080a 100644 --- a/test/livebook/intellisense_test.exs +++ b/test/livebook/intellisense_test.exs @@ -738,7 +738,7 @@ defmodule Livebook.IntellisenseTest do kind: :function, detail: "Kernel.SpecialForms.quote(opts, block)", documentation: "Gets the representation of any expression.", - insert_text: "quote($0)" + insert_text: "quote " } ] = Intellisense.get_completion_items("quot", binding, env) end @@ -1110,6 +1110,54 @@ defmodule Livebook.IntellisenseTest do {binding, env} = eval(do: nil) assert [] = Intellisense.get_completion_items("@attr.value", binding, env) end + + test "includes language keywords" do + {binding, env} = eval(do: nil) + + assert [ + %{ + label: "do", + kind: :keyword, + detail: "do-end block", + documentation: nil, + insert_text: "do\n $0\nend" + } + | _ + ] = Intellisense.get_completion_items("do", binding, env) + end + + test "includes space instead of parentheses for def* macros" do + {binding, env} = eval(do: nil) + + assert [ + %{ + label: "defmodule/2", + insert_text: "defmodule " + } + ] = Intellisense.get_completion_items("defmodu", binding, env) + end + + test "includes space instead of parentheses for keyword macros" do + {binding, env} = eval(do: nil) + + assert [ + %{ + label: "import/2", + insert_text: "import " + } + ] = Intellisense.get_completion_items("impor", binding, env) + end + + test "includes doesn't include space nor parentheses for macros like __ENV__" do + {binding, env} = eval(do: nil) + + assert [ + %{ + label: "__ENV__/0", + insert_text: "__ENV__" + } + ] = Intellisense.get_completion_items("__EN", binding, env) + end end describe "get_details/3" do diff --git a/test/livebook_web/live/session_live_test.exs b/test/livebook_web/live/session_live_test.exs index 7cdef2344..ab3740547 100644 --- a/test/livebook_web/live/session_live_test.exs +++ b/test/livebook_web/live/session_live_test.exs @@ -348,7 +348,8 @@ defmodule LivebookWeb.SessionLiveTest do |> render_hook("intellisense_request", %{ "cell_id" => cell_id, "type" => "completion", - "hint" => "System.ver" + "hint" => "System.ver", + "editor_auto_completion" => false }) assert_reply view, %{"ref" => nil} @@ -369,7 +370,8 @@ defmodule LivebookWeb.SessionLiveTest do |> render_hook("intellisense_request", %{ "cell_id" => cell_id, "type" => "completion", - "hint" => "System.ver" + "hint" => "System.ver", + "editor_auto_completion" => false }) assert_reply view, %{"ref" => ref}