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.

Sign up for GitHub

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

Jump to bottom

Refac + Improve extension parsing #519

Merged
merged 4 commits into from
Sep 7, 2024
Merged

Refac + Improve extension parsing #519

merged 4 commits into from
Sep 7, 2024

Conversation

marcospb19
Copy link
Member

Fixes #517.

marcospb19
marcospb19 commented Sep 18, 2023
View reviewed changes
Copy link
Member Author

@marcospb19 marcospb19 left a comment

Choose a reason for hiding this comment

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

One thing led to another and I ended up making lots of changes unrelated to the fix.

So are the main changes of this PR:

src/extension.rs Show resolved Hide resolved
src/extension.rs Show resolved Hide resolved
src/extension.rs Show resolved Hide resolved
tests/snapshots/ui__ui_test_err_format_flag-3.snap Outdated Show resolved Hide resolved
tests/ui.rs Outdated Show resolved Hide resolved
@marcospb19
Copy link
Member Author

Ah, CI is failing because I'm using the just stabilized OsStr::as_encoded_bytes() which isn't on stable yet.

@figsoda
Copy link
Member

figsoda commented Sep 20, 2023

this seems like a lot, could this be separated into multiple PRs? (or maybe reword the PR to reflect all the changes?)

@marcospb19
Copy link
Member Author

Yeah sorry for that, I'll split the PR into smaller ones.

@marcospb19 marcospb19 force-pushed the fix-extension-parsing branch 2 times, most recently from a7bb308 to 795c254 Compare March 7, 2024 19:43
@marcospb19 marcospb19 changed the title Fix extension parsing Improve extension parsing Mar 7, 2024
@marcospb19 marcospb19 changed the title Improve extension parsing Refac + Improve extension parsing Mar 7, 2024
@marcospb19
Copy link
Member Author

I gave up on separating this into smaller PRs, I'll avoid doing this again, but I did a mess and it would take effort to separate.

So... merging :v (when it passes).

@marcospb19
Add tests to the extension module
8b33328
@marcospb19
Fix --format parsing extensions with dots
c477e9b 
Also improve error reporting for `--format` with malformed or
unsupported extensions

This commit is very messy, as it also does an refac in the project,
which should ideally be in a separated commit
@marcospb19
update CHANGELOG
dbb2ab2
@marcospb19 marcospb19 merged commit 3d717ec into main Sep 7, 2024
82 checks passed
@marcospb19 marcospb19 deleted the fix-extension-parsing branch September 7, 2024 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@figsoda figsoda figsoda left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error message for --format .tar.gz outputs empty extension
2 participants
@marcospb19 @figsoda