diff --git a/apps/remote_control/lib/lexical/remote_control/build.ex b/apps/remote_control/lib/lexical/remote_control/build.ex index 3f80812c6..0137a3527 100644 --- a/apps/remote_control/lib/lexical/remote_control/build.ex +++ b/apps/remote_control/lib/lexical/remote_control/build.ex @@ -9,7 +9,7 @@ defmodule Lexical.RemoteControl.Build do require Logger use GenServer - @tick_interval_millis 50 + @timeout_interval_millis 50 # Public interface @@ -67,32 +67,30 @@ defmodule Lexical.RemoteControl.Build do @impl GenServer def handle_continue(:ensure_build_directory, %State{} = state) do State.ensure_build_directory(state) - schedule_tick() {:noreply, state} end @impl GenServer def handle_call({:force_compile_file, %Document{} = document}, _from, %State{} = state) do State.compile_file(state, document) - {:reply, :ok, state} + {:reply, :ok, state, @timeout_interval_millis} end @impl GenServer def handle_cast({:compile, force?}, %State{} = state) do - State.compile_project(state, force?) - {:noreply, state} + new_state = State.on_project_compile(state, force?) + {:noreply, new_state, @timeout_interval_millis} end @impl GenServer def handle_cast({:compile_file, %Document{} = document}, %State{} = state) do new_state = State.on_file_compile(state, document) - {:noreply, new_state} + {:noreply, new_state, @timeout_interval_millis} end @impl GenServer - def handle_info(:tick, %State{} = state) do - new_state = State.on_tick(state) - schedule_tick() + def handle_info(:timeout, %State{} = state) do + new_state = State.on_timeout(state) {:noreply, new_state} end @@ -101,8 +99,4 @@ defmodule Lexical.RemoteControl.Build do Logger.warning("Undefined message: #{inspect(msg)}") {:noreply, project} end - - defp schedule_tick do - Process.send_after(self(), :tick, @tick_interval_millis) - end end diff --git a/apps/remote_control/lib/lexical/remote_control/build/state.ex b/apps/remote_control/lib/lexical/remote_control/build/state.ex index 44883068b..baa7aa35a 100644 --- a/apps/remote_control/lib/lexical/remote_control/build/state.ex +++ b/apps/remote_control/lib/lexical/remote_control/build/state.ex @@ -14,33 +14,47 @@ defmodule Lexical.RemoteControl.Build.State do use RemoteControl.Progress - defstruct project: nil, build_number: 0, uri_to_source_and_edit_time: %{} + defstruct project: nil, + build_number: 0, + uri_to_document: %{}, + project_compile: :none def new(%Project{} = project) do %__MODULE__{project: project} end - def on_tick(%__MODULE__{} = state) do - {new_state, compiled_uris} = - Enum.reduce(state.uri_to_source_and_edit_time, {state, []}, fn - {uri, {document, edit_time}}, {state, compiled_uris} -> - if should_compile?(edit_time) do - new_state = increment_build_number(state) - compile_file(new_state, document) - {new_state, [uri | compiled_uris]} - else - {state, compiled_uris} - end + def on_timeout(%__MODULE__{} = state) do + new_state = + case state.project_compile do + :none -> state + :force -> compile_project(state, true) + :normal -> compile_project(state, false) + end + + # We need to compile the individual documents even after the project is + # compiled because they might have unsaved changes, and we want that state + # to be the latest state of the project. + new_state = + Enum.reduce(new_state.uri_to_document, state, fn {_uri, document}, state -> + compile_file(state, document) end) + %__MODULE__{new_state | uri_to_document: %{}, project_compile: :none} + end + + def on_file_compile(%__MODULE__{} = state, %Document{} = document) do %__MODULE__{ - new_state - | uri_to_source_and_edit_time: Map.drop(state.uri_to_source_and_edit_time, compiled_uris) + state + | uri_to_document: Map.put(state.uri_to_document, document.uri, document) } end - def compile_scheduled?(%__MODULE__{} = state, uri) do - Map.has_key?(state.uri_to_source_and_edit_time, uri) + def on_project_compile(%__MODULE__{} = state, force?) do + if force? do + %__MODULE__{state | project_compile: :force} + else + %__MODULE__{state | project_compile: :normal} + end end def ensure_build_directory(%__MODULE__{} = state) do @@ -65,7 +79,7 @@ defmodule Lexical.RemoteControl.Build.State do end end - def compile_project(%__MODULE__{} = state, initial?) do + defp compile_project(%__MODULE__{} = state, initial?) do state = increment_build_number(state) project = state.project @@ -106,17 +120,12 @@ defmodule Lexical.RemoteControl.Build.State do RemoteControl.broadcast(diagnostics_message) Plugin.diagnose(project, state.build_number) end) - end - def on_file_compile(%__MODULE__{} = state, %Document{} = document) do - %__MODULE__{ - state - | uri_to_source_and_edit_time: - Map.put(state.uri_to_source_and_edit_time, document.uri, {document, now()}) - } + state end def compile_file(%__MODULE__{} = state, %Document{} = document) do + state = increment_build_number(state) project = state.project Build.with_lock(fn -> @@ -169,6 +178,8 @@ defmodule Lexical.RemoteControl.Build.State do RemoteControl.broadcast(diagnostics) Plugin.diagnose(project, state.build_number, document) end) + + state end def set_compiler_options do @@ -201,15 +212,6 @@ defmodule Lexical.RemoteControl.Build.State do "Building #{Project.display_name(project)}" end - defp now do - System.system_time(:millisecond) - end - - defp should_compile?(last_edit_time) do - millis_since_last_edit = now() - last_edit_time - millis_since_last_edit >= edit_window_millis() - end - defp to_ms(microseconds) do microseconds / 1000 end @@ -218,10 +220,6 @@ defmodule Lexical.RemoteControl.Build.State do [columns: true, token_metadata: true] end - defp edit_window_millis do - Application.get_env(:remote_control, :edit_window_millis, 250) - end - defp increment_build_number(%__MODULE__{} = state) do %__MODULE__{state | build_number: state.build_number + 1} end diff --git a/apps/remote_control/test/lexical/remote_control/build/state_test.exs b/apps/remote_control/test/lexical/remote_control/build/state_test.exs index f838ec52e..d4894e9b8 100644 --- a/apps/remote_control/test/lexical/remote_control/build/state_test.exs +++ b/apps/remote_control/test/lexical/remote_control/build/state_test.exs @@ -6,7 +6,6 @@ defmodule Lexical.RemoteControl.Build.StateTest do alias Lexical.RemoteControl.Build.State alias Lexical.RemoteControl.Plugin - import Lexical.Test.EventualAssertions import Lexical.Test.Fixtures use ExUnit.Case, async: false @@ -67,22 +66,88 @@ defmodule Lexical.RemoteControl.Build.StateTest do {:ok, document: document} end - describe "throttled compilation" do - setup [:with_metadata_project, :with_a_valid_document] + def with_patched_compilation(_) do + patch(Build.Document, :compile, :ok) + patch(Build.Project, :compile, :ok) + :ok + end + + describe "throttled document compilation" do + setup [:with_metadata_project, :with_a_valid_document, :with_patched_compilation] test "it doesn't compile immediately", %{state: state, document: document} do - new_state = - state - |> State.on_file_compile(document) - |> State.on_tick() + State.on_file_compile(state, document) + + refute_called(Build.Document.compile(document)) + refute_called(Build.Project.compile(_, _)) + end + + test "it compiles files when on_timeout is called", %{state: state, document: document} do + state + |> State.on_file_compile(document) + |> State.on_timeout() + + assert_called(Build.Document.compile(document)) + refute_called(Build.Project.compile(_, _)) + end + end + + describe "throttled project compilation" do + setup [:with_metadata_project, :with_a_valid_document, :with_patched_compilation] + + test "doesn't compile immediately if forced", %{state: state} do + State.on_project_compile(state, true) + refute_called(Build.Project.compile(_, _)) + end + + test "doesn't compile immediately", %{state: state} do + State.on_project_compile(state, false) + refute_called(Build.Project.compile(_, _)) + end - assert State.compile_scheduled?(new_state, document.uri) + test "compiles if force is true after on_timeout is called", %{state: state} do + state + |> State.on_project_compile(true) + |> State.on_timeout() + + assert_called(Build.Project.compile(_, true)) end - test "it compiles after a timeout", %{state: state, document: document} do - state = State.on_file_compile(state, document) + test "compiles after on_timeout is called", %{state: state} do + state + |> State.on_project_compile(false) + |> State.on_timeout() + + assert_called(Build.Project.compile(_, false)) + end + end + + describe "mixed compilation" do + setup [:with_metadata_project, :with_a_valid_document, :with_patched_compilation] + + test "doesn't compile if both documents and projects are added", %{ + state: state, + document: document + } do + state + |> State.on_project_compile(false) + |> State.on_file_compile(document) + + refute_called(Build.Document.compile(_)) + refute_called(Build.Project.compile(_, _)) + end - refute_eventually(State.compile_scheduled?(State.on_tick(state), document.uri), 500) + test "compiles when on_timeout is called if both documents and projects are added", %{ + state: state, + document: document + } do + state + |> State.on_project_compile(false) + |> State.on_file_compile(document) + |> State.on_timeout() + + assert_called(Build.Document.compile(_)) + assert_called(Build.Project.compile(_, _)) end end end diff --git a/apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex b/apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex index 71e18923b..e6251213e 100644 --- a/apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex +++ b/apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex @@ -41,12 +41,16 @@ defimpl Lexical.Convertible, for: Lexical.Plugin.V1.Diagnostic.Result do Conversions.to_lsp(range) end - defp lsp_range(%Diagnostic.Result{} = diagnostic) do - with {:ok, document} <- Document.Store.open_temporary(diagnostic.uri) do + defp lsp_range(%Diagnostic.Result{uri: uri} = diagnostic) when is_binary(uri) do + with {:ok, document} <- Document.Store.open_temporary(uri) do position_to_range(document, diagnostic.position) end end + defp lsp_range(%Diagnostic.Result{}) do + {:error, :no_uri} + end + defp position_to_range(%Document{} = document, {start_line, start_column, end_line, end_column}) do start_pos = Position.new(document, start_line, max(start_column, 1)) end_pos = Position.new(document, end_line, max(end_column, 1)) diff --git a/projects/lexical_shared/lib/lexical/document/store.ex b/projects/lexical_shared/lib/lexical/document/store.ex index f438859e9..d305ba35d 100644 --- a/projects/lexical_shared/lib/lexical/document/store.ex +++ b/projects/lexical_shared/lib/lexical/document/store.ex @@ -268,7 +268,7 @@ defmodule Lexical.Document.Store do @spec open_temporary(Lexical.uri() | Path.t(), timeout()) :: {:ok, Document.t()} | {:error, term()} - def open_temporary(uri, timeout \\ 5000) do + def open_temporary(uri, timeout \\ 5000) when is_binary(uri) do ProcessCache.trans(uri, 50, fn -> GenServer.call(name(), {:open_temporarily, uri, timeout}) end)