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

Support count and count.index completion in blocks #134

Merged
merged 23 commits into from
Oct 14, 2022
Merged

Conversation

jpogran
Copy link
Contributor

@jpogran jpogran commented Sep 9, 2022

This provides completion hints inside blocks for count.index references within resource, data and module blocks anywhere the count meta-argument is supported. It detects if count is used already and does not suggest duplicates.

Closes hashicorp/terraform-ls#860

@jpogran jpogran self-assigned this Sep 9, 2022
@jpogran jpogran force-pushed the expression_extensions branch 5 times, most recently from ba4f9d6 to aaf1fa8 Compare September 23, 2022 14:05
@jpogran jpogran changed the title Support Expression Extensions Support count and count.index completion in blocks Sep 23, 2022
@jpogran jpogran marked this pull request as ready for review September 23, 2022 14:20
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

Quick review from my side: LGTM so far, just minor suggestions.

I think we should copy the BodySchema extensions when merging dependent body schemas as well. To be sure to cover every edge-case.

mergedSchema.Targets = depSchema.Targets.Copy()
mergedSchema.DocsLink = depSchema.DocsLink.Copy()
}

mergedSchema.Extensions = depSchema.Extensions.Copy()

context/context.go Outdated Show resolved Hide resolved
decoder/body_candidates.go Outdated Show resolved Hide resolved
@jpogran jpogran marked this pull request as draft September 26, 2022 14:09
@jpogran jpogran force-pushed the expression_extensions branch 2 times, most recently from 575c954 to 2b2b977 Compare September 27, 2022 16:47
This provides completion hints inside blocks for `count.index` references within resource, data and module blocks anywhere the `count` meta-argument is supported. It detects if count is used already and does not suggest duplicates.
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.

Looks pretty good and functional - just needs some cleanup - I left some comments in-line.

context/context.go Outdated Show resolved Hide resolved
decoder/body_candidates.go Outdated Show resolved Hide resolved
decoder/body_candidates.go Outdated Show resolved Hide resolved
decoder/body_candidates.go Outdated Show resolved Hide resolved
decoder/body_extensions_test.go Outdated Show resolved Hide resolved
decoder/semantic_tokens_test.go Outdated Show resolved Hide resolved
decoder/semantic_tokens_test.go Outdated Show resolved Hide resolved
decoder/semantic_tokens_test.go Outdated Show resolved Hide resolved
decoder/semantic_tokens_test.go Outdated Show resolved Hide resolved
schema/body_schema.go Outdated Show resolved Hide resolved
@jpogran jpogran marked this pull request as ready for review October 11, 2022 15:12
@jpogran jpogran added the enhancement New feature or request label Oct 11, 2022
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.

Just some error handling suggestions & a test.

decoder/candidates_test.go Outdated Show resolved Hide resolved
decoder/hover.go Outdated Show resolved Hide resolved
decoder/hover_test.go Outdated Show resolved Hide resolved
decoder/semantic_tokens.go Outdated Show resolved Hide resolved
decoder/semantic_tokens_test.go Show resolved Hide resolved
context/context.go Outdated Show resolved Hide resolved
jpogran and others added 2 commits October 12, 2022 12:23
@jpogran
Copy link
Contributor Author

jpogran commented Oct 13, 2022

Feedback is added and test is added!

jpogran added a commit to hashicorp/terraform-ls that referenced this pull request Oct 13, 2022
In hashicorp/hcl-lang#134 we now pass the context down to the decoder if using Hover or SemanticTokens.
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

Latest test LGTM!

@jpogran jpogran merged commit 7eceda0 into main Oct 14, 2022
@jpogran jpogran deleted the expression_extensions branch October 14, 2022 12:58
jpogran added a commit to hashicorp/terraform-ls that referenced this pull request Oct 18, 2022
In hashicorp/hcl-lang#134 we now pass the context down to the decoder if using Hover or SemanticTokens.

Updated hcl-lang and terraform-schema references.
jpogran added a commit to hashicorp/terraform-ls that referenced this pull request Oct 18, 2022
In hashicorp/hcl-lang#134 we now pass the context down to the decoder if using Hover or SemanticTokens.

Updated hcl-lang and terraform-schema references.
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.

Support count.index references in blocks w/ count
3 participants