Skip to content

Conversation

@FineFindus
Copy link
Contributor

Implements support for reading the uncompressed size without decompressing the input. As this is not supported by all formats, None may be returned.

@FineFindus FineFindus marked this pull request as ready for review October 13, 2025 19:53
@FineFindus FineFindus force-pushed the feat/uncompressed-size branch from 82a6995 to 30e2a2b Compare October 13, 2025 19:57
Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, one suggestion for the API

@FineFindus FineFindus force-pushed the feat/uncompressed-size branch from 30e2a2b to 08fb5a6 Compare October 14, 2025 08:14
@FineFindus
Copy link
Contributor Author

Changed.
After thinking about it a bit more, maybe it would be better to move this to a separate trait to more clearly indicate that the compression format does not support retrieving the uncompressed size? Since right now None can indicate either that a format does not support reading the uncompressed size/it's not yet implemented or an error occurred whilst reading. What's your opinion on this?

@NobodyXu
Copy link
Collaborator

Yeah putting it in a separate trait sounds reasonable, and , I suggest we could name it

trait DecodedSize {
    fn get_decoded_size(input: &[u8]) -> Result<usize>;
}
}

I also notice that this function does not depend on the state of decoder, so we should make that clear.

We could also just expose it as a regular function

Adds a new trait that to retrieve the size of the data when uncompressed. The
implementation of this trait on a format indicates that a format may support
retrieving the uncompressed size, however it's not guaranteed.
@FineFindus FineFindus force-pushed the feat/uncompressed-size branch from 04856e1 to dc031f3 Compare October 14, 2025 09:10
@FineFindus FineFindus force-pushed the feat/uncompressed-size branch from dc031f3 to 0738561 Compare October 14, 2025 09:14
@FineFindus
Copy link
Contributor Author

I also notice that this function does not depend on the state of decoder, so we should make that clear.

Not for the ones I implemented, but I didn't want to force a breaking change when adding other/future formats (for example gzip has an ISIZE field, but that's limited to 4GB, which is why I didn't implemented it).

We could also just expose it as a regular function

I think a trait works better here, as that allows it to be used in trait bounds.

Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'm currently working on refactoring on this repo, so I planned to cut a new release in at least one week's time

If you need it now, I can try to cut a release this week instead

@NobodyXu NobodyXu added this pull request to the merge queue Oct 15, 2025
Merged via the queue into Nullus157:main with commit f072ce5 Oct 15, 2025
20 checks passed
@codecov
Copy link

codecov bot commented Oct 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (1fa960f) to head (eb0efd1).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff     @@
##   main   #396   +/-   ##
===========================
===========================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot mentioned this pull request Oct 13, 2025
@FineFindus FineFindus deleted the feat/uncompressed-size branch October 15, 2025 17:17
@FineFindus
Copy link
Contributor Author

Thanks, I'm happy to wait (or use a git commit :))

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