On Monday, November 1, 2010, Jeremy Orlow <jor...@chromium.org> wrote: > On Sat, Oct 30, 2010 at 12:24 AM, Jonas Sicking <jo...@sicking.cc> wrote: > > > On Sun, Oct 24, 2010 at 8:06 AM, Jeremy Orlow <jor...@chromium.org> wrote: >> On Sat, Oct 23, 2010 at 2:45 AM, Jonas Sicking <jo...@sicking.cc> wrote: >>> >>> Hi all, >>> >>> IDBKeyRange is in need of some cleanup. The first issue is its >>> constructors. Currently the IDL for IDBKeyRange, define that the >>> constructors, .only, .leftBound, .rightBound, .bound, on object >>> instances themselves. I don't think this is intentional since first of >>> all it makes it impossible to create the first IDBKeyRange, second, >>> second it seems strange to have constructor functions on instances. >>> >>> It seems like the intent is that the following should work: >>> >>> x = new IDBKeyRange.bound(...); >>> and possibly also >>> x = IDBKeyRange.bound(...); >>> (the latter works for for example XMLHttpRequest, JS-heads have also >>> informed me that it should work). >> >> The latter is what we do in Chromium and what I'm pretty sure we agreed upon >> in earlier threads. Since these methods are factories, it seems as though >> the new operator should not be necessary. >> >>> >>> Unfortunately there is no way to express this in WebIDL, so I think we >>> need to describe it in prose instead. I'll raise this with Cameron, >>> but I think that since at this point only IndexedDB uses these >>> "static" functions, it might not make sense to add support to WebIDL. >> >> I believe "class methods" is the more accepted term for such methods. I'd >> rather we look at adding these to WebIDL now rather than waiting for the >> next instance that uses them as I'd hope we didn't add a new concept just >> for one interface in one API. Prose seems like the right solution in the >> mean time though. >> >>> >>> The other thing is that the .flags attribute is really not very >>> javascript friendly as it uses bitfields. It also is weird in that it >>> has multiple bits, but some have strong interdependencies. For example >>> if SINGLE is set LEFT_OPEN and RIGHT_OPEN must be false, and >>> LEFT_BOUND and RIGHT_BOUND must be true. It also seems like the >>> LEFT_BOUND flag is closely correlated to the .left property. >>> >>> I suggest the following API instead: >>> >>> interface IDBKeyRange { >>> readonly attribute any left; >>> readonly attribute any right; >>> readonly attribute boolean leftOpen; >>> readonly attribute boolean rightOpen; >>> }; >>> >>> the .left and .right properties return 'null' or 'undefined' (to be >>> discussed) when the keyrange isn't bound in that direction. >> >> I agree the flags attribute is confusing and not very JavaScript like. I >> also like your solution. null is probably what it should return when not >> bounded. > > Why did you end up using undefined rather than null? "The <a>key range</a> > <a href="#widl-IDBKeyRange-lower"><code>lower</code></a> value is > <code>undefined</code> or <a>less than</a> <var>key value</var>."
null is actually currently defined as a valid key, so we can't use it to signal an absence of key. Though that's something I want to change. (part of the whole keyPath-for-indexes discussion). However undefined also feels more javascripy to show that something is missing or not defined :) >> What should the open param return if its associated left/right is not bound? >> I don't care much, but it should definitely be specced. >>> >>> Another question I had is if "left" and "right" really are good names. >>> Seems to me that "upper" and "lower" are better words, but it might be >>> that left/right is established concepts in the database community? I >>> don't feel strongly on this issue. Oh, I see now that bug 10397 is >>> filed on this exact issue. >> >> Yup. Agree that left/right are bad names. >> The fact that we're discussing things over again points to the fact that we >> really need to get on top of all the bugs against the spec. Most of them >> should be pretty easy. Will the editors have time to address these before >> TPAC? If not, does anyone have a problem with me becoming an editor? >> >>> >>> Lastly, should we allow values to be passed directly to all APIs that >>> currently take IDBKeyRanges? It seems nice to not require people to >>> create an IDBKeyRange object using patterns like: >>> >>> req = objectStore.openCursor(new IDBKeyRange.only(foo)); >>> >>> and instead allow >>> >>> req = objectStore.openCursor(foo); >> >> I'm OK with this. >> >>> >>> Similarly, should we allow IDBObjectStore.get on and IDBIndex.get to >>> take a IDBKeyRange and return the first value that matches the >>> keyrange. This would allow things like "give me the first entry with a >>> value greater than X". >> >> That seems like it could be valuable. I'd support such a change. > > I've implemented these changes in > http://dvcs.w3.org/hg/IndexedDB/rev/e5a08025e518 > > There is definitely a lot of work remaining on the spec. I've added a > lot of infrastructure which should make this *much* easier, such as > defining algorithms for request handling, but the work to fully > integrate that everywhere is still remaining. Much of it tightening up > the language to make things more strictly defined. One thing that is > totally lacking for example is integration with event loops, another > is cursors. > > So the more help we can get, the better. We'd just need to be better > at assigning bugs to ourselves before working on them as to avoid > stepping on each others toes. > > The text looks better. The formatting seems to be screwed up though. (A > large part of the doc is now red.) Eep! How'd I miss that. Will fix asap. > I only skimmed, but I did notice one thing right away: 1.401: comma before or > is incorrect. I'm sure there are lots of editorial issues. Didn't bother spending too much time on it since I wanted to get to a point when the document is more stable. > Also, is this implying that opening a cursor that has no results should throw >and/or fire on error? It doesn't seem super clear. That's what the spec said prior to my changes. Didn't bother fixing it since cursors had bigger issues. / Jonas