Properly format conditionals #657
Conversation
index.bs
Outdated
| <dl class="switch"> | ||
| <dt> If the [=XR/XR device=] is <code>null</code> | ||
| <dd> [=Reject=] |promise| with <code>null</code>. | ||
| <dt> Else if the [=XR/XR device=]'s [=list of supported modes=] does not [=list/contain=] |mode| |
There was a problem hiding this comment.
Given the intro, I think this can just be "If..."
| <dd> [=Reject=] |promise| with a "{{NotSupportedError}}" {{DOMException}}. | ||
| <dt> Else if |immersive| is <code>true</code> and the algorithm is not [=triggered by user activation=] | ||
| <dd> [=Reject=] |promise| with a "{{SecurityError}}" {{DOMException}}. | ||
| </dl> |
There was a problem hiding this comment.
There should probably be an Otherwise given the intro. It can just say "Continue to the next step." or something like that.
index.bs
Outdated
| <dl class="switch"> | ||
| <dt> If |immersive| is <code>true</code> | ||
| <dd> Set the [=active immersive session=] to |session|, and set [=pending immersive session=] to <code>false</code>. | ||
| <dt> Else |
There was a problem hiding this comment.
Line 244 uses Otherwise, which is also used in at least some other specs. Most importantly, this spec should be consistent.
|
Addressed |
There was a problem hiding this comment.
Looks good. I'm a little wary of over-applying this particular pattern and accidentally making the spec harder to read as a result, but after reading through your changes carefully I think these are all appropriate cases for the switch statement. I spotted one minor error, but once that's fixed I'm happy to merge.
index.bs
Outdated
| <dd> Let |referenceSpace| be a new {{XRBoundedReferenceSpace}}. | ||
| <dt> Else | ||
| <dd> Let |referenceSpace| be a new {{XRReferenceSpace}}. | ||
| </dl> | ||
| 1. If |type| is {{bounded-floor}}, let |referenceSpace| be a new {{XRBoundedReferenceSpace}}. | ||
| 1. Else let |referenceSpace| be a new {{XRReferenceSpace}}. |
There was a problem hiding this comment.
This line and the one above it should be removed now, since they're covered in the new switch statement.
index.bs
Outdated
| <dl class="switch"> | ||
| <dt> If |type| is {{bounded-floor}} | ||
| <dd> Let |referenceSpace| be a new {{XRBoundedReferenceSpace}}. | ||
| <dt> Else |
There was a problem hiding this comment.
There are six instances of Else. Should those be Otherwise for consistency?
There was a problem hiding this comment.
Honestly I think Else is fine in these cases, especially if it's just a simple if/else statement. Where I would expect Otherwise to apply is mostly at the end of a switch with several branches. (Functioning as default in a C++ switch, essentially)
There was a problem hiding this comment.
It seems like other specs use Otherwise with this most of the time (as @ddorwin said above)
https://drafts.csswg.org/css-transforms-2/#serialization-of-the-computed-value , https://drafts.csswg.org/web-animations-2/#the-intrinsic-iteration-duration-of-a-group-effect
https://html.spec.whatwg.org/multipage/dom.html#directionality-of-the-attribute
Fixes #647
Contains no behavior changes, only formatting. In some cases we had "If foo do ... and abort these steps. Else do ...", I just simplified these cases by removing the else.
Rendered
cc @ddorwin
r? @toji