erlang_ls icon indicating copy to clipboard operation
erlang_ls copied to clipboard

Export snippet removes dash

Open eproxus opened this issue 3 years ago • 9 comments

Describe the bug I'm not 100% this is a bug in erlang_ls, it could be a combination of Sublime, Sublime LSP and erlang_ls. However, it only happens when erlang_ls is enabled.

When autocompleting export attributes, the leading dash is removed:

% Using erlang_ls
-exp%<TAB>
% Leads to
export([]).

% Without erlang_ls
-exp%<TAB>
% Leads to
--export([function/arity]).
% (The double dash is inserted by Sublime's default snippet, which expects 'exp' to be typed without the dash)

To Reproduce Press TAB after having typed -exp on a new line.

Expected behavior The export statement is completed to -export([]).

Actual behavior The export statement is completed to export([]).

Context

  • erlang_ls version (tag/sha): 0.17.0
  • Editor used: Sublime Text 4 (4109)
  • LSP client used: Sublime LSP v1.6.1

eproxus avatar Jul 01 '21 11:07 eproxus

I can't reproduce in Emacs with lsp-mode, so this might be an issue that needs to be reported to Sublime LSP.

It is true that the snippet it completes to is export([${1:}])., but erlang_ls have defined "-" as a trigger character and thus it should be kept by the client. Does this ONLY concern the -export attribute or any other attributes too?

Would be interesting to hear if it can be reproduced in other editors?

plux avatar Jul 01 '21 11:07 plux

@plux Yes, all attributes it seems.

eproxus avatar Jul 01 '21 15:07 eproxus

I submitted issue in Sublime-LSP, it is interesting to see what they think.

Would be interesting to hear if it can be reproduced in other editors?

quoting myself from that issue

I checked vscode, it keeps leading -. I asked about other editors and got the only reply that eglot in emacs also keeps leading -, but I can't treat it as a viable proof.

nwalker avatar Aug 25 '21 22:08 nwalker

https://github.com/sublimelsp/LSP/issues/1833

eproxus avatar Aug 26 '21 07:08 eproxus

I'm wondering if anyone had some more time to look at this? There seem to be a hint in the Sublime LSP thread that this could be fixed in the Erlang language server configuration (using filterText). Alternatively if anyone has a hint of where to start changing/debugging this suggestion, I could try myself (unfortunately I have no idea where to start...).

eproxus avatar Dec 07 '21 15:12 eproxus

Hi @eproxus , I simply had no time for looking into this (and I missed the comment on the Sublime LSP thread). That may be worth a try. It's unlikely for me to tackle this before a couple of weeks, though.

robertoaloi avatar Dec 09 '21 21:12 robertoaloi

@robertoaloi Thanks, I might look into it myself.

I tried the following changes:

diff --git a/apps/els_lsp/src/els_completion_provider.erl b/apps/els_lsp/src/els_completion_provider.erl
index 30c40ae..fa5fc1a 100644
--- a/apps/els_lsp/src/els_completion_provider.erl
+++ b/apps/els_lsp/src/els_completion_provider.erl
@@ -356,9 +356,9 @@ item_kind_file(Path) ->
 %%==============================================================================
 -spec snippet(atom()) -> item().
 snippet(attribute_behaviour) ->
-  snippet(<<"-behaviour().">>, <<"behaviour(${1:Behaviour}).">>);
+  snippet(<<"-behaviour().">>, <<"-behaviour(${1:Behaviour}).">>);
 snippet(attribute_export) ->
-  snippet(<<"-export().">>, <<"export([${1:}]).">>);
+  snippet(<<"-export().">>, <<"-export([${1:}]).">>);
 snippet(attribute_vsn) ->
   snippet(<<"-vsn(Version).">>, <<"vsn(${1:Version}).">>);
 snippet(attribute_callback) ->
@@ -403,6 +403,7 @@ snippet(attribute_compile) ->
 -spec snippet(binary(), binary()) -> item().
 snippet(Label, InsertText) ->
   #{ label => Label
+   , filterText => re:replace(Label, <<"[-\\(\\)\\[\\]\\.]">>, <<"">>, [global, {return, binary}])
    , kind  => ?COMPLETION_ITEM_KIND_SNIPPET
    , insertText => InsertText
    , insertTextFormat => ?INSERT_TEXT_FORMAT_SNIPPET

Observations:

  1. Adding the dash to the actual insertText makes the behavior correct in Sublime LSP. Not sure what the behavior would be in VS Code or other editors though.
  2. Adding a filterText property where extra characters are removed as suggested here broke erlang_ls completely for some reason (i.e. no completions work).

eproxus avatar Dec 10 '21 09:12 eproxus

I think the correct approach is to use textEdit instead of insertText, I looked into implementing it, but it turned our to be harder than I anticipated so I never finished a working implementation, maybe someone with more spare time than me wants to look at it? :)

plux avatar Dec 10 '21 10:12 plux

@plux Using textEdit, erlang_ls would be responsible for inserting only the rest of the completion, skipping what was already typed? (If I understood it correctly)

eproxus avatar Dec 10 '21 11:12 eproxus