ocaml-lsp icon indicating copy to clipboard operation
ocaml-lsp copied to clipboard

Add autocompletion for labelled args

Open lessp opened this issue 2 months ago • 6 comments

This adds auto-completion for labelled arguments. I decided to disable them for primitives like int or bool as it felt a bit iffy, but it'd work for those too. For int merlin seems to suggest 0 etc.

Examples

Here's an example with variants:

https://github.com/user-attachments/assets/2e27725e-1907-4658-b3bd-7a991374cb9e

And records:

https://github.com/user-attachments/assets/094254fc-c4a2-472b-b7fa-f7c2964cfc33

lessp avatar Oct 14 '25 05:10 lessp

Thank you @lessp, that's an interesting proposition. To clarify a bit, this PR does two things:

  1. Add : as a trigger character for completion
  2. Use Construct results to suggest labeled argument values in function application

Item 1 seems all-right when calling a function with a labelled parameter, this might actually be a good thing, but we should make sure that it is acceptable to trigger completion on all other usages of : in the syntax. And add appropriate tests.

About 2, I wonder how often this is going to be useful in practice, once we leave the realm of small handcrafted examples ? I think it's worth experimenting though, maybe by having this behavior configurable. Also, it feels like this should work for every function arguments, not only labelled ones... and maybe other places too ?

@xvw, @rgrinberg, do you have an opinion on this ?

voodoos avatar Oct 15 '25 13:10 voodoos

Thank you @lessp, that's an interesting proposition. To clarify a bit, this PR does two things:

Thanks for leaving your thoughts @voodoos! I primarily thought I'd open up the discussion on this.

  1. Add : as a trigger character for completion

Right, I realise one would have to be wary of where we'd trigger, and I don't want to get ahead of anything, but ideally we'd also be able to trigger for = for MLX and Reason JSX to get padding=.

  1. Use Construct results to suggest labeled argument values in function application

Yes, although I did realise that these are available without having to query merlin if you add e.g. ~color:_. They're quite noisy though see e.g. image, furthermore e.g. variants are scattered across and not sorted within the list.

image

Item 1 seems all-right when calling a function with a labelled parameter, this might actually be a good thing, but we should make sure that it is acceptable to trigger completion on all other usages of : in the syntax. And add appropriate tests.

About 2, I wonder how often this is going to be useful in practice, once we leave the realm of small handcrafted examples ? I think it's worth experimenting though, maybe by having this behavior configurable. Also, it feels like this should work for every function arguments, not only labelled ones... and maybe other places too ?

Can you expand on the statement below?

we leave the realm of small handcrafted examples?

The actual need for this sprung out of it being non-obvious what an argument takes, especially when the type itself is defined a few indirections away. In my experience this becomes even more useful in a larger code-base. The behaviour is something that I miss when switching between other popular languages like TypeScript.

For e.g.

(* style_vars.ml *)
type padding = 
  | NoPadding
  | Small
  | Medium
  | Large
  | Custom of int

type size = 
  | Small
  | Medium
  | Large

type z_layer =
  | Tooltip
  | Popover
  | Modal
  | ExpandableRow
  | Above of z_layer

(* row.ml *)
let create ~(padding: Css.padding) ... = ...

(* consumer.ml *)
let my_row = Row.create ~padding:

As for e.g. records. What is your current approach to constructing the person for something like the following?

(* some_file.ml *)
type employee = { 
  name: string;
  email: string;
}

type customer = {
  name: string;
  age: int
}

... additional types

(* some_consumer.ml *)
let x = create ~customer:

I'd argue that even if you just see the auto-completion, you'd know immediately what arguments you'd need to provide. The alternative as I see it would be to look at the hover information, see something like let create ~(customer: Some_file.customer) -> ... and you'd then do create ~customer:Some_file. and try to find a shape that matches, only to see what you'd need to construct it.

@xvw, @rgrinberg, do you have an opinion on this ?

lessp avatar Oct 15 '25 14:10 lessp

Another experiment I did was to auto-complete valid constructors that return the type in question.

So, instead of doing this:

https://github.com/user-attachments/assets/d33f73e6-0dcb-4b58-beab-2d1720061ec2

You'd, by some heuristic, list definitions that return that type:

image (1)

And on selection, you'd get the correct syntax e.g. ~color:(Test_re.Color.custom ~r:_ ~g:_ ~b:_) similar to the Custom variant.

https://github.com/user-attachments/assets/adcb7eb4-a298-4fdb-9ecb-7f839b2d978b

lessp avatar Oct 15 '25 15:10 lessp

Hi ! I totally agree with one, but I think that construct is usually to much used as an hack entry door so I guess that just relaying on completion is sufficient?

xvw avatar Oct 16 '25 03:10 xvw

Another experiment I did was to auto-complete valid constructors that return the type in question.

Note that this is already an option of the Construct query in Merlin, not reflected in the UI. Like many other such features that initially appear to be a good idea the shear amount of additional suggestions that they can bring to the completion box make them detrimental to the user experience, without a very smart sorting function, which I don't think we have yet.

I thing that it could be useful to experiment with is giving more control to users for construct on the editor side: there is a custom query that we could support on VScode with configuration option to also enable returning values. That would be a good first step, allowing users to call construct everywhere they like instead on relying on the _ completion hack.

voodoos avatar Oct 19 '25 10:10 voodoos

Another experiment I did was to auto-complete valid constructors that return the type in question.

Note that this is already an option of the Construct query in Merlin, not reflected in the UI. Like many other such features that initially appear to be a good idea the shear amount of additional suggestions that they can bring to the completion box make them detrimental to the user experience, without a very smart sorting function, which I don't think we have yet.

Ah, that's even better! Have you already experimented with it? We're in a great position to validate the usefulness of these ideas at scale at Ahrefs, so definitely happy to help. We've already discussed internally about pinning in order to help dogfood these ideas.

I thing that it could be useful to experiment with is giving more control to users for construct on the editor side: there is a custom query that we could support on VScode with configuration option to also enable returning values. That would be a good first step, allowing users to call construct everywhere they like instead on relying on the _ completion hack.

Yes, this sounds excellent. One could then consider if it'd work as the default.

lessp avatar Oct 20 '25 04:10 lessp