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

Add byte spans #102

Merged
merged 2 commits into from
Mar 7, 2024
Merged

Add byte spans #102

merged 2 commits into from
Mar 7, 2024

Conversation

VonTum
Copy link
Contributor

@VonTum VonTum commented Feb 29, 2024

I've added the possibility to use byte spans at the Report level. You can configure using byte spans by calling

report_builder.with_config(Config::default().with_index_type(IndexType::Byte))

I went down a few paths in trying to implement this without API breaks, before settling on making it configurable in the ReportBuilder. Doing it this way allows the user to use the same Span types as before, just with byte indices.

I performed this addition in two steps. Refactoring write.rs to store the computed char span in LabelInfo's char_span : Range<usize> field everywhere. In the second part I convert the given byte spans to char spans, if the Report's been configured to.

This way LabelInfo doesn't need to depend on the particular impl of Span. This also removes the redundancy in LabelInfo, because labels of the same file are stored in the same SourceGroup anyway.

This is in preparation for switching over to byte spans.
Closes zesterer#8
Closes Duplicate issues zesterer#71 and zesterer#57
@zesterer
Copy link
Owner

zesterer commented Mar 4, 2024

Thanks for the PR! Sorry about the delay in responding, I'm going to try to find time to review this over the next day.

@zesterer
Copy link
Owner

zesterer commented Mar 7, 2024

Thanks for the superb PR. Usually with a PR of this length I challenge myself to find at least one problem with it, but I'm at a loss here: all looks good. Thanks again!

@zesterer zesterer merged commit a061071 into zesterer:main Mar 7, 2024
1 check passed
@zesterer
Copy link
Owner

zesterer commented Mar 7, 2024

Reminder to self: update changelog and bump version

@VonTum
Copy link
Contributor Author

VonTum commented Mar 7, 2024

That is quite the compliment! Thank you 😄

@goto-bus-stop
Copy link
Contributor

great news, thank you both! 🥳

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

3 participants