barasher / go-exiftool

Golang wrapper for Exiftool : extract as much metadata as possible (EXIF, ...) from files (pictures, pdf, office documents, ...)
GNU General Public License v3.0
245 stars 43 forks source link

Adding the PrintGroupNames option #67

Closed agorman closed 1 year ago

agorman commented 1 year ago

I'd like to use the -G option so I'd like to add the PrintGroupNames method.

Thanks!

agorman commented 1 year ago

Bump. Don't want to bother you but hoping you take a look if you have a second.

barasher commented 1 year ago

Hello. I'm sorry but I'm on holidays, without any computer, I'll be back in one week. Sincerely Arnaud

Le mer. 10 mai 2023, 06:27, Andy Gorman @.***> a écrit :

Bump. Don't want to bother you but hoping you take a look if you have a second.

— Reply to this email directly, view it on GitHub https://github.com/barasher/go-exiftool/pull/67#issuecomment-1541014614, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA62VTFSEBZXULQC3MTET5TXFLHHXANCNFSM6AAAAAAXWKGFLU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

agorman commented 1 year ago

@barasher thank you for the response. Enjoy your holiday. Talk when you get back.

agorman commented 1 year ago

@barasher I believe you should be back now. If so can you take a look. If not then feel free to ignore this until you return.

Thanks!

barasher commented 1 year ago

Hi @agorman ! Once again sorry for the delay.

There's a little thing that bothers me with this implementation. You've chosen to use the -G option that is a shortcut to -G0. It fits your use-case, but I think that it isn't generic enough : some other people might prefer the -G1 option for instance.

What about adding another parameter that specifies which "number" to use ?

Sincerely, Arnaud

agorman commented 1 year ago

Great point. Because the -G option allows for multiple group number (-G[*NUM*][:*NUM*...]) I can change the PrintGroupNames function to take a string. That way a 0 could be passed or multiple group numbers like 3:1.

What do you think?

agorman commented 1 year ago

Alternatively I could change PrintGroupNames to take ...int. That way if nothing was passed I could assume 0. Otherwise I could create the group names string based on the passed ints.

barasher commented 1 year ago

Great point. Because the -G option allows for multiple group number (-G[*NUM*][:*NUM*...]) I can change the PrintGroupNames function to take a string. That way a 0 could be passed or multiple group numbers like 3:1.

What do you think?

I think that's the best option ! A string parameter would be great.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.26 :tada:

Comparison is base (caa7545) 80.54% compared to head (c2a9c7d) 80.80%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #67 +/- ## ========================================== + Coverage 80.54% 80.80% +0.26% ========================================== Files 2 2 Lines 293 297 +4 ========================================== + Hits 236 240 +4 Misses 40 40 Partials 17 17 ``` | [Impacted Files](https://app.codecov.io/gh/barasher/go-exiftool/pull/67?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [exiftool.go](https://app.codecov.io/gh/barasher/go-exiftool/pull/67?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZXhpZnRvb2wuZ28=) | `72.85% <100.00%> (+0.52%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

barasher commented 1 year ago

It seems great !

barasher commented 1 year ago

When running unit-tests, I have an error:

--- FAIL: TestPrintGroupNames (0.65s)
    exiftool_test.go:465: 
            Error Trace:    exiftool_test.go:465
            Error:          Expected nil, but got: &errors.errorString{s:"key not found"}
            Test:           TestPrintGroupNames
    exiftool_test.go:466: 
            Error Trace:    exiftool_test.go:466
            Error:          Not equal: 
                            expected: "64"
                            actual  : ""

                            Diff:
                            --- Expected
                            +++ Actual
                            @@ -1 +1 @@
                            -64
                            +
            Test:           TestPrintGroupNames

I tried:

barasher@linux:~/go/src/github.com/barasher/go-exiftool$ exiftool -j -G0:1:2:3:4:5:6:7 ./testdata/20190404_131804.jpg | grep Width
  "EXIF:ExifIFD:Image:Main:JPEG-APP1-IFD0-ExifIFD:int32u:ExifImageWidth": 4032,
  "EXIF:IFD1:Image:Main:Copy1:JPEG-APP1-IFD1:int32u:ImageWidth": 504,
  "File:Image:Main:JPEG-SOF0:ImageWidth": 64,

My exiftool version:

barasher@linux:~/go/src/github.com/barasher/go-exiftool$ exiftool -ver
11.88

What's your version ? Mine is outdated, I'll update it and try again.

barasher commented 1 year ago

I've updated exiftool and all tests are passing

barasher commented 1 year ago

Once more, thanks for the PR ! Released in v1.10.0