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

Added RangeInclusive Span support. #129

Merged
merged 2 commits into from
Sep 12, 2024

Conversation

scriptandcompile
Copy link
Contributor

This should remove the annoying clippy inclusive range suggestion for single character ranges which is fairly common.

This is a relatively minor change of two impl's which mirror the Range versions fairly closely.

…ippy inclusive suggestion for single character ranges which is fairly common.
@zesterer
Copy link
Owner

zesterer commented Sep 5, 2024

This doesn't seem correct implementation-wise: Shouldn't the end of the span be one past the inclusive end?

@scriptandcompile
Copy link
Contributor Author

No? We just want the start and the end of the range. How that is handled internally by the type is irrelevant to us.

The big difference here is that we want to support:

("file_name.py", 0..5) as well as ("file_name.py", 0..=4)

The first uses Range, the second uses RangeInclusive, but they should both return the same values.

@scriptandcompile
Copy link
Contributor Author

Apparently, I am not a smart man.

Just tested it and yup. You were right. had to add the +1 there. =P

@scriptandcompile
Copy link
Contributor Author

Anything else needed for this?

@zesterer
Copy link
Owner

I think this is fine. Thanks for the PR!

@zesterer zesterer merged commit 41b562a into zesterer:main Sep 12, 2024
3 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.

2 participants