Skip to content

Commit 2db8acc

Browse files
lukaszsamsonlukaszsamson
committed
improve selection ranges in document symbols
never return selection ranges outside ranges
1 parent a5fd8ec commit 2db8acc

File tree

2 files changed

+83
-57
lines changed

2 files changed

+83
-57
lines changed

‎apps/language_server/lib/language_server/providers/document_symbols.ex

Lines changed: 66 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
1010
require ElixirLS.LanguageServer.Protocol, as: Protocol
1111

1212
defmodule Info do
13-
defstruct [:type, :name, :location, :children, :selection_location]
13+
defstruct [:type, :name, :location, :children, :selection_location, :symbol]
1414
end
1515

1616
@defs [:def, :defp, :defmacro, :defmacrop, :defguard, :defguardp, :defdelegate]
@@ -92,6 +92,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
9292
end
9393

9494
# TODO extract module name location from Code.Fragment.surround_context?
95+
# TODO better selection ranges for defimpl?
9596
{extract_module_name(module_expression), module_name_location, module_body}
9697
end
9798

@@ -117,7 +118,8 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
117118
name: module_name,
118119
location: location,
119120
selection_location: module_name_location,
120-
children: module_symbols
121+
children: module_symbols,
122+
symbol: module_name
121123
}
122124
end
123125

@@ -175,12 +177,13 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
175177
|> String.replace("\n", "")
176178

177179
type = if type_kind in [:type, :typep, :opaque], do: :class, else: :event
178-
# TODO no closing metdata in type expressions
180+
179181
%Info{
180182
type: type,
181183
name: "@#{type_kind} #{type_name}",
182184
location: location,
183185
selection_location: type_head_location,
186+
symbol: "#{type_name}",
184187
children: []
185188
}
186189
end
@@ -207,6 +210,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
207210

208211
%Info{
209212
type: :function,
213+
symbol: "#{name}",
210214
name: "#{defname} #{name}",
211215
location: location,
212216
selection_location: head_location,
@@ -221,6 +225,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
221225

222226
%Info{
223227
type: :function,
228+
symbol: "#{name}",
224229
name: "#{defname} #{name}",
225230
location: location,
226231
selection_location: head_location,
@@ -324,15 +329,64 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
324329
do: Enum.map(info, &build_symbol_information_hierarchical(uri, text, &1))
325330

326331
defp build_symbol_information_hierarchical(uri, text, %Info{} = info) do
332+
selection_range =
333+
location_to_range(info.selection_location || info.location, text, info.symbol)
334+
335+
# range must contain selection range
336+
range =
337+
location_to_range(info.location, text, nil)
338+
|> maybe_extend_range(selection_range)
339+
327340
%Protocol.DocumentSymbol{
328341
name: info.name,
329342
kind: SymbolUtils.symbol_kind_to_code(info.type),
330-
range: location_to_range(info.location, text),
331-
selectionRange: location_to_range(info.selection_location || info.location, text),
343+
range: range,
344+
selectionRange: selection_range,
332345
children: build_symbol_information_hierarchical(uri, text, info.children)
333346
}
334347
end
335348

349+
defp maybe_extend_range(
350+
Protocol.range(start_line, start_character, end_line, end_character),
351+
Protocol.range(
352+
selection_start_line,
353+
selection_start_character,
354+
selection_end_line,
355+
selection_end_character
356+
)
357+
) do
358+
{extended_start_line, extended_start_character} =
359+
cond do
360+
selection_start_line < start_line ->
361+
{selection_start_line, selection_start_character}
362+
363+
selection_start_line == start_line ->
364+
{selection_start_line, min(selection_start_character, start_character)}
365+
366+
true ->
367+
{start_line, start_character}
368+
end
369+
370+
{extended_end_line, extended_end_character} =
371+
cond do
372+
selection_end_line > end_line ->
373+
{selection_end_line, selection_end_character}
374+
375+
selection_end_line == end_line ->
376+
{selection_end_line, max(selection_end_character, end_character)}
377+
378+
true ->
379+
{end_line, end_character}
380+
end
381+
382+
Protocol.range(
383+
extended_start_line,
384+
extended_start_character,
385+
extended_end_line,
386+
extended_end_character
387+
)
388+
end
389+
336390
defp build_symbol_information_flat(uri, text, info, parent_name \\ nil)
337391

338392
defp build_symbol_information_flat(uri, text, info, parent_name) when is_list(info),
@@ -347,7 +401,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
347401
kind: SymbolUtils.symbol_kind_to_code(info.type),
348402
location: %{
349403
uri: uri,
350-
range: location_to_range(info.location, text)
404+
range: location_to_range(info.location, text, nil)
351405
},
352406
containerName: parent_name
353407
}
@@ -360,14 +414,14 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
360414
kind: SymbolUtils.symbol_kind_to_code(info.type),
361415
location: %{
362416
uri: uri,
363-
range: location_to_range(info.location, text)
417+
range: location_to_range(info.location, text, nil)
364418
},
365419
containerName: parent_name
366420
}
367421
end
368422
end
369423

370-
defp location_to_range(location, text) do
424+
defp location_to_range(location, text, symbol) do
371425
{start_line, start_character} =
372426
SourceFile.elixir_position_to_lsp(text, {location[:line], location[:column]})
373427

@@ -389,6 +443,10 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
389443
{end_location[:line], end_location[:column] + 1}
390444
)
391445

446+
symbol != nil ->
447+
end_char = SourceFile.elixir_character_to_lsp(symbol, String.length(symbol))
448+
{start_line, start_character + end_char + 1}
449+
392450
true ->
393451
{start_line, start_character}
394452
end

‎apps/language_server/test/providers/document_symbols_test.exs

Lines changed: 17 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
133133
"start" => %{"character" => 8, "line" => 11}
134134
},
135135
selectionRange: %{
136-
"end" => %{"character" => 12, "line" => 11},
136+
"end" => %{"character" => 24, "line" => 11},
137137
"start" => %{"character" => 12, "line" => 11}
138138
}
139139
},
@@ -202,7 +202,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
202202
"start" => %{"character" => 6, "line" => 1}
203203
},
204204
selectionRange: %{
205-
"end" => %{"character" => 16, "line" => 1},
205+
"end" => %{"character" => 24, "line" => 1},
206206
"start" => %{"character" => 16, "line" => 1}
207207
}
208208
}
@@ -347,7 +347,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
347347
uri = "file:///project/file.ex"
348348
text = ~S[
349349
defmodule MyModule do
350-
defmodule SubModule do
350+
defmodule Sub.Module do
351351
def my_fn(), do: :ok
352352
end
353353
end
@@ -366,13 +366,13 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
366366
}
367367
],
368368
kind: 2,
369-
name: "SubModule",
369+
name: "Sub.Module",
370370
range: %{
371371
"end" => %{"character" => 11, "line" => 4},
372372
"start" => %{"character" => 8, "line" => 2}
373373
},
374374
selectionRange: %{
375-
"end" => %{"character" => 18, "line" => 2},
375+
"end" => %{"character" => 28, "line" => 2},
376376
"start" => %{"character" => 18, "line" => 2}
377377
}
378378
}
@@ -384,7 +384,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
384384
"start" => %{"character" => 6, "line" => 1}
385385
},
386386
selectionRange: %{
387-
"end" => %{"character" => 16, "line" => 1},
387+
"end" => %{"character" => 24, "line" => 1},
388388
"start" => %{"character" => 16, "line" => 1}
389389
}
390390
}
@@ -460,7 +460,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
460460
"start" => %{"character" => 6, "line" => 1}
461461
},
462462
selectionRange: %{
463-
"end" => %{"character" => 16, "line" => 1},
463+
"end" => %{"character" => 24, "line" => 1},
464464
"start" => %{"character" => 16, "line" => 1}
465465
}
466466
},
@@ -479,7 +479,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
479479
"start" => %{"character" => 6, "line" => 4}
480480
},
481481
selectionRange: %{
482-
"end" => %{"character" => 16, "line" => 4},
482+
"end" => %{"character" => 29, "line" => 4},
483483
"start" => %{"character" => 16, "line" => 4}
484484
}
485485
}
@@ -800,7 +800,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
800800
kind: 12,
801801
name: "def size(data)",
802802
range: %{
803-
"end" => %{"character" => 2, "line" => 2},
803+
"end" => %{"character" => 16, "line" => 2},
804804
"start" => %{"character" => 2, "line" => 2}
805805
},
806806
selectionRange: %{
@@ -816,7 +816,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
816816
"start" => %{"character" => 0, "line" => 0}
817817
},
818818
selectionRange: %{
819-
"end" => %{"character" => 12, "line" => 0},
819+
"end" => %{"character" => 22, "line" => 0},
820820
"start" => %{"character" => 12, "line" => 0}
821821
}
822822
},
@@ -827,7 +827,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
827827
kind: 12,
828828
name: "def size(binary)",
829829
range: %{
830-
"end" => %{"character" => 2, "line" => 6},
830+
"end" => %{"character" => 18, "line" => 6},
831831
"start" => %{"character" => 2, "line" => 6}
832832
},
833833
selectionRange: %{
@@ -854,7 +854,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
854854
kind: 12,
855855
name: "def size(param)",
856856
range: %{
857-
"end" => %{"character" => 2, "line" => 10},
857+
"end" => %{"character" => 17, "line" => 10},
858858
"start" => %{"character" => 2, "line" => 10}
859859
},
860860
selectionRange: %{
@@ -1188,7 +1188,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
11881188
"start" => %{"character" => 2, "line" => 1}
11891189
},
11901190
selectionRange: %{
1191-
"end" => %{"character" => 8, "line" => 1},
1191+
"end" => %{"character" => 17, "line" => 1},
11921192
"start" => %{"character" => 8, "line" => 1}
11931193
}
11941194
},
@@ -1879,15 +1879,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
18791879
}
18801880
],
18811881
kind: 2,
1882-
name: "MyModuleTest",
1883-
range: %{
1884-
"end" => %{"character" => 9, "line" => 4},
1885-
"start" => %{"character" => 6, "line" => 1}
1886-
},
1887-
selectionRange: %{
1888-
"end" => %{"character" => 16, "line" => 1},
1889-
"start" => %{"character" => 16, "line" => 1}
1890-
}
1882+
name: "MyModuleTest"
18911883
}
18921884
]} = DocumentSymbols.symbols(uri, text, true)
18931885
end
@@ -1971,15 +1963,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
19711963
}
19721964
],
19731965
kind: 2,
1974-
name: "MyModuleTest",
1975-
range: %{
1976-
"end" => %{"character" => 9, "line" => 6},
1977-
"start" => %{"character" => 6, "line" => 1}
1978-
},
1979-
selectionRange: %{
1980-
"end" => %{"character" => 16, "line" => 1},
1981-
"start" => %{"character" => 16, "line" => 1}
1982-
}
1966+
name: "MyModuleTest"
19831967
}
19841968
]} = DocumentSymbols.symbols(uri, text, true)
19851969
end
@@ -2028,15 +2012,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
20282012
}
20292013
],
20302014
kind: 2,
2031-
name: "MyModuleTest",
2032-
range: %{
2033-
"end" => %{"character" => 9, "line" => 6},
2034-
"start" => %{"character" => 6, "line" => 1}
2035-
},
2036-
selectionRange: %{
2037-
"end" => %{"character" => 16, "line" => 1},
2038-
"start" => %{"character" => 16, "line" => 1}
2039-
}
2015+
name: "MyModuleTest"
20402016
}
20412017
]} = DocumentSymbols.symbols(uri, text, true)
20422018

@@ -2202,15 +2178,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
22022178
}
22032179
],
22042180
kind: 2,
2205-
name: "MyModuleTest",
2206-
range: %{
2207-
"end" => %{"character" => 3, "line" => 9},
2208-
"start" => %{"character" => 0, "line" => 0}
2209-
},
2210-
selectionRange: %{
2211-
"end" => %{"character" => 10, "line" => 0},
2212-
"start" => %{"character" => 10, "line" => 0}
2213-
}
2181+
name: "MyModuleTest"
22142182
}
22152183
]} = DocumentSymbols.symbols(uri, text, true)
22162184
end

0 commit comments

Comments
 (0)