Twig extension refactoring #3701
Conversation
They make diffs harder to read, and window vertical split less useful.
Because of the last argument, this is actually impossible to use.
The special "exception" argument is better because it can even be used to display a stack trace.
|
Please review this @core23 @OskarStark |
|
LGTM |
|
ping @core23 @OskarStark please merge |
| $templateName, | ||
| $fieldDescription->getFieldName(), | ||
| $defaultTemplate | ||
| ), array('exception' => $e)); |
There was a problem hiding this comment.
@core23 @OskarStark BTW : this whole $defaultTemplate thing does not make much sense, don't you think? Removing it would be a BC break, but I think we should remove that for 3.0
There was a problem hiding this comment.
Feel free to create a separate PR for this. Do you want to remove the $defaultTemplate for the log or in general? @greg0ire
There was a problem hiding this comment.
In general. I can't imagine why one would want to have this.
There was a problem hiding this comment.
Don't know if there is a concrete case for this. Maybe someone else could review that?
There was a problem hiding this comment.
The code was initially added with an interesting comment :
// todo: find a better solution
There was a problem hiding this comment.
I'll work on a PR, see if it breaks things.
I wanted to work on the api closing, but ended up making this big backwards-compatible refactoring, which is BC.