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

New Code Action: Fill block with optional+required attributes #1530

Open
2 tasks
radeksimko opened this issue Dec 20, 2023 · 1 comment
Open
2 tasks

New Code Action: Fill block with optional+required attributes #1530

radeksimko opened this issue Dec 20, 2023 · 1 comment
Labels

Comments

@radeksimko
Copy link
Member

radeksimko commented Dec 20, 2023

Background

The following two PRs introduced support for pre-filling required attributes on completion:

which can be enabled via prefillRequiredFields and corresponding setting in VS Code terraform.experimentalFeatures.prefillRequiredFields. It looks like this in action:

2023-12-20 09 54 45

One of the problems with the above approach is that it's unclear what the user wants to do and what the completion should do when there are pre-existing attributes in the block body. There may be contexts where overwriting it is correct and other contexts where just adding missing attributes is correct. In the specific example above it would most likely make more sense to overwrite things because two different resources are unlikely to have common attributes. This may not be the case for all blocks (other than resource) though.

Additionally, manipulating larger ranges of configuration during completion, especially if they require a lot of context is not common in LSP and similar problems in other servers are more frequently implemented as code actions. The LSP makes this difficult (intentionally or not) by mandating textEdits to be computed by the time the completion is requested and these cannot be resolved at a later stage via completionItem/resolve, as the spec says:

All other properties (usually sortText, filterText, insertText and textEdit) must be provided in the textDocument/completion response and must not be changed during resolve.

This in turn can make such pre-filling mechanism very expensive in the sense that the server pre-computes text edits for potentially hundreds of options (and send all of them through RPC) when only one of them will be chosen and the rest is thrown away.

Another example where similar approach won't work for the reasons above is pre-filling of inputs in module block based on source because it would be prohibitevely expensive to download all metadata from the Registry API for all modules the user may wish to complete.

Code actions could provide a different and more flexible way (from maintenance perspective) to solve this problem, even if that comes at the cost of different UX for the user.

Example of a similar action in gopls:
2023-12-20 10 11 09

Proposal

  • Implement code action which pre-fills missing required attributes
  • Implement code action which pre-fills all attributes

Implementation Notes

The problem of "what to do with existing attributes" does not go away with code actions but it may be resolved a little more easily. We could offer different code actions and let the user choose what's appropriate (overwrite vs keep) and more importantly keeping code and calculating relatively costly text edit is more easily done at a later stage (compared to having to pre-compute everything early during completion).

@kevineor
Copy link

kevineor commented Jan 2, 2024

I'm working on a proposal regarding this issue,

My idea was to fill the Diagnostics.data, while we are running the missing attribute validator.

It looks that this is the intended solution provided by the lsp specification : https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnostic

This should not be CPU expensive as we are already publishing diagnostics.

At me moment i'm filling variables with a null default value

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

No branches or pull requests

2 participants