Skip to content

Conversation

@gazerro
Copy link
Member

@gazerro gazerro commented Jan 16, 2026

scriggo: add `UnexpandedTransformer` and rename `TreeTransformer`

Introduce a new build option for templates that allows applying a
transformation to the AST of each parsed file before expansion.

This is particularly useful when changes need to be applied prior to
expansion, such as rewriting paths.

As part of this change, 'BuildOptions.TreeTransformer' has been renamed
to ExpandedTransformer to better reflect when it is executed.

Introduce a new build option for templates that allows applying a
transformation to the AST of each parsed file before expansion.

This is particularly useful when changes need to be applied prior to
expansion, such as rewriting paths.

As part of this change, 'BuildOptions.TreeTransformer' has been renamed
to ExpandedTransformer to better reflect when it is executed.
@gazerro gazerro requested a review from zapateo January 16, 2026 17:00
// Any error related to the compilation itself is returned as a CompilerError.
//
// If noParseShow is true, short show statements are not parsed.
// If a transformer is provided, invoke it with the tree before it is expanded.
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite get the verb tense in this documentation. Shouldn't it be something like "it is invoked..."?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed in commit 931249c.

var render *ast.Render
astutil.Inspect(tree, func(node ast.Node) bool {
if render == nil {
render, _ = node.(*ast.Render)
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite understand this code. This particular line performs a type assertion but silently fails if the assertion fails. Is this intentional? Wouldn't it be better to fail?

Copy link
Member Author

@gazerro gazerro Jan 23, 2026

Choose a reason for hiding this comment

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

Yes, this is intentional and the code is correct. It relies on the value assigned to render, which is nil when the node is not a Render. In other words, the logic is effectively saying: "as long as render is nil, keep traversing".

Alternatively, it could be written more explicitly like this:

astutil.Inspect(tree, func(node ast.Node) bool {
    if render != nil {
        return false
    }
    var ok bool
    render, ok = node.(*ast.Render)
    return !ok
}

However, I'm not convinced this version is actually clearer than the current implementation.

@gazerro gazerro force-pushed the unexpanded-transformer branch from 54db906 to 45533ac Compare January 23, 2026 16:25
@gazerro gazerro requested a review from zapateo January 23, 2026 16:57
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