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

feat: required arguments pre-filled when creating resource #96

Closed
mazingerzzz opened this issue Jul 24, 2019 · 6 comments
Closed

feat: required arguments pre-filled when creating resource #96

mazingerzzz opened this issue Jul 24, 2019 · 6 comments

Comments

@mazingerzzz
Copy link

It would be awesome to have pre-filled required arguments when created new ressource.

@paultyng paultyng transferred this issue from hashicorp/vscode-terraform May 12, 2020
@radeksimko radeksimko changed the title feat: required argument pre-filled when create ressource feat: required arguments pre-filled when creating resource May 12, 2020
@radeksimko
Copy link
Member

radeksimko commented May 12, 2020

Agreed - this would be a useful feature to have.

This could be implemented as part of the completion method via snippets:

func snippetForAttrType(placeholder int, attrType cty.Type) string {
mapSnippet := func(aType cty.Type) string {
return fmt.Sprintf("{\n"+` "${0:key}" = %s`+"\n}",
snippetForAttrType(1, aType))
}
switch attrType {
case cty.String:
return fmt.Sprintf(`"${%d:value}"`, placeholder)
case cty.List(cty.String), cty.Set(cty.String):
return fmt.Sprintf(`["${%d:value}"]`, placeholder)
case cty.Map(cty.String):
return mapSnippet(cty.String)
case cty.Bool:
return fmt.Sprintf(`${%d:false}`, placeholder)
case cty.List(cty.Bool), cty.Set(cty.Bool):
return fmt.Sprintf(`[${%d:false}]`, placeholder)
case cty.Map(cty.Bool):
return mapSnippet(cty.Bool)
case cty.Number:
return fmt.Sprintf(`${%d:42}`, placeholder)
case cty.List(cty.Number), cty.Set(cty.Number):
return fmt.Sprintf(`[${%d:42}]`, placeholder)
case cty.Map(cty.Number):
return mapSnippet(cty.Number)
}
return ""
}
func snippetForNestedBlock(name string) string {
return fmt.Sprintf("%s {\n ${0}\n}", name)
}

just hooked from HCL labels, rather than attributes and blocks.

With that the required arguments would be pre-filled automatically after picking the type

Screenshot 2020-05-12 at 20 56 16

There's just one edge which comes to my mind - what happens if there are already existing attributes within the block? Should we remove all of these and replace them with new ones which are relevant for the new picked type? I think it's important to understand why/when would this happen - it may be because the user made a mistake when picking the type at first and triggered completion again, or it could be because they are migrating away from a deprecated resource name (e.g. aws_lb -> aws_alb). I think the second use case could probably eventually be dealt with by Terraform's own CLI as it also has the power to move resources within state.

So I'm leaning more towards just replacing whatever there may be inside the block with new required arguments, but I'm keen to hear about other use cases where this may not be appropriate.

@FernandoMiguel
Copy link

replace only if file is saved is probably a good idea, so the user can revert if they dont want that.

and if possible, if auto-save is on (i do), only if committed

@danieldreier
Copy link
Contributor

my impulse for the least-surprising or least-destructive would be to strictly add the required attributes/blocks without removing the old ones, and count on other validations to notify the user if they end up with unwanted attributes that no longer fit the resource.

For example, in the use case where I'm refactoring a resource from old name/attributes to the new, it would be annoying to lose everything, because I might want to reference the known-invalid syntax to copy/refactor an expression, reference, etc.

@radeksimko
Copy link
Member

We have also discussed the possibility of pre-filling required attributes via code action, which may be the most flexible option here.

Example from gopls:

2021-07-23 15 32 46

@radeksimko
Copy link
Member

@jpogran implemented this as part of the following PRs which will be part of the upcoming LS / extension release:

As mentioned within these issues, the functionality will be (at least initially) opt-in via prefillRequiredFields config option under terraform-ls.experimentalFeatures.

We also plan to add code action that would provide comparable functionality for existing blocks, which is tracked under hashicorp/vscode-terraform#801

@github-actions
Copy link

github-actions bot commented Nov 8, 2021

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants