Re: [w3c/IndexedDB] Add non-normative documentation for methods/properties. (#121)

domenic commented on this pull request.

This overall looks really, really good. (And, reading through the rest of the spec as I scroll down, I'm impressed by all the stuff in between as well :D. What a great exemplar of a modern spec.)

One nit, not new in this PR, is that I think saying `"InvalidStateError" DOMException` is clearer for readers, especially developers, who are otherwise confused by how `InvalidStateError` is the name but `TypeError` is the class. See https://github.com/whatwg/html/issues/1176 for when we made the switch in HTML.

> +      <var>request</var> = <var>store</var> .
+          {{IDBObjectStore/delete()|delete}}(<var>query</var>)
+    </dt>
+    <dd>
+       Deletes [=records=] in |store| with the given
+       [=/key=] or in the given [=key range=] in |query|.
+
+       If successful, |request|'s {{IDBRequest/result}} will
+       be undefined.
+    </dd>
+
+    <dt>
+      <var>request</var> = <var>store</var> . {{IDBObjectStore/clear()|clear}}()
+    </dt>
+    <dd>
+       Deletes all records [=records=] in |store|.

extra "records"

> +       set to {{ConstraintError}}.
+
+       If successful, |request|'s {{IDBRequest/result}} will be the
+       [=record=]'s [=/key=].
+    </dd>
+
+    <dt>
+      <var>request</var> = <var>store</var> .
+          {{IDBObjectStore/delete()|delete}}(<var>query</var>)
+    </dt>
+    <dd>
+       Deletes [=records=] in |store| with the given
+       [=/key=] or in the given [=key range=] in |query|.
+
+       If successful, |request|'s {{IDBRequest/result}} will
+       be undefined.

I wonder if, in developer-facing documentation at least, `undefined` should be in code font? I'm not sure, since we have a pretty strong precedent of leaving it in normal font inside algorithms.

> +  The following methods throw {{ReadOnlyError}} if called within a
+  [=read-only transaction=], and {{TransactionInactiveError}} if
+  called when the [=/transaction=] is not [=transaction/active=].
+
+  <dl class=domintro>
+    <dt>
+      <var>request</var> = <var>store</var>
+          . {{IDBObjectStore/put()|put}}(<var>value</var> [, <var>key</var>])
+    </dt>
+    <dt>
+      <var>request</var> = <var>store</var>
+          . {{IDBObjectStore/add()|add}}(<var>value</var> [, <var>key</var>])
+    </dt>
+    <dd>
+       Adds or updates a [=record=] in |store| with the given |value|
+       and |key|. If the store uses [=in-line keys=] then |key| must

Comma after in-line keys would make this flow better IMO

> +      Throws {{/InvalidStateError}} if not called within an [=upgrade
+      transaction=].
+    </dd>
+    <dt><var>index</var> . {{IDBIndex/objectStore}}</dt>
+    <dd>
+      Returns the {{IDBObjectStore}} the index belongs to.
+    </dd>
+    <dt><var>index</var> . keyPath</dt>
+    <dd>
+      Returns the [=/key path=] of the index.
+    </dd>
+    <dt><var>index</var> . multiEntry</dt>
+    <dd>
+      Returns true if the index's [=index/multiEntry flag=] is set.
+    </dd>
+    <dt><var>index</var> . multiEntry</dt>

Copypasta

> @@ -3574,6 +4103,28 @@ interface IDBKeyRange {
 };
 </pre>
 
+<div class=note>
+  <dl class=domintro>
+    <dt><var>range</var> . {{IDBKeyRange/lower}}</dt>
+    <dd>
+      Returns [=lower bound=], or undefined if none.
+    </dd>
+    <dt><var>range</var> . {{IDBKeyRange/upper}}</dt>
+    <dd>
+      Returns [=upper bound=], or undefined if none.
+    </dd>
+
+    <dt><var>range</var> . {{IDBKeyRange/lowerOpen}}</dt>
+    <dd>
+      Returns true if [=lower open flag=] is set, and false otherwise.

Missing "the" here and below.

> @@ -3722,6 +4314,31 @@ enum IDBCursorDirection {
 };
 </pre>
 
+
+<div class=note>
+  <dl class=domintro>
+    <dt><var>cursor</var> . {{IDBCursor/source}}</dt>
+    <dd>
+      Returns the {{IDBObjectStore}} or {{IDBIndex}} the cursor was opened from.
+    </dd>
+    <dt><var>range</var> . {{IDBCursor/direction}}</dt>
+    <dd>
+      Returns the [=cursor/direction=] of the cursor.

Maybe parenthetically duplicating the contents of the enum might be helpful here (i.e. "next", "nextunique", ...)

> @@ -4082,6 +4781,33 @@ enum IDBTransactionMode {
 };
 </pre>
 
+<div class=note>
+  <dl class=domintro>
+    <dt><var>transaction</var> . {{IDBTransaction/objectStoreNames}}</dt>
+    <dd>
+      Returns a list of the names of [=/object stores=] in the
+      transaction's [=transaction/scope=]. For an [=upgrade transaction=]
+      this is all object stores in the [=database=].
+    </dd>
+    <dt><var>transaction</var> . {{IDBTransaction/mode}}</dt>
+    <dd>
+      Returns the [=transaction/mode=] the transaction was created with
+      ({{"readonly"}} or {{"readwrite"}}) or {{"versionchange"}} for

Comma after parenthesis for extra clarity

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/IndexedDB/pull/121#pullrequestreview-11251649

Received on Friday, 2 December 2016 22:34:09 UTC