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

introduce Generator type with string handling strategy options #682

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mbenson
Copy link

@mbenson mbenson commented May 28, 2024

There have been for several years a number of open issues relating to interpolation sequences in string literals. The community approach seems typically to hand-code these on an individual basis, which is simple enough, but when such strings are potentially embedded in more complex constructs there reaches a point where it makes more sense to support the preservation/non-escaping of such sequences in hcl proper. To that end, this PR introduces a Generator type that can be parameterized with options to guide token generation behavior in a manner that supports nested structures as described. The options structure, at time of submission is:

type HandlingStrategy = int

const (
	AsLiteral  = 0
	AsTemplate = 1
)

type Handling struct {
	String HandlingStrategy
}

type GenerateOptions struct {
	Handling Handling
}

e.g., to handle string types as templates, you would create Generator{GenerateOptions{Handling{String: AsTemplate}}}.

Note that the template handling panics if the value cannot be lexed; it might make sense to return an error from the Generator methods and translate these to panic for the legacy functions only. If this is desired, I am glad to make the change.

#442
#615
#392

@mbenson
Copy link
Author

mbenson commented May 29, 2024

The GenerateOptions could for example allow a user to set their desired HEREDOC terminator string, for a slight rework of #540 .

@apparentlymart
Copy link
Member

Hi @mbenson! Thanks for working on this.

Unfortunately the hclwrite part of HCL has seen relatively little design investment compared to the rest of HCL because HCL development is driven primarily by Terraform's needs and Terraform makes only very limited use of hclwrite.

I do agree that what you've proposed here is a plausible design for solving one specific annoyance with hclwrite's design -- the fact that it's hard to generate arbitrary template expressions -- but since there's no overall design guidance for this package to draw from, and this approach has very significant opinions built into it about how the API should behave, I don't feel equipped to evaluate what you've proposed at the moment.

I think before we could move forward here we'd need to establish what the overall design goals for this package even are, what patterns it intends to follow, what are the intended limits to its scope, etc. Then it would be possible to compare what you proposed with those decisions to decide if this design is a good fit.

I realize this is a very unsatisfying response. I wish I could give you some more concrete feedback that could result in immediate progress, but I'm afraid nobody is working in this area at the moment to give this the full review it deserves, and what you've contributed here is significant enough that it would represent a constraint on how this package could evolve in future. Therefore I don't really know how we can move forward here.

If I had infinite time I'd love to think more deeply about what hclwrite's scope ought to be and therefore what design is most appropriate for it, but ever since starting this package I've been constantly assigned to other things far away from here and have only been able to make small incremental improvements as a result. 😖

Therefore much as the issues you linked to have been in limbo, so will be this PR. I don't like it, but I'm afraid hclwrite's current approach is essentially frozen until such time as the Terraform team is able to spend more time designing it.

Thanks again for working on this, and I'm sorry this is the best I can do with this at the moment.

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.

None yet

2 participants