-
Notifications
You must be signed in to change notification settings - Fork 75
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
Remove tar combinations from compression format #133
Remove tar combinations from compression format #133
Conversation
It's been some time since I have been thinking about a rewrite of pub struct Extension {
compression_formats: Vec<CompressionFormat>,
display_text: String, // or &str or OsStr or OsString, idk
} This would allow us removing Tgz and Tlzma from that enum too. So the extension Extension { compression_formats: vec![Tar, Gz], display_text: "tgz".into() } Cause the code in this PR will likely break this piece of code: Line 63 in a02eb45
In this line, we get all the compression formats and use them to compose this string of extensions, so we can have the flexibility to change This requires the Lines 28 to 47 in a02eb45
Of For example: If the user typed Ideas:
(See #88 too) |
Is this about what you were thinking? |
or we could just not store the extension strings and get it from the original path every time we want to print an error |
That's also possible, but in that case we would need come with another system to do the parsing and reformatting again, could happen, but may be harder to implement. |
Merged with some warnings still, but I can fix those. Maybe the missing_docs lints is too aggressive, idk. |
Oops, found one bug:
The last line:
Should be EDIT: is it really a regression tho? Or was my code there always wrong? |
Well that case used to panic before so overall it's an improvement I'd say haha |
Bug 2 comments above was fixed by #219 🙂. |
This allows us to remove a lot of duplicate code and make things simpler. It also doesn't remove any functionality as far as I can see.