Skip to content

Conversation

doorgan
Copy link
Collaborator

@doorgan doorgan commented Jun 4, 2025

This change replaces the use of Lexical.Protocol data structures for the GenLSP counterparts, and handles conversion between those and the internal data structures for ranges, positions and locations. Since neither Proto nor Protocol are really needed after this change, I also removed them altogether and consolidated the few modules we do need for them into the Forge application.

This PR needs GenLSP support for cancellation before it can be reviewed/merged

Besides cleanups in general and removal of dead code, currently there are a few issues I'm working on:

  • On neovim, completions for module segments after the . not always show, but they are recognized once you start typing after the dot fixed
  • On VSCode, code actions are not being recognized(but they are and work properly on neovim) fixed
  • On neovim, workspace symbols don't show up but they do show up in VSCode happens on main too
  • On tests, diagnostics aren't being cleaned up after a document is marked non-dirty

Other than that, expert using this pr is functional

@doorgan doorgan force-pushed the doorgan/gen_lsp_datastructures branch from 5644547 to e4cb2d7 Compare June 5, 2025 16:38
@doorgan doorgan changed the base branch from main to doorgan/rename_to_expert June 5, 2025 16:38
@mhanberg
Copy link
Member

mhanberg commented Jun 5, 2025

You're a beast

doorgan added 6 commits June 5, 2025 16:13
Don't be sad because they're gone, be happy because they were here
I'm not familiar with the background for this regression test so I'm just
making it pass. It is very likely this change doesn't test the same thing
it tested before, but I can't tell
Comment on lines 39 to 40
range: root.range,
selection_range: root.detail_range
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@doorgan looks like this is still a Forge range, needs to be converted to a GenLSP.Structures.Range

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is converted here:

def write(io_device, %_{} = payload) do
with {:ok, lsp} <- encode(payload),
{:ok, json} <- Jason.encode(lsp) do
write(io_device, json)
end
end
def write(io_device, %{} = payload) do
with {:ok, encoded} <- Jason.encode(payload) do
write(io_device, encoded)
end
end
def write(io_device, payload) when is_binary(payload) do
message =
case io_device do
device when device in [:stdio, :standard_io] or is_pid(device) ->
{:ok, json_rpc} = JsonRpc.encode(payload)
json_rpc
_ ->
payload
end
IO.binwrite(io_device, message)
end
def write(_, nil) do
end
def write(_, []) do
end
defp encode(%{id: id, result: result}) do
with {:ok, result} <- dump_lsp(result) do
{:ok,
%{
"jsonrpc" => "2.0",
"id" => id,
"result" => result
}}
end
end
defp encode(%{id: id, error: error}) do
with {:ok, error} <- dump_lsp(error) do
{:ok,
%{
"jsonrpc" => "2.0",
"id" => id,
"error" => error
}}
end
end
defp encode(%_{} = request) do
dump_lsp(request)
end
defp dump_lsp(%module{} = item) do
with {:ok, item} <- Convert.to_lsp(item),
{:ok, item} <- Schematic.dump(module.schematic(), item) do
{:ok, item}
end
end
defp dump_lsp(list) when is_list(list) do
dump =
Enum.map(list, fn item ->
case dump_lsp(item) do
{:ok, dumped} -> dumped
{:error, reason} -> throw({:error, reason})
end
end)
{:ok, dump}
catch
{:error, reason} ->
{:error, reason}
end
defp dump_lsp(other), do: {:ok, other}

Conversely, they are converted from genlsp to forge here:

{:ok, request} <- Convert.to_native(request) do
# Logger.info("Handling request: #{inspect(request, pretty: true)}")

I think we'd need to do what I'm doing in std_io somewhere in expert.ex when using genlsp

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was about to comment back, I realized I deleted the code that did the conversion.

I'll get it converted again

@doorgan doorgan mentioned this pull request Jun 18, 2025
Base automatically changed from doorgan/rename_to_expert to main June 18, 2025 14:03
@doorgan doorgan changed the title [draft] Use GenLSP data structures Use GenLSP data structures Jun 18, 2025
doorgan added 4 commits June 18, 2025 11:31
The tests read "on compile" but the test code was not simulating
a compilation, causing the diagnostics to not be pruned
@doorgan doorgan force-pushed the doorgan/gen_lsp_datastructures branch from 21ada8c to 6813b52 Compare June 18, 2025 15:36
@doorgan
Copy link
Collaborator Author

doorgan commented Jun 18, 2025

Been using this branch for a while with no issues, so I'm merging it

@doorgan doorgan merged commit c081bf9 into main Jun 18, 2025
12 checks passed
@doorgan doorgan deleted the doorgan/gen_lsp_datastructures branch June 18, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants