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] add an option for queryStr separator #129

Merged
merged 2 commits into from
Jul 23, 2024
Merged

Conversation

ryotafuwa-dev
Copy link
Contributor

@ryotafuwa-dev ryotafuwa-dev commented Jul 22, 2024

WHAT

(Write the change being made with this pull request)
title

WHY

(Write the motivation why you submit this pull request)
Terraform allows comma-separated attributes (e.g. aliased providers like google.common inside a module). In order to modify attributes with comma in its name, we need a way to specify the queryStr split separator charactor.

Behavior

given:

module "A" {
  providers = {
    google.aliasA = google.aliasA
  }
}

where:

hcledit.Create("module.A.providers.google.aliasB", RawVal("google.aliasB"))

then:

module "A" {
  providers = {
    google.aliasA = google.aliasA
  }
}

Because queryStr considers all the commas as the query separator.

target:

module "A" {
  providers = {
    google.aliasA = google.aliasA
    google.aliasB = google.aliasB
  }
}

@ryan-ph
Copy link
Member

ryan-ph commented Jul 22, 2024

Can you include a functional test plan in the PR description (i.e. an input HCL config, the current result of running the query, and the new result of running the query)?

I'm not sure I quite understand where exactly these non-. separators are actually used in TF. The unit test uses /, but as far as I understand, this is not an accepted separator in HCL and I don't understand the benefit of adding this.

@ryotafuwa-dev
Copy link
Contributor Author

ryotafuwa-dev commented Jul 22, 2024

As I stated in the PR description, this PR tries to add a way to specify a charactor other than "." as a query separator in HCLEditor due to this.

Terraform allows comma-separated attributes (e.g. aliased providers like google.common inside a module).

Can you include a functional test plan in the PR description (i.e. an input HCL config, the current result of running the query, and the new result of running the query)?

I think the unit tests I added are enough for this change. Are there more tests you'd like to see for this change?

@ryan-ph
Copy link
Member

ryan-ph commented Jul 22, 2024

I think the unit tests I added are enough for this change. Are there more tests you'd like to see for this change?

I'm trying to understand where a non-. separator is actually used because I have never seen it before. Neither the unit test nor the PR description include a , separator being used in the HCL config that's being read / modified.

Copy link
Member

@ryan-ph ryan-ph left a comment

Choose a reason for hiding this comment

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

So this is meant to support provider aliasing1 left hand side operands as part of the query.

Footnotes

  1. https://developer.hashicorp.com/terraform/language/providers/configuration#alias-multiple-provider-configurations

option.go Outdated Show resolved Hide resolved
@ryotafuwa-dev ryotafuwa-dev requested a review from ryan-ph July 23, 2024 01:21
@ryan-ph ryan-ph merged commit 412c6b2 into main Jul 23, 2024
5 checks passed
@ryan-ph ryan-ph deleted the rfuwa/strquery-sep branch July 23, 2024 01:37
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