[gwt-contrib] Re: Fwd: Review: JsArrays patch
Submit @r5200. On Wed, Apr 8, 2009 at 8:09 PM, Freeland Abbott wrote: > Thanks, Kelly. Apologies for making you find a tree with wi-fi > available > > > On Wed, Apr 8, 2009 at 8:56 AM, Kelly Norton wrote: > >> Internet is alive today in the boonies.LGTM. >> /kel >> >> >> On Wed, Apr 8, 2009 at 3:58 AM, Freeland Abbott wrote: >> >>> Bruce, with Kelly away and Scott abdicating over JsArrayBase, it falls >>> back to you for this patch. >>> >>> This should have only the non-contentious stuff (namely push and >>> shift)... plus toSource, which I argue is dead-stripped if unused, and >>> plausibly useful for debugging (yes, on Mozilla only, but it tests for >>> definition, and I don't imagine anything else should want that method >>> name). I can make an in-project utility class to do sort, since Kelly was >>> nervous about setting an ill-considered trend for JSO functors. >>> >>> >>> >>> >>> -- Forwarded message -- >>> From: Freeland Abbott >>> Date: Tue, Mar 31, 2009 at 9:55 PM >>> Subject: Re: Review: JsArrays patch >>> To: Kelly Norton >>> Cc: Scott Blum , Bruce Johnson , >>> GWTcontrib >>> >>> >>> Okay. I'll look into sort and toSource tomorrow; right now I'm away from >>> that project code to see whether I want to try to fight for sort. >>> So this patch should, I hope, be just the easy stuff. Usually when I say >>> something rash like that it turns out I'm very wrong, but we'll see. >>> >>> Regarding JSO, I pulled toSource, but left the I-think-helpful >>> toString(). I know Scott worried about "pulling in" other types' >>> toString(), but in separate private email I think his worry is >>> unfounded---best I know, we don't analyze JSNI bodies, so while this >>> implementation references toString() if available, it can't change code >>> size by pulling anything in that wasn't already coming for the ride. I >>> think; I'm sure he or someone will correct me if I'm wrong on that! >>> >>> >>> >>> On Tue, Mar 31, 2009 at 5:44 AM, Kelly Norton wrote: >>> Few things: Overall, I'd like to be more conservative landing things in JavaScriptObject for a couple of reasons: (1) It's hard to take a mulligan with these because of their constraints (2) there is always a trivial work around to create application specific subclasses (with toll free casting). >> From r5082: I don't think toSource is appropriate for JavaScriptObject. It only works on mozilla browsers. >> JsArray.push: As I recall, this[this.length] = value is faster than this.push(value) on all browsers. It's not a complexity change like array.pop() is, but it can be significant. (How I do wish we had continuous perf testing). >> javadoc: The javadoc for these should be written to describe what the function does. "Direct mapping to underlying sort" is a good implementation note, but we should actually way what it does and not rely on developers having an understanding of the JavaScript equivalent. >> sort(JavaScriptObject): I'd like to avoid this one if we can. It just opens up larger questions about the right way to do this. We don't currently have a convention for representing JavaScript functions in Java. Someone should probably have a good story there before we add something like this to JavaScriptObject. /kel On Fri, Mar 27, 2009 at 2:15 PM, Freeland Abbott wrote: > I think the argument is more for "unnecessary" rather than "bad"... > although without JsArrayBase (we can make it package-protected, and call > it > JsArrayImpl if anyone cares), we duplicate the JSNI implementation for a > couple trivial methods. I thought refactoring them into one place was > nice, > but trivial enough that I'm not fighting over it. Revised patch > attached; I > can go either way on this. > > > > > > On Fri, Mar 27, 2009 at 2:06 PM, Scott Blum wrote: > >> I'm going to punt this review to Bruce & Kelly, 'cause I have no idea >> why having JsArrayBase would be bad. :) >> >> On Fri, Mar 27, 2009 at 1:28 PM, Freeland Abbott < >> gwt.team.fabb...@gmail.com> wrote: >> >>> Scott, we already talked about this, but here's the patch for public >>> review. >>> >>> The basic goal is to surface the native length, sort, push, and shift >>> operators for JsArrays... I know you mentioned that IE6's push may be >>> slower >>> than indexed extension, and thus a candidate for deferred binding, but I >>> wanted to get a basic implementation in first. >>> >>> There should be only checkstyle changes from what we discussed >>> (though that obviously doesn't help the rest GWTC). I also added some >>> checkstyle fixes to JavaScriptObject, introduced by my c5082. >>> >> >> > -- If you received this communication by mistake, you are entitled
[gwt-contrib] Re: Fwd: Review: JsArrays patch
Thanks, Kelly. Apologies for making you find a tree with wi-fi available On Wed, Apr 8, 2009 at 8:56 AM, Kelly Norton wrote: > Internet is alive today in the boonies.LGTM. > /kel > > > On Wed, Apr 8, 2009 at 3:58 AM, Freeland Abbott wrote: > >> Bruce, with Kelly away and Scott abdicating over JsArrayBase, it falls >> back to you for this patch. >> >> This should have only the non-contentious stuff (namely push and shift)... >> plus toSource, which I argue is dead-stripped if unused, and plausibly >> useful for debugging (yes, on Mozilla only, but it tests for definition, and >> I don't imagine anything else should want that method name). I can make an >> in-project utility class to do sort, since Kelly was nervous about setting >> an ill-considered trend for JSO functors. >> >> >> >> >> -- Forwarded message -- >> From: Freeland Abbott >> Date: Tue, Mar 31, 2009 at 9:55 PM >> Subject: Re: Review: JsArrays patch >> To: Kelly Norton >> Cc: Scott Blum , Bruce Johnson , >> GWTcontrib >> >> >> Okay. I'll look into sort and toSource tomorrow; right now I'm away from >> that project code to see whether I want to try to fight for sort. >> So this patch should, I hope, be just the easy stuff. Usually when I say >> something rash like that it turns out I'm very wrong, but we'll see. >> >> Regarding JSO, I pulled toSource, but left the I-think-helpful toString(). >> I know Scott worried about "pulling in" other types' toString(), but in >> separate private email I think his worry is unfounded---best I know, we >> don't analyze JSNI bodies, so while this implementation references >> toString() if available, it can't change code size by pulling anything in >> that wasn't already coming for the ride. I think; I'm sure he or someone >> will correct me if I'm wrong on that! >> >> >> >> On Tue, Mar 31, 2009 at 5:44 AM, Kelly Norton wrote: >> >>> Few things: >>> Overall, I'd like to be more conservative landing things in >>> JavaScriptObject for a couple of reasons: (1) It's hard to take a mulligan >>> with these because of their constraints (2) there is always a trivial work >>> around to create application specific subclasses (with toll free casting). >>> >>> >> From r5082: I don't think toSource is appropriate for >>> JavaScriptObject. It only works on mozilla browsers. >>> >> JsArray.push: As I recall, this[this.length] = value is faster than >>> this.push(value) on all browsers. It's not a complexity change like >>> array.pop() is, but it can be significant. (How I do wish we had continuous >>> perf testing). >>> >>> >> javadoc: The javadoc for these should be written to describe what the >>> function does. "Direct mapping to underlying sort" is a good implementation >>> note, but we should actually way what it does and not rely on developers >>> having an understanding of the JavaScript equivalent. >>> >>> >> sort(JavaScriptObject): I'd like to avoid this one if we can. It just >>> opens up larger questions about the right way to do this. We don't currently >>> have a convention for representing JavaScript functions in Java. Someone >>> should probably have a good story there before we add something like this to >>> JavaScriptObject. >>> >>> /kel >>> >>> On Fri, Mar 27, 2009 at 2:15 PM, Freeland Abbott wrote: >>> I think the argument is more for "unnecessary" rather than "bad"... although without JsArrayBase (we can make it package-protected, and call it JsArrayImpl if anyone cares), we duplicate the JSNI implementation for a couple trivial methods. I thought refactoring them into one place was nice, but trivial enough that I'm not fighting over it. Revised patch attached; I can go either way on this. On Fri, Mar 27, 2009 at 2:06 PM, Scott Blum wrote: > I'm going to punt this review to Bruce & Kelly, 'cause I have no idea > why having JsArrayBase would be bad. :) > > On Fri, Mar 27, 2009 at 1:28 PM, Freeland Abbott < > gwt.team.fabb...@gmail.com> wrote: > >> Scott, we already talked about this, but here's the patch for public >> review. >> >> The basic goal is to surface the native length, sort, push, and shift >> operators for JsArrays... I know you mentioned that IE6's push may be >> slower >> than indexed extension, and thus a candidate for deferred binding, but I >> wanted to get a basic implementation in first. >> >> There should be only checkstyle changes from what we discussed (though >> that obviously doesn't help the rest GWTC). I also added some checkstyle >> fixes to JavaScriptObject, introduced by my c5082. >> > > >>> >>> >>> -- >>> If you received this communication by mistake, you are entitled to one >>> free ice cream cone on me. Simply print out this email including all >>> relevant SMTP headers and present them at my desk to claim your creamy >>> treat. We'll have a laugh at my emailing incompetence, a
[gwt-contrib] Re: Fwd: Review: JsArrays patch
Internet is alive today in the boonies.LGTM. /kel On Wed, Apr 8, 2009 at 3:58 AM, Freeland Abbott wrote: > Bruce, with Kelly away and Scott abdicating over JsArrayBase, it falls back > to you for this patch. > > This should have only the non-contentious stuff (namely push and shift)... > plus toSource, which I argue is dead-stripped if unused, and plausibly > useful for debugging (yes, on Mozilla only, but it tests for definition, and > I don't imagine anything else should want that method name). I can make an > in-project utility class to do sort, since Kelly was nervous about setting > an ill-considered trend for JSO functors. > > > > > -- Forwarded message -- > From: Freeland Abbott > Date: Tue, Mar 31, 2009 at 9:55 PM > Subject: Re: Review: JsArrays patch > To: Kelly Norton > Cc: Scott Blum , Bruce Johnson , > GWTcontrib > > > Okay. I'll look into sort and toSource tomorrow; right now I'm away from > that project code to see whether I want to try to fight for sort. > So this patch should, I hope, be just the easy stuff. Usually when I say > something rash like that it turns out I'm very wrong, but we'll see. > > Regarding JSO, I pulled toSource, but left the I-think-helpful toString(). > I know Scott worried about "pulling in" other types' toString(), but in > separate private email I think his worry is unfounded---best I know, we > don't analyze JSNI bodies, so while this implementation references > toString() if available, it can't change code size by pulling anything in > that wasn't already coming for the ride. I think; I'm sure he or someone > will correct me if I'm wrong on that! > > > > On Tue, Mar 31, 2009 at 5:44 AM, Kelly Norton wrote: > >> Few things: >> Overall, I'd like to be more conservative landing things in >> JavaScriptObject for a couple of reasons: (1) It's hard to take a mulligan >> with these because of their constraints (2) there is always a trivial work >> around to create application specific subclasses (with toll free casting). >> >> >> From r5082: I don't think toSource is appropriate for JavaScriptObject. >> It only works on mozilla browsers. >> >> JsArray.push: As I recall, this[this.length] = value is faster than >> this.push(value) on all browsers. It's not a complexity change like >> array.pop() is, but it can be significant. (How I do wish we had continuous >> perf testing). >> >> >> javadoc: The javadoc for these should be written to describe what the >> function does. "Direct mapping to underlying sort" is a good implementation >> note, but we should actually way what it does and not rely on developers >> having an understanding of the JavaScript equivalent. >> >> >> sort(JavaScriptObject): I'd like to avoid this one if we can. It just >> opens up larger questions about the right way to do this. We don't currently >> have a convention for representing JavaScript functions in Java. Someone >> should probably have a good story there before we add something like this to >> JavaScriptObject. >> >> /kel >> >> On Fri, Mar 27, 2009 at 2:15 PM, Freeland Abbott wrote: >> >>> I think the argument is more for "unnecessary" rather than "bad"... >>> although without JsArrayBase (we can make it package-protected, and call it >>> JsArrayImpl if anyone cares), we duplicate the JSNI implementation for a >>> couple trivial methods. I thought refactoring them into one place was nice, >>> but trivial enough that I'm not fighting over it. Revised patch attached; I >>> can go either way on this. >>> >>> >>> >>> >>> >>> On Fri, Mar 27, 2009 at 2:06 PM, Scott Blum wrote: >>> I'm going to punt this review to Bruce & Kelly, 'cause I have no idea why having JsArrayBase would be bad. :) On Fri, Mar 27, 2009 at 1:28 PM, Freeland Abbott < gwt.team.fabb...@gmail.com> wrote: > Scott, we already talked about this, but here's the patch for public > review. > > The basic goal is to surface the native length, sort, push, and shift > operators for JsArrays... I know you mentioned that IE6's push may be > slower > than indexed extension, and thus a candidate for deferred binding, but I > wanted to get a basic implementation in first. > > There should be only checkstyle changes from what we discussed (though > that obviously doesn't help the rest GWTC). I also added some checkstyle > fixes to JavaScriptObject, introduced by my c5082. > >>> >> >> >> -- >> If you received this communication by mistake, you are entitled to one >> free ice cream cone on me. Simply print out this email including all >> relevant SMTP headers and present them at my desk to claim your creamy >> treat. We'll have a laugh at my emailing incompetence, and play a game of >> ping pong. (offer may not be valid in all States). >> > > > > > > -- If you received this communication by mistake, you are entitled to one free ice cream cone on me. Simply print out this email including all relevant SMTP headers and pres
