Skip to content

Minimal change for unbounded ranges #17

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

Closed
wants to merge 1 commit into from

Conversation

CAD97
Copy link
Collaborator

@CAD97 CAD97 commented Mar 13, 2020

Closes #15

This is what I believe to be the minimal diff to support right-unbounded ranges with TextSize/TextRange. TextSize(u32::MAX) is reserved to mean ∞/unbounded, and the only functions that outright need to be adjusted to respect those semantics are checked_add (just replaced with saturating_add for now), len (return TextSize::INF for unbounded ranges), and the actual indexing.

This added semantics to TextSize does make the following impls questionable, and deserving of a new fresh look:

  • From<u32> for TextSize: u32::MAX is lossy
  • From<TextSize> for u32: TextSize::INF is lossy
  • TryFrom<usize> for TextSize: u32::MAX is lossy
  • From<TextSize> for u32: TextSize::INF is lossy

@CAD97 CAD97 requested a review from matklad March 13, 2020 18:56
@CAD97
Copy link
Collaborator Author

CAD97 commented Mar 13, 2020

As for constructors taking advantage of this, I think a not-horrible option might be fn TextRangeTo(_: TextSize) -> TextRange and fn TextRangeFrom(_: TextSize) -> TextRange. Definitely... not traditional, but work to get the point across.

@matklad
Copy link
Member

matklad commented Mar 13, 2020

Not a fan of it, for roughly the same reason I was not a fan of reversed ranges: this does make one corner case significantly better, but it also expands the (conceptual) set of values a TextRange can have, making the invariants less strict. And that makes the common case slightly more awkward. But, because the common case is so much more common, I feel like overall this adds to awkwardness.

I think usize::from(ts).. and ..usize::from(ts) should be fine. We can also add more targeted hacks like s[ts.suffix()] and s[ts.prefix()] (the hacks returning std::ops::Range{To,From}<usize>).

@CAD97 CAD97 closed this Mar 13, 2020
@CAD97 CAD97 deleted the unbounded branch March 13, 2020 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Idea: RangeFrom
2 participants