Skip to content

Conversation

prestontimmons
Copy link
Contributor

Prior to 55f12f8, the template origin was available on each node via
self.token.source[0]. This behavior was removed when debug handling was
simplified, but 3rd-party debugging tools still depended on it's presence.
This updates the Parser to set origin on individual nodes. This enables the
source template to be determined even when template extending or including is
used.

@prestontimmons
Copy link
Contributor Author

@nedbat Could you test this branch out? I'd like confirmation if it works for you or not.

@carljm or @aaugustin I'm on the fence about this. It kinda feels hacky for Parser to accept origin? I could instead set node.origin after the node list is compiled in Template.compile_nodelist(). Yay or nay on this change?

Coverage needs to be able to determine the template file a node came from when Node.render() is called. I had a few other ideas, but this approach is the simplest I've come up with.

@carljm
Copy link
Member

carljm commented Jan 15, 2016

Hi @prestontimmons - this seems OK to me. I know it modifies the Parser init signature, but on the whole it doesn't look all that invasive.

But I may not be the best reviewer for it, as I'm not especially committed to the future of the DTL :-) I hope to see it slowly phased out in favor of Jinja2.

@nedbat
Copy link
Contributor

nedbat commented Jan 16, 2016

@prestontimmons thanks. I cherry-picked this commit onto the stable/1.9.x branch, and with a slight adaptation in my plugin, it all works, so that's great!

The bad news is that this branch itself is broken, because the tip is broken in other ways that I don't understand yet.

@aaugustin
Copy link
Member

This looks all right to me.

@timgraham
Copy link
Member

Would the test be any better in test_nodelist.py? Could you add a mention in the 1.9.2 release notes?

@timgraham
Copy link
Member

Is the answer "no" about moving the test?

@prestontimmons
Copy link
Contributor Author

Hi. Yes, I thought the same thing at first, but the tests in test_nodelist.py are specific to the NodeList class. This change doesn't affect that class at all. Instead it's a template level behavior. I feel like it still makes more sense in tests.py instead. Thanks!

Prior to 55f12f8, the template origin was available on each node via
`self.token.source[0]`. This behavior was removed when debug handling was
simplified, but 3rd-party debugging tools still depended on it's presence.
This updates the Parser to set origin on individual nodes. This enables the
source template to be determined even when template extending or including is
used.
@timgraham
Copy link
Member

merged in cfda1fa, thanks!

@timgraham timgraham closed this Jan 26, 2016
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.

5 participants