Editorial: replace steps embedded in lookup tables with more typical algorithms #2915
Conversation
michaelficarra
added
editorial change
editor call
|
so many claps; this is awesome |
|
This seems to me to be worse; I'd mildly prefer to leave as-is. |
|
Worse how? Algorithm steps are so much clearer imo than a table. |
|
I find the table easier to read /shrug |
|
I'm actually really happy with this change. It exceeded my expectations. |
|
The ids of the former |
michaelficarra
force-pushed
the
tables-r-bad
branch
from
40917b1
to
f9e78da
Compare
|
@jmdyck Good point. Done. |
|
I think the algorithmic steps here help clarity over tables, especially when reviewing implementations. It'd also be nice to have this be precedent setting as editorial style to not loop over tables in algorithmic steps. To wit I'd like to tell 404 proposal authors to stop doing stuff like step 17 here in the Intl.DurationFormat proposal. |
|
I'm fine with this approach, then. |
michaelficarra
added
ready to merge
|
@bakkot Addressed comments. |
michaelficarra
added
the
ready to merge
| </emu-table> | ||
| <emu-alg> | ||
| 1. If _argument_ is a Boolean, return _argument_. | ||
| 1. If _argument_ is any of *undefined*, *null*, *+0*<sub>𝔽</sub>, *-0*<sub>𝔽</sub>, *NaN*, *0*<sub>ℤ</sub>, or the empty String, return *false*. |
There was a problem hiding this comment.
PR #2877 proposes "any of" used this way, but the current spec doesn't use it. Instead, the current spec would say:
_argument_ is *undefined*, ..._argument_ is either *undefined*, ..._argument_ is one of *undefined*, ...
There was a problem hiding this comment.
I'm okay with this. It's informal language.
There was a problem hiding this comment.
It's informal language.
It is, but that's true of most of the spec. Every "consistent phrasing" PR changes informal language. So it seems like "informal language" can't be a reason to not aim for consistency. So why bring it up here?
Now, if you were to say "Yeah, it's an anomaly, but #2877 will take care of it soon, so it's not worth fixing here," I'd find that less puzzling.
There was a problem hiding this comment.
@jmdyck I am aiming for consistency, but am okay with more inconsistency for the time being (in informal language) if it's heading in the direction we want to go, as here.
This will help us catch editorial errors (it already caught 1) and IMO reads better by allowing for other groupings (such as by output in
ToBoolean).I'd recommend reviewing before/after rendering instead of reading the changeset.
There's probably more opportunities to replace tables, but this is a good start.