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

Warn user if file extension is passed as file name #277

Merged
merged 2 commits into from
Oct 12, 2022

Conversation

marcospb19
Copy link
Member

example: .tar.gz is a file named .tar with extension .gz

this can be confusing because people might expect a
path '.tar.gz' to be a .tar.gz archive, but it's not

@marcospb19
Copy link
Member Author

Branch name was supposed to be waRn-file-extension-passed-as-file-name, not Wann, I think if I change it now the PR will break so I'll leave it like this xD

src/extension.rs Outdated Show resolved Hide resolved
src/extension.rs Outdated Show resolved Hide resolved
@figsoda
Copy link
Member

figsoda commented Oct 11, 2022

Unrelated to this, but using split might have some performance gain over what we have right now, especially when we have to do extensions.reverse()

@marcospb19
Copy link
Member Author

Unrelated to this, but using split might have some performance gain over what we have right now, especially when we have to do extensions.reverse()

Yeah, but in the context of compressing and decompressing files, I think that the performance gain from parsing the path is negligible (this also runs only for CLI given args I think).

example: .tar.gz is a file named .tar with extension .gz

this can be confusing because people might expect .tar.gz to be a
.tar.gz archive, but it's currently not
@marcospb19 marcospb19 force-pushed the wann-file-extension-passed-as-file-name branch from 18cc40a to 4f0839f Compare October 11, 2022 23:58
@marcospb19 marcospb19 merged commit e12c25e into main Oct 12, 2022
@figsoda figsoda deleted the wann-file-extension-passed-as-file-name branch October 12, 2022 00:19
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