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

Debug Undefined Feature to echo back Uknown tokens. #1150

Merged
merged 2 commits into from
Jan 19, 2024

Conversation

abarrafo
Copy link
Contributor

Inspired by this PR - Render back unresolved tokens. Implemented instead with features as suggested by @jasmith-hs and named ECHO_UNDEFINED . I originally wanted to call it DEBUG_UNDEFINED to get close to this feature in jinja2, but that offers a bit more.

Copy link
Contributor

@jasmith-hs jasmith-hs left a comment

Choose a reason for hiding this comment

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

Thank you for taking a look at this

Comment on lines 46 to 53
final FeatureActivationStrategy feat = interpreter
.getConfig()
.getFeatures()
.getActivationStrategy(ECHO_UNDEFINED);

if (result.isEmpty() && feat.isActive(interpreter.getContext())){
result = master.getImage();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If the expression renders an empty string, {{ '' }} then we should allow an empty string to be output.

If instead we moved this to line 24:

if (var == null && feat.isActive(interpreter.getContext()) {
  return new RenderedOutputNode(master.getImage());
}

We can make sure that we're only echoing back when it's undefined.

Also, make sure that you run mvn prettier:write to format the code, as we have tab spacing set to 2 for this project.

@jasmith-hs jasmith-hs merged commit 4820ad3 into HubSpot:master Jan 19, 2024
2 checks passed
@marius-diumia
Copy link

thank you both @jasmith-hs & @abarrafo for helping with this and merging

@abarrafo
Copy link
Contributor Author

Hey @jasmith-hs , just curious on what your release lifecycle is? Any idea on when the next release will make is into maven central? Thanks so much

@jasmith-hs
Copy link
Contributor

@abarrafo I put it on my todo list. I'm going out of town so I'll either be able to release it tomorrow or in about a week and a half

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.

3 participants