ecma262 icon indicating copy to clipboard operation
ecma262 copied to clipboard

Editorial: replace steps embedded in lookup tables with more typical algorithms

Open michaelficarra opened this issue 3 years ago • 10 comments

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.

michaelficarra avatar Sep 27 '22 01:09 michaelficarra

so many claps; this is awesome

ljharb avatar Sep 27 '22 01:09 ljharb

This seems to me to be worse; I'd mildly prefer to leave as-is.

bakkot avatar Sep 27 '22 01:09 bakkot

Worse how? Algorithm steps are so much clearer imo than a table.

ljharb avatar Sep 27 '22 01:09 ljharb

I find the table easier to read /shrug

bakkot avatar Sep 27 '22 01:09 bakkot

I'm actually really happy with this change. It exceeded my expectations.

michaelficarra avatar Sep 27 '22 01:09 michaelficarra

The ids of the former <emu-table> elements should maybe become oldids on the respective <emu-clause> elements.

jmdyck avatar Sep 27 '22 17:09 jmdyck

@jmdyck Good point. Done.

michaelficarra avatar Sep 27 '22 18:09 michaelficarra

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.

syg avatar Sep 28 '22 22:09 syg

I'm fine with this approach, then.

bakkot avatar Sep 29 '22 04:09 bakkot

@syg @bakkot Can I get a review for correctness then?

michaelficarra avatar Sep 30 '22 17:09 michaelficarra

@bakkot Addressed comments.

michaelficarra avatar Nov 01 '22 15:11 michaelficarra