From afe06517d769d9abe4411ec8ccfac35c59ccf9f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Tue, 27 Jul 2021 16:50:05 +0200 Subject: [PATCH] Convert Elixir columns range to JavaScript (#472) --- lib/livebook/evaluator.ex | 2 +- lib/livebook/js_interop.ex | 25 +++++++++++++++---- lib/livebook/runtime.ex | 2 +- .../runtime/erl_dist/runtime_server.ex | 2 +- lib/livebook_web/live/session_live.ex | 21 ++++++++++++++-- test/livebook/evaluator_test.exs | 11 +++++--- test/livebook/js_interop_test.exs | 23 ++++++++++++++--- .../runtime/erl_dist/runtime_server_test.exs | 9 ++++--- 8 files changed, 74 insertions(+), 21 deletions(-) diff --git a/lib/livebook/evaluator.ex b/lib/livebook/evaluator.ex index ede4f005b..4339ec90d 100644 --- a/lib/livebook/evaluator.ex +++ b/lib/livebook/evaluator.ex @@ -225,7 +225,7 @@ defmodule Livebook.Evaluator do error -> Logger.error(Exception.format(:error, error, __STACKTRACE__)) end - send(send_to, {:intellisense_response, ref, response}) + send(send_to, {:intellisense_response, ref, request, response}) {:noreply, state} end diff --git a/lib/livebook/js_interop.ex b/lib/livebook/js_interop.ex index e8d027ac1..475c6c350 100644 --- a/lib/livebook/js_interop.ex +++ b/lib/livebook/js_interop.ex @@ -42,11 +42,10 @@ defmodule Livebook.JSInterop do @doc """ Returns a column number in the Elixir string corresponding to - the given column interpreted in terms of UTF-16 code units as - JavaScript does. + the given column interpreted in terms of UTF-16 code units. """ - @spec convert_column_to_elixir(pos_integer(), String.t()) :: pos_integer() - def convert_column_to_elixir(column, line) do + @spec js_column_to_elixir(pos_integer(), String.t()) :: pos_integer() + def js_column_to_elixir(column, line) do line |> string_to_utf16_code_units() |> Enum.take(column - 1) @@ -55,7 +54,23 @@ defmodule Livebook.JSInterop do |> Kernel.+(1) end - # --- + @doc """ + Returns a column represented in terms of UTF-16 code units + corresponding to the given column number in Elixir string. + """ + @spec elixir_column_to_js(pos_integer(), String.t()) :: pos_integer() + def elixir_column_to_js(column, line) do + line + |> string_take(column - 1) + |> string_to_utf16_code_units() + |> length() + |> Kernel.+(1) + end + + defp string_take(_string, 0), do: "" + defp string_take(string, n) when n > 0, do: String.slice(string, 0..(n - 1)) + + # UTF-16 helpers defp string_to_utf16_code_units(string) do string diff --git a/lib/livebook/runtime.ex b/lib/livebook/runtime.ex index bb31f16cf..c64ef3fa2 100644 --- a/lib/livebook/runtime.ex +++ b/lib/livebook/runtime.ex @@ -184,7 +184,7 @@ defprotocol Livebook.Runtime do the text editor. The response is sent to the `send_to` process as - `{:intellisense_response, ref, response}`. + `{:intellisense_response, ref, request, response}`. The given `locator` idenfities an evaluation that may be used as context when resolving the request (if relevant). diff --git a/lib/livebook/runtime/erl_dist/runtime_server.ex b/lib/livebook/runtime/erl_dist/runtime_server.ex index b4e1e6f89..03d7d58f0 100644 --- a/lib/livebook/runtime/erl_dist/runtime_server.ex +++ b/lib/livebook/runtime/erl_dist/runtime_server.ex @@ -223,7 +223,7 @@ defmodule Livebook.Runtime.ErlDist.RuntimeServer do binding = [] env = :elixir.env_for_eval([]) response = Livebook.Intellisense.handle_request(request, binding, env) - send(send_to, {:intellisense_response, ref, response}) + send(send_to, {:intellisense_response, ref, request, response}) end) end diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index e0b858c28..6257ecf51 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -8,6 +8,7 @@ defmodule LivebookWeb.SessionLive do alias LivebookWeb.SidebarHelpers alias Livebook.{SessionSupervisor, Session, Delta, Notebook, Runtime, LiveMarkdown} alias Livebook.Notebook.Cell + alias Livebook.JSInterop @impl true def mount(%{"id" => session_id}, %{"current_user_id" => current_user_id} = session, socket) do @@ -633,7 +634,7 @@ defmodule LivebookWeb.SessionLive do {:completion, hint} %{"type" => "details", "line" => line, "column" => column} -> - column = Livebook.JSInterop.convert_column_to_elixir(column, line) + column = JSInterop.js_column_to_elixir(column, line) {:details, line, column} %{"type" => "format", "code" => code} -> @@ -758,7 +759,8 @@ defmodule LivebookWeb.SessionLive do |> push_redirect(to: Routes.home_path(socket, :page))} end - def handle_info({:intellisense_response, ref, response}, socket) do + def handle_info({:intellisense_response, ref, request, response}, socket) do + response = process_intellisense_response(response, request) payload = %{"ref" => inspect(ref), "response" => response} {:noreply, push_event(socket, "intellisense_response", payload)} end @@ -1046,6 +1048,21 @@ defmodule LivebookWeb.SessionLive do end) end + defp process_intellisense_response( + %{range: %{from: from, to: to}} = response, + {:details, line, _column} + ) do + %{ + response + | range: %{ + from: JSInterop.elixir_column_to_js(from, line), + to: JSInterop.elixir_column_to_js(to, line) + } + } + end + + defp process_intellisense_response(response, _request), do: response + # Builds view-specific structure of data by cherry-picking # only the relevant attributes. # We then use `@data_view` in the templates and consequently diff --git a/test/livebook/evaluator_test.exs b/test/livebook/evaluator_test.exs index 8a5580a54..f4a49d8f6 100644 --- a/test/livebook/evaluator_test.exs +++ b/test/livebook/evaluator_test.exs @@ -223,7 +223,9 @@ defmodule Livebook.EvaluatorTest do test "sends completion response to the given process", %{evaluator: evaluator} do request = {:completion, "System.ver"} Evaluator.handle_intellisense(evaluator, self(), :ref, request) - assert_receive {:intellisense_response, :ref, %{items: [%{label: "version/0"}]}}, 1_000 + + assert_receive {:intellisense_response, :ref, ^request, %{items: [%{label: "version/0"}]}}, + 1_000 end test "given evaluation reference uses its bindings and env", %{evaluator: evaluator} do @@ -237,12 +239,15 @@ defmodule Livebook.EvaluatorTest do request = {:completion, "num"} Evaluator.handle_intellisense(evaluator, self(), :ref, request, :code_1) - assert_receive {:intellisense_response, :ref, %{items: [%{label: "number"}]}}, 1_000 + + assert_receive {:intellisense_response, :ref, ^request, %{items: [%{label: "number"}]}}, + 1_000 request = {:completion, "ANSI.brigh"} Evaluator.handle_intellisense(evaluator, self(), :ref, request, :code_1) - assert_receive {:intellisense_response, :ref, %{items: [%{label: "bright/0"}]}}, 1_000 + assert_receive {:intellisense_response, :ref, ^request, %{items: [%{label: "bright/0"}]}}, + 1_000 end end diff --git a/test/livebook/js_interop_test.exs b/test/livebook/js_interop_test.exs index 2f4772e1d..c9dfa094a 100644 --- a/test/livebook/js_interop_test.exs +++ b/test/livebook/js_interop_test.exs @@ -44,18 +44,18 @@ defmodule Livebook.JSInteropTest do end end - describe "convert_column_to_elixir/2" do + describe "js_column_to_elixir/2" do test "keeps the column as is for ASCII characters" do column = 4 line = "String.replace" - assert JSInterop.convert_column_to_elixir(column, line) == 4 + assert JSInterop.js_column_to_elixir(column, line) == 4 end test "shifts the column given characters spanning multiple UTF-16 code units" do # 🚀 consists of 2 UTF-16 code units, so JavaScript assumes "🚀".length is 2 column = 7 line = "🚀🚀 String.replace" - assert JSInterop.convert_column_to_elixir(column, line) == 5 + assert JSInterop.js_column_to_elixir(column, line) == 5 end test "returns proper column if a middle UTF-16 code unit is given" do @@ -63,7 +63,22 @@ defmodule Livebook.JSInteropTest do # 3th and 4th code unit correspond to the second 🚀 column = 3 line = "🚀🚀 String.replace" - assert JSInterop.convert_column_to_elixir(column, line) == 2 + assert JSInterop.js_column_to_elixir(column, line) == 2 + end + end + + describe "elixir_column_to_js/2" do + test "keeps the column as is for ASCII characters" do + column = 4 + line = "String.replace" + assert JSInterop.elixir_column_to_js(column, line) == 4 + end + + test "shifts the column given characters spanning multiple UTF-16 code units" do + # 🚀 consists of 2 UTF-16 code units, so JavaScript assumes "🚀".length is 2 + column = 5 + line = "🚀🚀 String.replace" + assert JSInterop.elixir_column_to_js(column, line) == 7 end end end diff --git a/test/livebook/runtime/erl_dist/runtime_server_test.exs b/test/livebook/runtime/erl_dist/runtime_server_test.exs index d8f169185..dbee36f1a 100644 --- a/test/livebook/runtime/erl_dist/runtime_server_test.exs +++ b/test/livebook/runtime/erl_dist/runtime_server_test.exs @@ -135,7 +135,7 @@ defmodule Livebook.Runtime.ErlDist.RuntimeServerTest do request = {:completion, "System.ver"} RuntimeServer.handle_intellisense(pid, self(), :ref, request, {:c1, nil}) - assert_receive {:intellisense_response, :ref, %{items: [%{label: "version/0"}]}} + assert_receive {:intellisense_response, :ref, ^request, %{items: [%{label: "version/0"}]}} end test "provides extended completion when previous evaluation reference is given", %{pid: pid} do @@ -145,7 +145,7 @@ defmodule Livebook.Runtime.ErlDist.RuntimeServerTest do request = {:completion, "num"} RuntimeServer.handle_intellisense(pid, self(), :ref, request, {:c1, :e1}) - assert_receive {:intellisense_response, :ref, %{items: [%{label: "number"}]}} + assert_receive {:intellisense_response, :ref, ^request, %{items: [%{label: "number"}]}} end end @@ -154,7 +154,8 @@ defmodule Livebook.Runtime.ErlDist.RuntimeServerTest do request = {:details, "System.version", 10} RuntimeServer.handle_intellisense(pid, self(), :ref, request, {:c1, nil}) - assert_receive {:intellisense_response, :ref, %{range: %{from: 1, to: 15}, contents: [_]}} + assert_receive {:intellisense_response, :ref, ^request, + %{range: %{from: 1, to: 15}, contents: [_]}} end end @@ -163,7 +164,7 @@ defmodule Livebook.Runtime.ErlDist.RuntimeServerTest do request = {:format, "System.version"} RuntimeServer.handle_intellisense(pid, self(), :ref, request, {:c1, nil}) - assert_receive {:intellisense_response, :ref, %{code: "System.version()"}} + assert_receive {:intellisense_response, :ref, ^request, %{code: "System.version()"}} end end