Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optional arguments supplied with ~ are completed with ? instead #1179

Open
bcc32 opened this issue Sep 12, 2023 · 2 comments
Open

Optional arguments supplied with ~ are completed with ? instead #1179

bcc32 opened this issue Sep 12, 2023 · 2 comments

Comments

@bcc32
Copy link

bcc32 commented Sep 12, 2023

When requesting completion for a function's optional labeled argument, which the user intends to pass with ~, OCaml-LSP's completion suggestion uses ? instead and replaces the original intended meaning, e.g.:

let f ?abc ~def () = assert false

let () = f ~

If, at the end of that snippet, the user presses TAB (or whatever completion key), OCaml-LSP suggests:

?abc : 'a option
~def : 'b

as some of the completion results. Selecting ?abc causes the code to now look like:

let () = f ?abc

whereas the expected behavior is more likely:

let () = f ~abc

This issue also occurs if the user has typed some prefix of the optional argument label first.

Here's the event log item I got from Emacs:

(:jsonrpc "2.0" :id 176 :method "textDocument/completion" :params
          (:textDocument
           (:uri "file:///home/azeng/z.ml")
           :position
           (:line 8 :character 12)
           :context
           (:triggerKind 1)))
[server-reply] (id:176) Tue Sep 12 16:49:13 2023:
(:id 176 :jsonrpc "2.0" :result
     (:isIncomplete :json-false :items
                    [(:data
                      (:position
                       (:character 12 :line 8)
                       :textDocument
                       (:uri "file:///home/azeng/z.ml"))
                      :deprecated :json-false :detail "'a option" :kind 5 :label "?abc" :sortText "0000" :textEdit
                      (:newText "?abc" :range
                                (:end
                                 (:character 12 :line 8)
                                 :start
                                 (:character 10 :line 8))))
                     (:data
                      (:position
                       (:character 12 :line 8)
                       :textDocument
                       (:uri "file:///home/azeng/z.ml"))
                      :deprecated :json-false :detail "'b" :kind 5 :label "~def" :sortText "0001" :textEdit
                      (:newText "~def" :range
                                (:end
                                 (:character 12 :line 8)
                                 :start
                                 (:character 10 :line 8))))
                     (:data
                      (:position
                       (:character 12 :line 8)
                       :textDocument
                       (:uri "file:///home/azeng/z.ml"))
                      :deprecated :json-false :detail "'_weak42" :kind 5 :label "~a" :sortText "0002" :textEdit
                      (:newText "~a" :range
                                (:end
                                 (:character 12 :line 8)
                                 :start
                                 (:character 10 :line 8))))]))

I think maybe the label should remain ?abc but the newText should be ~abc instead?

@jfeser
Copy link
Collaborator

jfeser commented Sep 14, 2023

Or we could offer completions for ?abc, ~abc, and ~def.

@ddickstein
Copy link
Collaborator

In Neovim ?abc doesn't even appear as a completion option. I agree with @bcc32 though; newText should be ~abc. @jfeser I'm not sure adding ?abc as an additional completion candidate adds more value than noise, as the user has already signaled how they want to pass the argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants