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.

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: consistency in phrasing of ranges and inclusivity/exclusivity #2848

Merged
merged 2 commits into from
Sep 27, 2022

Conversation

michaelficarra
Copy link
Member

I chose to use "within the inclusive range from A to B" or use chained comparisons where possible. For ranges that have an open end, I name each side as either exclusive or inclusive.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

This is a great improvement. Another option tho would be to use [a, b)-style notation throughout - that way it'd be unambiguous which sides were inclusive or exclusive, and all permutations could be represented. Any reason not to prefer that?

spec.html Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
@michaelficarra
Copy link
Member Author

Thoughts on using the term "interval" instead of "range"? What about using (and explaining) the standard interval notation from maths? Any considerations for using this form for both floats and mathematical values?

@ljharb
Copy link
Member

ljharb commented Aug 8, 2022

I think defining and using interval notation would be much less ambiguous than prose.

@michaelficarra
Copy link
Member Author

Updated with far more rigour and renamed ranges to intervals.

spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
@michaelficarra michaelficarra added editor call to be discussed in the next editor call and removed editor call to be discussed in the next editor call labels Aug 9, 2022
@ljharb
Copy link
Member

ljharb commented Aug 11, 2022

@michaelficarra was the decision not to use (a, b] notation?

@michaelficarra
Copy link
Member Author

@ljharb We did not discuss that today in the editor call, but both @bakkot and I prefer this form to the interval notation, so I'm going to stick with it.

spec.html Show resolved Hide resolved
@michaelficarra michaelficarra requested review from a team and bakkot August 12, 2022 18:13
spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated
@@ -1760,17 +1761,17 @@ <h1>The Number Type</h1>
<div class="math-display">
_s_ &times; _m_ &times; 2<sup>_e_</sup>
</div>
<p>where _s_ is 1 or -1, _m_ is an integer such that 2<sup>52</sup> &le; _m_ &lt; 2<sup>53</sup>, and _e_ is an integer such that -1074 &le; _e_ &le; 971.</p>
<p>where _s_ is 1 or -1, _m_ is an integer in the interval from 2<sup>52</sup> (inclusive) to 2<sup>53</sup> (exclusive), and _e_ is an integer in the inclusive interval from -1074 to 971.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

This one really doesn't seem like an improvement.

spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
@michaelficarra michaelficarra requested review from bakkot and a team August 24, 2022 17:22
@michaelficarra michaelficarra added the editor call to be discussed in the next editor call label Aug 24, 2022
spec.html Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
@michaelficarra michaelficarra removed the editor call to be discussed in the next editor call label Aug 24, 2022
@michaelficarra
Copy link
Member Author

I noticed this weird ecmarkup rendering issue, so we may want to hold off merging until it is fixed.

image

Ping @bakkot if you want to open an ecmarkup issue with a repro case. I'm guessing it has to do with the HTML comment?

spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
Copy link
Contributor

@bakkot bakkot left a comment

Choose a reason for hiding this comment

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

LGTM except

  • I am not a fan of the DecimalDigit change, though I am not going to die on that hill
  • needs the just-published ecmarkup v14.1.2 to be upstreamed to fix the rendering issue before this lands (possibly as part of this PR)

@michaelficarra
Copy link
Member Author

@bakkot done

@bakkot bakkot added the editor call to be discussed in the next editor call label Sep 20, 2022
spec.html Outdated Show resolved Hide resolved
@michaelficarra michaelficarra added ready to merge Editors believe this PR needs no further reviews, and is ready to land. and removed editor call to be discussed in the next editor call labels Sep 27, 2022
@ljharb ljharb merged commit 2600c07 into main Sep 27, 2022
@ljharb ljharb deleted the inclusive-range branch September 27, 2022 01:54
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Sep 27, 2022
String.prototype.lastIndexOf must define _searchLen_ before using it.

(fixup for PR tc39#2848)
ljharb pushed a commit to jmdyck/ecma262 that referenced this pull request Sep 27, 2022
String.prototype.lastIndexOf must define _searchLen_ before using it.

(fixup for PR tc39#2848)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change establishes editorial conventions ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants