Some cleanup and minor optimizations #1212
Conversation
|
Do we need a (deprecated) version of ScriptRuntime.enumNext() for backward compatibility? |
| @@ -2290,16 +2289,15 @@ public static void setEnumNumbers(Object enumObj, boolean enumNumbers) { | |||
| ((IdEnumeration) enumObj).enumNumbers = enumNumbers; | |||
| } | |||
|
|
|||
| public static Boolean enumNext(Object enumObj) { | |||
There was a problem hiding this comment.
Sadly yes, since this is public and in ScriptRuntime it's certainly possible that someone somewhere depends on it. But can we have a version that takes no "cx" and is also marked deprecated?
Also, most of our functions that take a Context take it as the first argument -- would you consider doing it that way?
There was a problem hiding this comment.
Backward compatibility done.
There was a problem hiding this comment.
Also, most of our functions that take a Context take it as the first argument -- would you consider doing it that way?
I'm aware of that but the one that implements the methods
- ScriptRuntime.enumId(Object, Context)
- ScriptRuntime.enumInit(Object, Context, boolean)
- ScriptRuntime.enumInit(Object, Context, int)
- ScriptRuntime.enumInit(Object, Context, Scriptable, int)
- ScriptRuntime.enumValue(Object, Context)
has used a different pattern.
Because all these methods are public i think we have to use the same pattern for the enumNext; otherwise we have to add some more deprecated stuff.
There was a problem hiding this comment.
Yes, you're right, we do! Thanks.
|
Thanks for this. I do think that if we're going to change the signature of a public method in ScriptRuntime that we should deprecate the old signature but keep it working. |
|
Thanks -- makes sense to me. |
No description provided.