-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
proposal: text/template, html/template: add LookupFunc, LookupOption methods #43062
Comments
Actually, there is another bit of info that needs to be exposed: the options set with |
LookupFunc would be convenient (although for user functions it can be done with reflection). However, the types of the built-in functions are unavailable. |
I'm not entirely sure these are general enough to warrant the complexity they would require us to support. It seems like any kind of type-checking will stop being able to do anything as soon as there is an interface{}, right? |
I'd settle for making the built-in template functions visible. I don't understand the comment about interface{}. If the call is t.Execute(x), then t.Check(xx) ought to work fine as long as reflect.TypeOf(xx) == reflect.Type(x). Type checking just needs to check that the fields and methods used in the template exist on the argument. My use case is in the debug server started by gopls. It stopped working when some underlying data structures were refactored. There's no way to detect that in unit tests presently, but t.Check() could go in debug_test.go, and would discover that the templates could no longer execute because of type errors. |
I think this means that if you pass a value of
to the type-checker, then You can check
and "versions" does assume a specific type. So I just call Check on t.Lookup("versions") with that type. |
It is possible to get user-defined func maps via reflection, and I've made that change. But it makes the code depend on implementation details. For instance, for html/template I need
They can. You can also pass a different dot type to Execute than the one you pass to Check. I'm not sure I understand your point. I think you're arguing that this sort of checker isn't very useful because it can be fooled, but in practice one would write tests to avoid that as much as possible—for instance, by defining the FuncMap in a global and referencing it in the tests. |
The difficult part about this issue is that committing to public API is forever, and we don't really know whether we want to commit to this. But I also don't want to stand in the way of you building a useful tool and exploring the space of possible changes. For example it might be that the right long-term API is not LookupFunc and LookupOption but instead adding your Check to the template package, where it will have access to internals. I don't know. It sounds like if you've got a way to not be blocked, by using reflection, that might be the right answer for now, and then we don't have to decide whether to add LookupFunc and LookupOption more generally. |
I'm not blocked by this issue. |
Should I file a separate proposal for that, or co-opt this one? |
Please file a different proposal once you have some experience using it and think all the wrinkles are ironed out. Thanks! Will treat this one as retracted. |
No change in consensus, so declined. |
It is possible using only exported symbols to write a type-checker for templates. There is just one missing piece: to check a function call like
{{len .}}
, the checker needs the function signature, but there is no way to retrieve that from aTemplate
.To work around this, I copied the information for built-ins from
text/template
, and I must ask the user to pass to my checker whateverFuncMap
s they used when parsing the template. The first solution is brittle and the second unpleasant.All that is missing is the method
Template.LookupFunc(name string) interface{}
. It would behave much like the unexportedfindFunction
function intext/template/funcs.go
, but it would return the unreflected function value or nil if the name was undefined.I propose it be added to both
text/template.Template
andhtml/template.Template
.The text was updated successfully, but these errors were encountered: