Internet is alive today in the boonies.LGTM. /kel On Wed, Apr 8, 2009 at 3:58 AM, Freeland Abbott <[email protected]> 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 <[email protected]> > Date: Tue, Mar 31, 2009 at 9:55 PM > Subject: Re: Review: JsArrays patch > To: Kelly Norton <[email protected]> > Cc: Scott Blum <[email protected]>, Bruce Johnson <[email protected]>, > GWTcontrib <[email protected]> > > > 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 <[email protected]> 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 <[email protected]>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 <[email protected]> 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 < >>>> [email protected]> 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 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). --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
