Majored / rs-async-zip

An asynchronous ZIP archive reading/writing crate.
MIT License
123 stars 40 forks source link

Handle UTF-8 Extra Fields Automatically #104

Closed ArcticLampyrid closed 9 months ago

ArcticLampyrid commented 11 months ago

Goals

Ensure automatic handling of UTF-8 extra fields.
This feature is CRUCIAL for CJK users since most archives created on Windows utilize extra fields to store the UTF-8 path and comment.

Changes

Major

Minor

Notes on Breaking Changes

Related

Issue #103

Acknowledgement

Special thanks to ChatGPT by OpenAI for assisting in refining this report. Your language skills are greatly appreciated!

ArcticLampyrid commented 11 months ago
error: all variants have the same postfix: `ExtraField`
  --> src/spec/header.rs:53:1
   |
53 | / pub enum ExtraField {
54 | |     Zip64ExtendedInformationExtraField(Zip64ExtendedInformationExtraField),
55 | |     InfoZipUnicodeCommentExtraField(InfoZipUnicodeCommentExtraField),
56 | |     InfoZipUnicodePathExtraField(InfoZipUnicodePathExtraField),
57 | |     UnknownExtraField(UnknownExtraField),
58 | | }
   | |_^
   |
   = help: remove the postfixes and use full paths to the variants instead of glob imports
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names

Clippy flags the ExtraField postfix as an issue, even though this naming convention is used previously, as seen in Zip64ExtendedInformationExtraField and UnknownExtraField. Should we consider removing it?

Based on the documentation, the lint regarding variant names is only triggered when there are a minimum of 3 enum variants. This might explain why we didn't receive any warnings earlier.

Majored commented 10 months ago

Apologies for the late response. Yep, if we could remove the postfixes, that would be perfect.

ArcticLampyrid commented 10 months ago

The back-to-school season just made me lazy. Sorry for my late commit. Postfixes are removed in the commit https://github.com/Majored/rs-async-zip/pull/104/commits/4b53dcde729e1914618ec907ba3fec3945337ce0.

ArcticLampyrid commented 10 months ago

I'm sorry to forget to format the code after renaming the fields. 🥲 I believe it should be okey now.

Majored commented 10 months ago

@ArcticLampyrid I've just merged #106 and it looks like that's caused a few conflicts.

Would you be happy to look into those? No worries if not.

ArcticLampyrid commented 10 months ago

Not to worry. I've resolved the conflicts and retested to ensure everything is in order.

ArcticLampyrid commented 9 months ago

Conflicts introduced from PR #110 are also resolved.

Majored commented 9 months ago

Merging, thanks for all your work on this. 👍