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

element(): support negative indices in tuples #200

Closed
wants to merge 1 commit into from

Conversation

liamcervante
Copy link
Contributor

Support was added for negative indices in 7b73cce. This change actually only supports lists, while tuples continued to error. This PR adds the same fix for the tuple branch when calculating the return type.

@apparentlymart - it might be that you actually don't want to support this (since tuples have a concretely known length) but it was super quick for me to raise this to start the discussion. I do note the changelog entry just says the element function now accepts negative numbers without clarification that this is only for lists and not tuples.

From Terraform's perspective, users don't really differentiate between a tuple and a list - they just want to execute element([0, 1, 2], -1) without having to wrap the tuple in a tolist(...) call first. I can see arguments for both sides

@liamcervante
Copy link
Contributor Author

Relevant Terraform issue: hashicorp/terraform#36369.

@apparentlymart
Copy link
Collaborator

Hi @liamcervante! Thanks for working on this.

Someone actually contacted me about this problem separately yesterday and so I already had a fix in progress in my local tree. I had some additional tests in mine and so I've just pushed it but the main code change was the same. To recognize your contribution here I wrote the changelog entry as if this PR had been the source of the change, but since this is now conflicting I'm going to close it.

Thanks again!

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

Successfully merging this pull request may close these issues.

2 participants