Skip to content

Commit 93c196f

Browse files
committed
Improve LSP consistency
position should trim to the end of the line
1 parent edb2e4d commit 93c196f

File tree

5 files changed

+55
-16
lines changed

5 files changed

+55
-16
lines changed

apps/elixir_ls_debugger/lib/debugger/utils.ex

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,6 @@ defmodule ElixirLS.Debugger.Utils do
3434

3535
byte_size = byte_size(utf16_line)
3636

37-
# if character index is over the length of the string assume we pad it with spaces (1 byte in utf8)
38-
diff = div(max(dap_character * 2 - byte_size, 0), 2)
39-
4037
utf8_character =
4138
utf16_line
4239
|> (&binary_part(
@@ -47,6 +44,6 @@ defmodule ElixirLS.Debugger.Utils do
4744
|> characters_to_binary!(:utf16, :utf8)
4845
|> String.length()
4946

50-
utf8_character + diff
47+
utf8_character
5148
end
5249
end

apps/elixir_ls_debugger/test/utils_test.exs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ defmodule ElixirLS.Debugger.UtilsTest do
3939
assert 0 == Utils.dap_character_to_elixir("", 0)
4040
end
4141

42+
test "dap_character_to_elixir empty after end" do
43+
assert 0 == Utils.dap_character_to_elixir("", 1)
44+
end
45+
4246
test "dap_character_to_elixir first char" do
4347
assert 0 == Utils.dap_character_to_elixir("abcde", 0)
4448
end
@@ -47,6 +51,14 @@ defmodule ElixirLS.Debugger.UtilsTest do
4751
assert 1 == Utils.dap_character_to_elixir("abcde", 1)
4852
end
4953

54+
test "dap_character_to_elixir before line start" do
55+
assert 0 == Utils.dap_character_to_elixir("abcde", -1)
56+
end
57+
58+
test "dap_character_to_elixir after line end" do
59+
assert 5 == Utils.dap_character_to_elixir("abcde", 15)
60+
end
61+
5062
test "dap_character_to_elixir utf8" do
5163
assert 1 == Utils.dap_character_to_elixir("🏳️‍🌈abcde", 6)
5264
end

apps/language_server/lib/language_server/source_file.ex

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -283,9 +283,6 @@ defmodule ElixirLS.LanguageServer.SourceFile do
283283

284284
byte_size = byte_size(utf16_line)
285285

286-
# if character index is over the length of the string assume we pad it with spaces (1 byte in utf8)
287-
diff = div(max(lsp_character * 2 - byte_size, 0), 2)
288-
289286
utf8_character =
290287
utf16_line
291288
|> (&binary_part(
@@ -296,19 +293,30 @@ defmodule ElixirLS.LanguageServer.SourceFile do
296293
|> characters_to_binary!(:utf16, :utf8)
297294
|> String.length()
298295

299-
utf8_character + 1 + diff
296+
utf8_character + 1
300297
end
301298

299+
def lsp_position_to_elixir(_urf8_text, {lsp_line, _lsp_character}) when lsp_line < 0,
300+
do: {1, 1}
301+
302302
def lsp_position_to_elixir(_urf8_text, {lsp_line, lsp_character}) when lsp_character <= 0,
303303
do: {max(lsp_line + 1, 1), 1}
304304

305305
def lsp_position_to_elixir(urf8_text, {lsp_line, lsp_character}) do
306-
utf8_character =
307-
lines(urf8_text)
308-
|> Enum.at(max(lsp_line, 0))
309-
|> lsp_character_to_elixir(lsp_character)
310-
311-
{lsp_line + 1, utf8_character}
306+
source_file_lines = lines(urf8_text)
307+
total_lines = length(source_file_lines)
308+
309+
if lsp_line > total_lines - 1 do
310+
# sanitize to position after last char in last line
311+
{total_lines, String.length(source_file_lines |> Enum.at(total_lines - 1)) + 1}
312+
else
313+
utf8_character =
314+
source_file_lines
315+
|> Enum.at(lsp_line)
316+
|> lsp_character_to_elixir(lsp_character)
317+
318+
{lsp_line + 1, utf8_character}
319+
end
312320
end
313321

314322
def elixir_character_to_lsp(_utf8_line, elixir_character) when elixir_character <= 1, do: 0

apps/language_server/test/providers/completion_test.exs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ defmodule ElixirLS.LanguageServer.Providers.CompletionTest do
476476
defstruct [some: nil, other: 1]
477477
478478
def dummy_function(var = %MyModule{}) do
479-
%{var |
479+
%{var |
480480
# ^
481481
end
482482
end
@@ -496,7 +496,7 @@ defmodule ElixirLS.LanguageServer.Providers.CompletionTest do
496496
text = """
497497
defmodule MyModule do
498498
def dummy_function(var = %{some: nil, other: 1}) do
499-
%{var |
499+
%{var |
500500
# ^
501501
end
502502
end

apps/language_server/test/source_file_test.exs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,18 @@ defmodule ElixirLS.LanguageServer.SourceFileTest do
655655
assert {1, 2} == SourceFile.lsp_position_to_elixir("abcde", {0, 1})
656656
end
657657

658+
# This is not specified in LSP but some clients fail to synchronize text properly
659+
test "lsp_position_to_elixir single line before line start" do
660+
assert {1, 1} == SourceFile.lsp_position_to_elixir("abcde", {0, -1})
661+
end
662+
663+
# LSP spec 3.17 https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#position
664+
# position character If the character value is greater than the line length it defaults back to the line length
665+
test "lsp_position_to_elixir single line after line end" do
666+
assert {1, 6} == SourceFile.lsp_position_to_elixir("abcde", {0, 15})
667+
assert {1, 1} == SourceFile.lsp_position_to_elixir("", {0, 15})
668+
end
669+
658670
test "lsp_position_to_elixir single line utf8" do
659671
assert {1, 2} == SourceFile.lsp_position_to_elixir("🏳️‍🌈abcde", {0, 6})
660672
end
@@ -663,6 +675,16 @@ defmodule ElixirLS.LanguageServer.SourceFileTest do
663675
assert {2, 2} == SourceFile.lsp_position_to_elixir("abcde\n1234", {1, 1})
664676
end
665677

678+
# This is not specified in LSP but some clients fail to synchronize text properly
679+
test "lsp_position_to_elixir multi line before first line" do
680+
assert {1, 1} == SourceFile.lsp_position_to_elixir("abcde\n1234", {-1, 2})
681+
end
682+
683+
# This is not specified in LSP but some clients fail to synchronize text properly
684+
test "lsp_position_to_elixir multi line after last line" do
685+
assert {2, 5} == SourceFile.lsp_position_to_elixir("abcde\n1234", {8, 2})
686+
end
687+
666688
test "elixir_position_to_lsp empty" do
667689
assert {0, 0} == SourceFile.elixir_position_to_lsp("", {1, 1})
668690
end

0 commit comments

Comments
 (0)