Anders429 / substring

A substring method for string types.
Apache License 2.0
10 stars 1 forks source link

Use RangeBounds<usize> instead of usize. #9

Open Anders429 opened 3 years ago

Anders429 commented 3 years ago

The parameters for the substring method can be changed to a RangeBounds. This will allow for providing inclusive end points, as well as providing unbounded start/end points (as has been requested in #5).

This would be a breaking change, but I think it will make the library a bit easier to use. I would rather do this than introduce several methods for each specific case. It will also raise MSRV to match what is required for RangeBounds.

Anders429 commented 3 years ago

Here is a rough version of what the new API could be.

I was trying to see about supporting both usize and isize RangeBounds, to possibly support in the future what #6 was requesting, but I don't think it's possible at this time. The issue is that I could implement both RangeBounds<usize> and RangeBounds<isize>, which would be conflicting. Maybe there's some specialization thing that could resolve this in the future, but I don't think it's worth it, frankly. All other indexing in Rust uses only usizes.

Anders429 commented 3 years ago

I've written up two (very similar) versions: This one with respect to grapheme clusters, and this one which does not respect grapheme clusters, instead respecting the char codepoints. They both are O(n), so it just comes down to preference of graphemes vs. no graphemes.

Alternatively, both could be included, with the grapheme one being named grapheme_substring() or something similar. I worry this may become confusing, but it would cover all use cases. I think most dependents of this crate would likely prefer respect to graphemes, since they often use it for visual purposes (for example, hackernews_tui uses substring to shorten URLs for user interface. The readability of this would benefit if graphemes were respected.

Anders429 commented 3 years ago

Moving char v grapheme stuff to #10.