Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Editorial: use Infra list syntax in mutations #762

Merged
merged 2 commits into from
May 21, 2019
Merged

Editorial: use Infra list syntax in mutations #762

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ddb0a4f
Editorial: use Infra list syntax in mutations
annevk May 17, 2019
09c786e
semicolons for the semicolon god
annevk May 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions dom.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2427,10 +2427,8 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
its <a for=range>end offset</a> by <var>count</var>.
</ol>

<li>Let <var>nodes</var> be <var>node</var>'s
<a>children</a> if <var>node</var> is
a {{DocumentFragment}} <a>node</a>, and a
list containing solely <var>node</var> otherwise.
<li><p>Let <var>nodes</var> be <var>node</var>'s <a>children</a>, if <var>node</var> is a
{{DocumentFragment}} <a>node</a>, « <var>node</var> » otherwise.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads really wrong; I'm not happy with where we've landed if this is what happens. Commas just don't work that way. Semicolon could work, or using a conjunction (either "and" or "or").

Copy link
Member Author

@annevk annevk May 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same if otherwise comes first? I see now I put them wrong way around. I guess I can do some follow-up on Monday in that original thread on this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see. If otherwise comes first it isn't as viscerally wrong, but I still have a slight preference for a conjunction or semicolon.


<li>If <var>node</var> is a {{DocumentFragment}}
<a>node</a>,
Expand Down Expand Up @@ -2602,16 +2600,16 @@ within a <var>parent</var>, run these steps:
<p>If <var>child</var>'s <a for=tree>parent</a> is not null, then:

<ol>
<li><p>Set <var>removedNodes</var> to a list solely containing <var>child</var>.
<li><p>Set <var>removedNodes</var> to « <var>child</var> ».

<li><p><a for=/>Remove</a> <var>child</var> from its <var>parent</var> with the
<i>suppress observers flag</i> set.
</ol>

<p class="note no-backref">The above can only be false if <var>child</var> is <var>node</var>.

<li>Let <var>nodes</var> be <var>node</var>'s <a>children</a> if <var>node</var> is a
{{DocumentFragment}} <a>node</a>, and a list containing solely <var>node</var> otherwise.
<li><p>Let <var>nodes</var> be <var>node</var>'s <a>children</a> if <var>node</var> is a
{{DocumentFragment}} <a>node</a>, « <var>node</var> » otherwise.
<!-- This needs to come before insert as that removes the children of a
DocumentFragment node. -->

Expand All @@ -2637,11 +2635,12 @@ To <dfn export for=Node id=concept-node-replace-all>replace all</dfn> with a
<li>Let <var>removedNodes</var> be <var>parent</var>'s
<a>children</a>.

<li>Let <var>addedNodes</var> be the empty list if <var>node</var> is
null, <var>node</var>'s <a>children</a> if
<var>node</var> is a {{DocumentFragment}}
<a>node</a>, and a list containing <var>node</var>
otherwise.
<li><p>Let <var>addedNodes</var> be the empty list.

<li><p>If <var>node</var> is {{DocumentFragment}} <a>node</a>, then set <var>addedNodes</var> to
<var>node</var>'s <a>children</a>.

<li><p>Otherwise, if <var>node</var> is non-null, set <var>addedNodes</var> to « <var>node</var> ».
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@domenic this is the only change that felt a little weird here due to it getting so much longer, but it's probably better?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this matches the code better. Nested ternaries are not great in either English or code, IMO. You could also use dl class="switch".


<li><a for=/>Remove</a> all
<var>parent</var>'s <a>children</a>, in
Expand Down