Changes to process integer object as integer and long as long, not as…

[ghost]

Here is the problem: rhino is used inside handlebars library. When handlebars have to process js, it calls rhino for help. Assume there is a simple piece of js code: "data.lenght". When handlebars ask rhino to process it, it is expected a number without a decimal place (for instance 4) to be returned. In fact 4.0 is returned.
Small changes in this pull request change this behavior.

[gbrail]

This seems like it's going to break someone, somewhere, who is expecting those properties to return numbers with decimal points in them. If this is important then we should consider whether it should be enabled by a feature flag.

Also, do you have an example of a bit of JavaScript that doesn't work due to the existing behavior?

[ghost]

It would be great to enable this on demand by feature flag. What is the way to put it under toggle?

Using handlebars I'm adding some js helper, something like this:

Handlebars.registerHelper('bannerLength', function(bannerTotal, options) {
return bannerTotal.length;
});

and to process it Rhino is used inside handlebars. It returns number with decimal place.

[gbrail]

Yes, perhaps others don't agree, but I feel that this change has a real chance to break existing code, and to even cause a performance regression. At the same time, I understand that it's important to your project and others may feel the same.

The way to make something optional in Rhino is to use the Context class, and by following the pattern used there.

For instance, you would:

1. Add a constant to Context.java like "FEATURE_CONVERT_NUMBER_TO_INTEGER" with comments that describes it.
2. Add a case in ContextFactory.java to set the default value (which I think should be false)
3. Use it in your code by calling Context.hasFeature when you want to select the behavior. (And if you don't have a Context object in scope, you can call Context.getCurrentContext to get it.)

And again, you can follow the example of existing flags...

And once you do that, I'd like to also see some tests specifically for this behavior that test the behavior with the flag both on and off.

Thanks!

[ghost]

Thank you for clear explanation, will update pull request soon.

[gbrail]

Thanks for doing this -- it looks good, and if others find it useful or doesn't break their code then we can always look at making it the default.

[ghost]

Thank you for your time.
I'd like to ask a question - what is the approximate time for next release (where this functionality can be used)?

[gbrail]

There's no formal schedule -- it has to do with need. It turns out that I'd like to get the release out anyway to some of my own customers, so it might happen soon, because of: #390 Do you have a deadline you're working to? In general, releases don't take long. However, there is some upstream effect as they end up in YUM repos, Homebrew, etc.

…

On Tue, Mar 13, 2018 at 6:38 AM, alexandrmaximenko ***@***.*** > wrote: Thank you for your time. I'd like to ask a question - what is the approximate time for next release (where this functionality can be used)? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#398 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAf0a8W_iEJaqSXdCxUd871dPqion-Xpks5td8vLgaJpZM4SVJ-G> .

[ghost]

It really didn't take long and exceeded our expectations, thank you very much!