Advanced Set tag #87
Advanced Set tag #87
Conversation
There was a problem hiding this comment.
Mainly LGTM except the typos 😁
I wondered at first if it was a good choice to have the same tag name sometimes needing a end tag and sometimes not. Kind of asymmetrical here.
In the end it's still probably worth having the same tag name for both use cases (as the end goal is the same, set a variable), despite this asymmetry, but I hope this won't be too confusing for users. Maybe worth at least ensuring that errors/diagnostics are nice if people use both 3 components and a body.
Documentation/tag-set.md
Outdated
| The parameters and tags of this node depend on which mode you want to use: | ||
|
|
||
| - Use `{% set myVar %}...{% endset %}` to render and store everything between the start and end tag into the variable. | ||
| - Use `{% set myVar someOtherVar.prop1.prop2 #}` to evaluate and store an expression's result into the variable. |
There was a problem hiding this comment.
I've seen you twice (here and in the PR description) tell that this ends with #} not %}. Is this intentional or a double typo?
There was a problem hiding this comment.
I have no idea how that got in there 😆 It's in the PR description because I copy/pasted it.
Documentation/tag-set.md
Outdated
| | Name | Description | | ||
| |-----------|------------------------------------------------------------------------| | ||
| | Tag Name | `set` | | ||
| | End Tag | `endset` or N/A (if you're creating an alias of an existing variable) | |
There was a problem hiding this comment.
Nitpick: one space too many before 'or'
Sources/SetNode.swift
Outdated
| enum Content { | ||
| case nodes([NodeType]) | ||
| case value(Resolvable) | ||
| } |
There was a problem hiding this comment.
Damn this quite confused me… nodes is actually used when rendering to a single string value, and value is used when pointing to a node tree/object… 😅
I now get the logic but I needed to read the code twice to understand and be sure you didn't invert the logic but my reading did ^^
Maybe renaming case value to case reference(to:) would have helped? Idk
Refs SwiftGen/SwiftGen#379 (comment).
This feature adds a variation of the
settag to directly set the value of a variable to the contents of another expression, without rendering it down to aString(like the currentsetdoes).Users will be able to use the
settag in 2 ways:{% set myVar %}...{% endset %}to render and store everything between the start and end tag into the variable.{% set myVar someOtherVar.prop1.prop2 #}to evaluate and store an expression's result into the variable.This is particularly useful if the expression is a complex object such as an
Arrayor aDictionary.