From af50646a8e56b0902b939d214bee9bdcc6858dd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Mon, 26 Jul 2021 11:57:51 +0200 Subject: [PATCH] Convert JavaScript string column to Elixir (#467) * Fix column/index wording * Convert JavaScript string column to Elixir --- assets/js/cell/live_editor.js | 4 ++-- lib/livebook/intellisense.ex | 20 ++++++++++---------- lib/livebook/js_interop.ex | 15 +++++++++++++++ lib/livebook/runtime.ex | 4 ++-- lib/livebook_web/live/session_live.ex | 5 +++-- test/livebook/js_interop_test.exs | 23 +++++++++++++++++++++++ 6 files changed, 55 insertions(+), 16 deletions(-) diff --git a/assets/js/cell/live_editor.js b/assets/js/cell/live_editor.js index 8567b5940..fc96f822a 100644 --- a/assets/js/cell/live_editor.js +++ b/assets/js/cell/live_editor.js @@ -238,9 +238,9 @@ class LiveEditor { this.editor.getModel().__getHover = (model, position) => { const line = model.getLineContent(position.lineNumber); - const index = position.column - 1; + const column = position.column; - return this.__asyncIntellisenseRequest("details", { line, index }) + return this.__asyncIntellisenseRequest("details", { line, column }) .then((response) => { const contents = response.contents.map((content) => ({ value: content, diff --git a/lib/livebook/intellisense.ex b/lib/livebook/intellisense.ex index 1f40e560c..2ac3da615 100644 --- a/lib/livebook/intellisense.ex +++ b/lib/livebook/intellisense.ex @@ -31,8 +31,8 @@ defmodule Livebook.Intellisense do %{items: items} end - def handle_request({:details, line, index}, binding, env) do - get_details(line, index, binding, env) + def handle_request({:details, line, column}, binding, env) do + get_details(line, column, binding, env) end def handle_request({:format, code}, _binding, _env) do @@ -140,12 +140,12 @@ defmodule Livebook.Intellisense do @doc """ Returns detailed information about identifier being - at `index` in `line`. + in `column` in `line`. """ - @spec get_details(String.t(), non_neg_integer(), Code.binding(), Macro.Env.t()) :: + @spec get_details(String.t(), pos_integer(), Code.binding(), Macro.Env.t()) :: Livebook.Runtime.details() | nil - def get_details(line, index, binding, env) do - {from, to} = subject_range(line, index) + def get_details(line, column, binding, env) do + {from, to} = subject_range(line, column) if from < to do subject = binary_part(line, from, to - from) @@ -170,9 +170,9 @@ defmodule Livebook.Intellisense do @closing_identifier '?!' @punctuation @non_closing_punctuation ++ @closing_punctuation - defp subject_range(line, index) do - {left, right} = String.split_at(line, index) - bytes_until_index = byte_size(left) + defp subject_range(line, column) do + {left, right} = String.split_at(line, column) + bytes_until_column = byte_size(left) left = left @@ -187,7 +187,7 @@ defmodule Livebook.Intellisense do |> consume_until(@space ++ @operators ++ @punctuation, @closing_identifier) |> List.to_string() - {bytes_until_index - byte_size(left), bytes_until_index + byte_size(right)} + {bytes_until_column - byte_size(left), bytes_until_column + byte_size(right)} end defp consume_until(acc \\ [], chars, stop, stop_include) diff --git a/lib/livebook/js_interop.ex b/lib/livebook/js_interop.ex index a9e98eeef..e8d027ac1 100644 --- a/lib/livebook/js_interop.ex +++ b/lib/livebook/js_interop.ex @@ -40,6 +40,21 @@ defmodule Livebook.JSInterop do apply_to_code_units(ops, Enum.slice(code_units, n..-1)) end + @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. + """ + @spec convert_column_to_elixir(pos_integer(), String.t()) :: pos_integer() + def convert_column_to_elixir(column, line) do + line + |> string_to_utf16_code_units() + |> Enum.take(column - 1) + |> utf16_code_units_to_string() + |> String.length() + |> Kernel.+(1) + end + # --- defp string_to_utf16_code_units(string) do diff --git a/lib/livebook/runtime.ex b/lib/livebook/runtime.ex index 12e7255c9..bb31f16cf 100644 --- a/lib/livebook/runtime.ex +++ b/lib/livebook/runtime.ex @@ -72,9 +72,9 @@ defprotocol Livebook.Runtime do @type completion_item_kind :: :function | :module | :type | :variable | :field @typedoc """ - Looks up more details about an identifier found at `index` in `line`. + Looks up more details about an identifier found in `column` in `line`. """ - @type details_request :: {:details, line :: String.t(), index :: non_neg_integer()} + @type details_request :: {:details, line :: String.t(), column :: pos_integer()} @type details_response :: %{ range: %{ diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index 1ae738ae9..e0b858c28 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -632,8 +632,9 @@ defmodule LivebookWeb.SessionLive do %{"type" => "completion", "hint" => hint} -> {:completion, hint} - %{"type" => "details", "line" => line, "index" => index} -> - {:details, line, index} + %{"type" => "details", "line" => line, "column" => column} -> + column = Livebook.JSInterop.convert_column_to_elixir(column, line) + {:details, line, column} %{"type" => "format", "code" => code} -> {:format, code} diff --git a/test/livebook/js_interop_test.exs b/test/livebook/js_interop_test.exs index 20465fc6f..2f4772e1d 100644 --- a/test/livebook/js_interop_test.exs +++ b/test/livebook/js_interop_test.exs @@ -43,4 +43,27 @@ defmodule Livebook.JSInteropTest do assert JSInterop.apply_delta_to_string(delta, string) == " cats" end end + + describe "convert_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 + 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 + end + + test "returns proper column if a middle UTF-16 code unit is given" do + # 🚀 consists of 2 UTF-16 code units, so JavaScript assumes "🚀".length is 2 + # 3th and 4th code unit correspond to the second 🚀 + column = 3 + line = "🚀🚀 String.replace" + assert JSInterop.convert_column_to_elixir(column, line) == 2 + end + end end