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

Prefill Required Fields on Completion #89

Merged
merged 1 commit into from
Oct 7, 2021
Merged

Conversation

jpogran
Copy link
Contributor

@jpogran jpogran commented Sep 28, 2021

On block completion, provide a snippet that completes the label and also the required fields for that block.

Currently users can introduce a new block into their configuration using our auto-complete snippet. This snippet only provides the label name and completes the stanza. However the block is not considered complete/valid until it has all required attributes and blocks. This means that users have to either scroll over a potentially long list of attributes and blocks to add all required ones and/or consult documentation in a separate window.

This commit modifies the label completion logic to provide an auto-complete snippet that lists all required fields (attributes and blocks) and expands the TextEdit range to the entire line instead of just to the tabstop. This will replace the entire line with the chosen snippet. The attributes and blocks are sorted alphabetically to ensure consistent ordering for each invocation.

For example, when completing the aws_appmesh_route resource type, the mesh_name, name, virtual_router_name attributes and the spec block will fill in on accepting the completion. Then, the user can tab through to enter in the appropriate value for each field.

resource "aws_appmesh_route" "{2:name}" {
  mesh_name = "${3:value}"
  name = "${4:value}"
  virtual_router_name = "${:value}"
  spec {
  }
  ${0}
}

Enabled in LS in hashicorp/terraform-ls#657
Surfaced in VS Code in hashicorp/vscode-terraform#799 as an experimental feature

Completes hashicorp/vscode-terraform#719

@jpogran jpogran marked this pull request as draft September 28, 2021 16:24
@jpogran jpogran self-assigned this Sep 28, 2021
@jpogran jpogran added the enhancement New feature or request label Sep 28, 2021
@jpogran jpogran force-pushed the f-prefill-required branch 2 times, most recently from 0cb8396 to 7ce71de Compare September 29, 2021 16:49
@jpogran jpogran marked this pull request as ready for review September 29, 2021 17:28
@jpogran jpogran requested a review from a team September 29, 2021 17:28
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the completion of blocks I only have very minor stylistic suggestions. The PR overall looks pretty good! Well done.

decoder/block_candidates.go Outdated Show resolved Hide resolved
decoder/candidates_test.go Outdated Show resolved Hide resolved
decoder/label_candidates.go Outdated Show resolved Hide resolved
decoder/label_candidates.go Outdated Show resolved Hide resolved
decoder/label_schema_test.go Outdated Show resolved Hide resolved
decoder/decoder.go Outdated Show resolved Hide resolved
@jpogran jpogran force-pushed the f-prefill-required branch 3 times, most recently from 4606715 to f75873e Compare October 4, 2021 14:00
decoder/label_schema_test.go Outdated Show resolved Hide resolved
schema/body_schema.go Outdated Show resolved Hide resolved
schema/body_schema.go Outdated Show resolved Hide resolved
decoder/label_candidates.go Outdated Show resolved Hide resolved
decoder/label_candidates.go Outdated Show resolved Hide resolved
decoder/expression_candidates.go Show resolved Hide resolved
decoder/block_candidates.go Outdated Show resolved Hide resolved
@jpogran jpogran force-pushed the f-prefill-required branch 2 times, most recently from 4abe8f8 to 86c2c23 Compare October 6, 2021 15:25
@jpogran jpogran changed the title Prefill Required Attributes on Block Completion Prefill Required Fields on Block Completion Oct 6, 2021
@jpogran jpogran changed the title Prefill Required Fields on Block Completion Prefill Required Fields on Completion Oct 6, 2021
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I just left some very minor suggestions, but none of them are blocking.

:shipit:

decoder/label_candidates.go Outdated Show resolved Hide resolved
decoder/label_candidates.go Outdated Show resolved Hide resolved
decoder/label_candidates.go Outdated Show resolved Hide resolved
decoder/label_candidates_test.go Outdated Show resolved Hide resolved
decoder/label_candidates_test.go Outdated Show resolved Hide resolved
decoder/label_candidates_test.go Outdated Show resolved Hide resolved
@jpogran jpogran force-pushed the f-prefill-required branch 2 times, most recently from 193e439 to 0286430 Compare October 7, 2021 13:16
On block completion, provide a snippet that completes the label and also the required fields for that block.

Currently users can introduce a new block into their configuration using our auto-complete snippet. This snippet only provides the lable name and completes the stanza. However the block is not considered complete/valid until it has all required attributes and blocks. This means that users have to either scroll over a potentially long list of attributes and blocks to add all required ones and/or consult documentation in a separate window.

This commit modifies the label completion logic to provide an auto-complete snippet that lists all required fields (attributes and blocks) and expands the TextEdit range to the entire line instead of just to the tabstop. This will replace the entire line with the chosen snippet. The attributes and blocks are sorted alphabetically to ensure consistent ordering for each invocation.

For example, when completing the `aws_appmesh_route` resource type, the `mesh_name`, `name`, `virtual_router_name` attributes and the `spec` block will fill in on accepting the completion. Then, the user can tab through to enter in the appropriate value for each field.

```
resource "aws_appmesh_route" "{2:name}" {
  mesh_name = "${3:value}"
  name = "${4:value}"
  virtual_router_name = "${:value}"
  spec {
  }
  ${0}
}
```
@jpogran jpogran merged commit 213283e into main Oct 7, 2021
@jpogran jpogran deleted the f-prefill-required branch October 7, 2021 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants