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

Autocompletion from a contract computed by merging #1499

Closed
Tracked by #1588
thufschmitt opened this issue Aug 4, 2023 · 3 comments · Fixed by #1584
Closed
Tracked by #1588

Autocompletion from a contract computed by merging #1499

thufschmitt opened this issue Aug 4, 2023 · 3 comments · Fixed by #1584

Comments

@thufschmitt
Copy link
Contributor

Is your feature request related to a problem? Please describe.

The Nickel lsp currently provides completion for field names when writing a record with a record contract. For instance:

{
  ab<CURSOR>
} | ({ abcde | Number, fghij | Number })

will offer abcde as completion.

However this only works if the contract is a record literal. For instance, there will be no completion for the snippet below:

{
  ab<CURSOR>
} | ({ abcde | Number } & { fghij | Number })

Describe the solution you'd like

Allowing any kind of computation in the LSP is probably not a good idea, but ome heuristic to allow completion with contracts which are marginally more complex than a record literal would be very useful. In particular plain merging could be allowed here.

Describe alternatives you've considered

Not merging contracts. But that would make my life very painful. Or living without completion, but that would be sad too.

Additional context

https://github.com/nickel-lang/nickel-nix is making a heavy usage of merged contracts (all the builders and shells are defined with some variation of Derivation & { some_extra_arg | Number, some_other_extra_arg | String }), so this would be very valuable

@thufschmitt
Copy link
Contributor Author

A variation of the above (let me know if you think it deserves its own issue) is completion based on field names for merged records. For instance

{
  abcde | Number,
  ab<CURSOR>
}

offers a completion for abcde, but not

{
  abcde | Number,
} & {
  ab<CURSOR>
}

@jneem jneem added the area: lsp label Aug 4, 2023
@yannham
Copy link
Member

yannham commented Aug 7, 2023

Related: #1477. I think the current state of the semantics doesn't address this, and the semantics of completion for merging should be quite natural (probably completion_items(a & b) = completion_items(a) U completion_items(b))

@yannham
Copy link
Member

yannham commented Aug 7, 2023

My bad, it did address it already, I just hadn't seen the change.

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

Successfully merging a pull request may close this issue.

3 participants