Submit @r5200. On Wed, Apr 8, 2009 at 8:09 PM, Freeland Abbott <fabb...@google.com> 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 <knor...@google.com> wrote: > >> Internet is alive today in the boonies.LGTM. >> /kel >> >> >> On Wed, Apr 8, 2009 at 3:58 AM, Freeland Abbott <fabb...@google.com>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 <fabb...@google.com> >>> Date: Tue, Mar 31, 2009 at 9:55 PM >>> Subject: Re: Review: JsArrays patch >>> To: Kelly Norton <knor...@google.com> >>> Cc: Scott Blum <sco...@google.com>, Bruce Johnson <br...@google.com>, >>> GWTcontrib <Google-Web-Toolkit-Contributors@googlegroups.com> >>> >>> >>> 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 <knor...@google.com>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 <fabb...@google.com>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 <sco...@google.com> 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 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 -~----------~----~----~----~------~----~------~--~---
