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

feat(SourceSpan): add impl From<InclusiveRange> #385

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

Benjamin-L
Copy link
Contributor

This can be used to avoid awkward start..(end + 1) constructions, which will trigger the clippy::range_plus_one lint.

I added a single impl for InclusiveRange rather than a blanket impl<T: RangeBounds> From<T> for SourceSpan for two reasons. The first is that the blanket impl would be a breaking change (because dependent crates may currently define a type T: RangeBounds and also have impl From<T> for SourceSpan). The second is that this would allow one-sided ranges (..x, x..). In order to support bounded-below ranges, we would need to change SourceSpan so that length may depend on the the specific SourceCode instance. That seems like an unlikely use case.

This can be used to avoid awkward `start..(end + 1)` constructions,
which will trigger the `clippy::range_plus_one` lint.

I added a single impl for `InclusiveRange` rather than a blanket
`impl<T: RangeBounds> From<T> for SourceSpan` for two reasons. The first
is that the blanket impl would be a breaking change (because dependent
crates may currently define a type `T: RangeBounds` and also have `impl
From<T> for SourceSpan`). The second is that this would allow one-sided
ranges (`..x`, `x..`). In order to support bounded-below ranges, we
would need to change `SourceSpan` so that `length` may depend on the the
specific `SourceCode` instance. That seems like an unlikely use case.
@Benjamin-L
Copy link
Contributor Author

CI should be fixed by #382

@zkat zkat merged commit 73da45b into zkat:main Jun 26, 2024
2 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants