rexif icon indicating copy to clipboard operation
rexif copied to clipboard

Refactor a lot of library internals

Open Ortham opened this issue 8 years ago • 1 comments

I was going to try adding limited write support for GPS tags to rexif for my own use, but found how it was implemented difficult to work with, so started to refactor the library internals to more easily accommodate write support, and add some tests.

However, since then I've noticed that kamadak/exif-rs does pretty much the same thing as rexif, but with an implementation that's neater than my own refactoring efforts, and it already has tests, so I've pretty much jumped ship. It would be great if the two of you could join forces, because it's a pretty small niche, and having one preferred pure-Rust EXIF parser would be great.

Either way I thought I'd make this PR in case you wanted to merge some or all of what I've done. I realise there's a lot of different bits to it, so if you like some but not others, I can split it up into smaller PRs.

Ortham avatar Jul 07 '17 16:07 Ortham

Thanks for the message, I will check how to collaborate with the exif-rs project.

-- Elvis Pfutzenreuter +55 47 98801 6105 https://epxx.co

On 7 Jul 2017, at 13:38, Oliver Hamlet [email protected] wrote:

I was going to try adding limited write support for GPS tags to rexif for my own use, but found how it was implemented difficult to work with, so started to refactor the library internals to more easily accommodate write support, and add some tests.

However, since then I've noticed that kamadak/exif-rs https://github.com/kamadak/exif-rs does pretty much the same thing as rexif, but with an implementation that's neater than my own refactoring efforts, and it already has tests, so I've pretty much jumped ship. It would be great if the two of you could join forces, because it's a pretty small niche, and having one preferred pure-Rust EXIF parser would be great.

Either way I thought I'd make this PR in case you wanted to merge some or all of what I've done. I realise there's a lot of different bits to it, so if you like some but not others, I can split it up into smaller PRs.

You can view, comment on, or merge this pull request online at:

https://github.com/elvis-epx/rexif/pull/10 https://github.com/elvis-epx/rexif/pull/10 Commit Summary

Remove IfdEntry::ext_data field Remove IfdEntry::ifd_data field Replace ExifEntry::unit with a member function Replace ifdformat_new() with IfdFormat::new() Refactor IfdFormat size() Use 16-bit literals for ExifTag values Fix some Exif tag names Tidy up self match references Add IfdTag enum to wrap ExifTag Remove ifd field from ExifEntry Use underscores for unused match variables Don't cast IfdFormat in TagValue::Invalid Refactor IfdEntry -> ExifEntry conversion Refactor IFD parsing slightly for clarity Return count bounds as an optional tuple Replace numarray_to_string with a ToCsv trait Simplify other_tag() implementation Remove nop formatter Merge the rational_value and rational_values formatters Refactor Exif postprocessing Use "if let" for exifreadable functions Split up the ifdformat module File Changes

M Cargo.toml https://github.com/elvis-epx/rexif/pull/10/files#diff-0 (3) M src/exif.rs https://github.com/elvis-epx/rexif/pull/10/files#diff-1 (396) M src/exifpost.rs https://github.com/elvis-epx/rexif/pull/10/files#diff-2 (349) M src/exifreadable.rs https://github.com/elvis-epx/rexif/pull/10/files#diff-3 (1092) D src/ifdformat.rs https://github.com/elvis-epx/rexif/pull/10/files#diff-4 (132) M src/lib.rs https://github.com/elvis-epx/rexif/pull/10/files#diff-5 (10) M src/main.rs https://github.com/elvis-epx/rexif/pull/10/files#diff-6 (18) M src/tiff.rs https://github.com/elvis-epx/rexif/pull/10/files#diff-7 (96) A src/to_csv.rs https://github.com/elvis-epx/rexif/pull/10/files#diff-8 (51) M src/types.rs https://github.com/elvis-epx/rexif/pull/10/files#diff-9 (259) M src/types_impl.rs https://github.com/elvis-epx/rexif/pull/10/files#diff-10 (343) Patch Links:

https://github.com/elvis-epx/rexif/pull/10.patch https://github.com/elvis-epx/rexif/pull/10.patch https://github.com/elvis-epx/rexif/pull/10.diff https://github.com/elvis-epx/rexif/pull/10.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/elvis-epx/rexif/pull/10, or mute the thread https://github.com/notifications/unsubscribe-auth/AExOgCS8pqVKQD4g4YVFeEikxfOe7qHIks5sLl8JgaJpZM4ORK6R.

elvis-epx avatar Jul 07 '17 18:07 elvis-epx