-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Editorial: Improve consistency of referring to String values #2711
Conversation
5c5fb5c
to
b745b63
Compare
spec.html
Outdated
@@ -42032,7 +42032,7 @@ <h1>Atomics.sub ( _typedArray_, _index_, _value_ )</h1> | |||
|
|||
<emu-clause id="sec-atomics.wait"> | |||
<h1>Atomics.wait ( _typedArray_, _index_, _value_, _timeout_ )</h1> | |||
<p>`Atomics.wait` puts the calling agent in a wait queue and puts it to sleep until it is notified or the sleep times out. The following steps are taken:</p> | |||
<p>Puts the calling agent in a wait queue and suspends it until notified or the wait times out, returning a String differentiating those cases. The following steps are taken:</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<p>Puts the calling agent in a wait queue and suspends it until notified or the wait times out, returning a String differentiating those cases. The following steps are taken:</p> | |
<p>`Atomics.wait` puts the calling agent in a wait queue and suspends it until notified or the wait times out, returning a String differentiating those cases. The following steps are taken:</p> |
We don't normally use sentence fragments in this position, I don't think? We're inconsistent in the terminology here, but I don't think we have that particular inconsistency, at least not often.
Also I prefer "until it is notified" rather than "until notified", since it makes explicit which thing is being notified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sentence fragments here are not uncommon (there are lots of examples like <p>Returns …
and <p>Performs …
and <p>Produces …
, cf. Number.prototype.toLocaleString/BigInt.prototype.toLocaleString and Math and String.prototype.matchAll and String.prototype.split and RegExp.prototype.exec ( string ) and Array.prototype.concat and %TypedArray%.prototype.subarray, with honorable mentions for Bitwise Shift Operators and String.prototype.charCodeAt/String.prototype.codePointAt [in which it is part of a note]).
But I actually share your grammatical preference, and don't remember my motivation here so I have accepted your suggestions. And in the long run, I expect this to be resolved by introducing more structure so the phrasing will become almost moot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jmdyck, I fully support the efforts of #2592. @bakkot Should I pull forward and merge its changes here?
<p>Puts the calling agent in a wait queue and suspends it until notified or the wait times out, returning a String differentiating those cases. The following steps are taken:</p> | |
<p>This function puts the calling agent in a wait queue and suspends it until it is notified or the wait times out, returning a String differentiating those cases.</p> | |
<p>It performs the following steps when called:</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I pull forward and merge its changes here?
Ehhh that seems pretty unrelated. I'd prefer not to try to tweak wording piecemeal; better for each PR to do its own thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than comments.
spec.html
Outdated
@@ -42032,7 +42032,7 @@ <h1>Atomics.sub ( _typedArray_, _index_, _value_ )</h1> | |||
|
|||
<emu-clause id="sec-atomics.wait"> | |||
<h1>Atomics.wait ( _typedArray_, _index_, _value_, _timeout_ )</h1> | |||
<p>`Atomics.wait` puts the calling agent in a wait queue and puts it to sleep until it is notified or the sleep times out. The following steps are taken:</p> | |||
<p>`Atomics.wait` puts the calling agent in a wait queue and suspends it until it is notified or the wait times out, returning a String differentiating those cases. The following steps are taken:</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While not grammatically wrong, I don't think, I'd prefer "... and suspends it until notified or until the wait times out", i.e. duplicating the "until", which I think reads better.
30b8262
to
c0e39e5
Compare
Rebased and accepted @syg's suggestion. Should be ready to merge now. |
c0e39e5
to
7043389
Compare
This leaves a few cases of
the String value *"…"*
where it seems important to highlight that the value is a string (and most consistently in sections defining the initial value of a@@toStringTag
property), but otherwise updates to just*"…"*
. There is also some minor cleanup of operation descriptions in the neighborhood of those changes, for CanonicalNumericIndexString and Atomics.wait.