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: improve variable handling #61

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

Conversation

lukasjarosch
Copy link
Owner

No description provided.

@lukasjarosch lukasjarosch changed the title feat: initial version of improved undefined-value detection feat: improve undefined-value detection in templates Nov 30, 2022
@lukasjarosch lukasjarosch linked an issue Nov 30, 2022 that may be closed by this pull request
@andaryjo
Copy link
Collaborator

andaryjo commented Dec 6, 2022

@lukasjarosch These changes do not seem to resolve the problem we discussed. Trying to reference a class that goes by the same name as the yaml key still throws the same error, just less helpful this time 😄

Previous:

jandary@mbp doratheexplorer % go run main.go
reference to invalid variable '${flags:sandbox}': unexpected node type string at index 1

With skipper on feat/improve-template-undefined-value-handling:

jandary@mbp doratheexplorer % go run main.go
unexpected node type string at index 1

@andaryjo
Copy link
Collaborator

andaryjo commented Dec 6, 2022

My bad, on the second try it somehow worked:

jandary@mbp doratheexplorer % go run main.go
recursive variable usage found at path 'features': ${features:sandbox}

@andaryjo
Copy link
Collaborator

andaryjo commented Dec 6, 2022

@lukasjarosch
Copy link
Owner Author

I'd love to see the demo setup, but the Repo seems to be private 😄

@lukasjarosch
Copy link
Owner Author

I have reviewed your demo and it did indeed not behave as expected. The recursion detection was not working properly.
I've now improved the error messages and fixed up the recursion detection.

With your example you should now receive:

cannot load target data for variable ${features:sandbox}: GetPath: undefined path: [features sandbox]

This is now indeed what a user would expect at this point, don't you think?

@lukasjarosch lukasjarosch linked an issue Dec 8, 2022 that may be closed by this pull request
@lukasjarosch lukasjarosch changed the title feat: improve undefined-value detection in templates feat: improve variable handling Dec 8, 2022
@lukasjarosch lukasjarosch linked an issue Dec 8, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants