-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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
Create some lightweight forks of text/template and html/template #6594
Comments
I share your concern with maintaining a fork and compatibility, but if you can come up with some lightweight interfaces that are easy to maintain, it could make many things easier. Also, if we could come up with a really tight, generally useful change to the stdlib API, it may have a shot of getting pushed upstream. Long shot, but we can aim for it. |
This would also, short term, probably mean deprecation/removal of Ace/Amber, which I don't use. I suspect some people will scream at me for that, but those people are not paying me anything to spend the countless hours needed to maintain/develop it. So, this is picking what I think it's the least amount of friction. I have spent countless hours trying to work around the limitation of those packages, and I swill cannot do what I want. |
This commit also removes support for Ace and Amber templates. Fixes gohugoio#6594
This commit also removes support for Ace and Amber templates. Updates gohugoio#6594
This commit also removes support for Ace and Amber templates. Updates gohugoio#6594
This commit also removes support for Ace and Amber templates. Updates gohugoio#6594
This commit also removes support for Ace and Amber templates. Updates gohugoio#6594
This commit also removes support for Ace and Amber templates. Updates gohugoio#6594
This commit also removes support for Ace and Amber templates. Updates gohugoio#6594
This commit also removes support for Ace and Amber templates. Updates gohugoio#6594
This commit also removes support for Ace and Amber templates. Updates gohugoio#6594
This commit also removes support for Ace and Amber templates. Updates gohugoio#6594
This commit also removes support for Ace and Amber templates. Updates gohugoio#6594
This is a big commit, but it deletes lots of code and simplifies a lot. * Resolving the template funcs at execution time means we don't have to create template clones per site * Having a custom map resolver means that we can remove the AST lower case transformation for the special lower case Params map Not only is the above easier to reason about, it's also faster, especially if you have more than one language, as in the benchmark below: ``` name old time/op new time/op delta SiteNew/Deep_content_tree-16 53.7ms ± 0% 48.1ms ± 2% -10.38% (p=0.029 n=4+4) name old alloc/op new alloc/op delta SiteNew/Deep_content_tree-16 41.0MB ± 0% 36.8MB ± 0% -10.26% (p=0.029 n=4+4) name old allocs/op new allocs/op delta SiteNew/Deep_content_tree-16 481k ± 0% 410k ± 0% -14.66% (p=0.029 n=4+4) ``` This should be even better if you also have lots of templates. Closes gohugoio#6594
This commit also removes support for Ace and Amber templates. Updates gohugoio#6594
This is a big commit, but it deletes lots of code and simplifies a lot. * Resolving the template funcs at execution time means we don't have to create template clones per site * Having a custom map resolver means that we can remove the AST lower case transformation for the special lower case Params map Not only is the above easier to reason about, it's also faster, especially if you have more than one language, as in the benchmark below: ``` name old time/op new time/op delta SiteNew/Deep_content_tree-16 53.7ms ± 0% 48.1ms ± 2% -10.38% (p=0.029 n=4+4) name old alloc/op new alloc/op delta SiteNew/Deep_content_tree-16 41.0MB ± 0% 36.8MB ± 0% -10.26% (p=0.029 n=4+4) name old allocs/op new allocs/op delta SiteNew/Deep_content_tree-16 481k ± 0% 410k ± 0% -14.66% (p=0.029 n=4+4) ``` This should be even better if you also have lots of templates. Closes gohugoio#6594
@moorereason just a quick note: I will write up a proposal early next year for Go. The thing we have done here is surprisingly unintrusive, and it's basically a stricter separation of parsing (the AST) and template execution, which I think was a mistake in the original design -- and should be a possible sell for a minor Go redesign in this area. There are many issues on Go's issue tracker that would be solved by this. |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Note that any changes will only touch the template execution, but I suspect that it's interconnected.
While working on the thing I'm working on, I have created a new design for template changes (a dependency graph). All of this was fine and dandy until I came to the
.Render
method. I could probably have pushed that problem to another day, but I have spent a stupid amount of time today bending a working solution together, only to see my benchmarks slow down 20+ %.I have thought about this before, but imagined it would be too hard to do, maintain etc. But I suspect that if I script this and patch lightly, the future time and gray hair saved should be massive.
Here are some motivation points as to "why":
maps.Params
in Hugo, which has methods for lower case lookups.Note that I have been fairly active in wishing stuff on Go's issue tracker, but while I feel they listen, that project, and especially these packages, is moving slower than I would like.
What do you think, @moorereason ?
The text was updated successfully, but these errors were encountered: