-
Notifications
You must be signed in to change notification settings - Fork 680
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
[Template] avoid thousands of MissingPropertyExceptions #1067
Comments
Do you have any examples of which code breaks? We found that without this change, sometimes you could #{set} something and then use the value with just ${} |
@angryziber As I said in #1039, I am doing a PR with tests for that. |
I thought the goal of set is to influence get with the same name? In most
cases they lived in separate namespaces, afaik. But I found one case, where
the set value was accessible via ${} before, but not anymore.
…On Wed, 28 Dec 2016, 02:50 Alexandre Chatiron, ***@***.***> wrote:
@angryziber <https://github.com/angryziber> As I said in #1039
<#1039>, I am doing a PR with
tests for that.
Yes this is the goal of set, so you mean that with your change with cannot
do it anymore.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1067 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA11YJ6VWC3uhQgDKJxOQ-UYBivNuvzQks5rMbJogaJpZM4LV-vH>
.
|
Yes I could be used by Get, but as nothing block to used it another way, we cannot block now a behavior that was accepted before, or it will affect users projects, or we should do it in a major revision. |
In my project.
But do you say that that was an intended behaviour, #{set} and then ${}
instead of #{get}?
…On Thu, 29 Dec 2016, 02:45 Alexandre Chatiron, ***@***.***> wrote:
So we cannot block now a behavior that was accepted before, or it will
affect users projects.
But what do you mean by But I found one case, where the set value was
accessible via ${} before, but not anymore., in your project, in
play/examples?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1067 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA11YBHeno9jl4M96Yy0oWbed0-ORN-Qks5rMwKwgaJpZM4LV-vH>
.
|
No I just say that the initial behavior was to used #{set} /#{get} but as we allowed to used ${} in the past, I am not sure we can remove this behavior in the minor release |
[#1067] test(tag): add demo and test for used of #{set} and #{get}
From @angryziber in #1039:
Binding can only contain variables (basically, a Map), not properties.
If groovy doesn't find a variable, it throws a MissingPropertyException populating the stacktrace.
GroovyTemplate.getProperty() first calls the getVariable(), if it throws (happens very often if you have optional bindings), then it catches and delegates to getProperty(), that does some slow work itself again.
This small change avoids these excessive exceptions with stacktraces on every missing variable access.
The text was updated successfully, but these errors were encountered: