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

DateValue is now a bit more permissive with malformed dates #2146

Merged
merged 1 commit into from
Mar 12, 2022

Conversation

piponazo
Copy link
Collaborator

In #1547 we got stricter about the ISO 8601 date formats. This PR should fix #2144 by being a bit more permissive with dates containing 0s in the month or day fields (check the notes from IPTC ).

@piponazo
Copy link
Collaborator Author

@jim-easterbrook let me know if this would be enough for your special case, or you think we should consider some additional cases.

@codecov
Copy link

codecov bot commented Mar 11, 2022

Codecov Report

Merging #2146 (56c52be) into main (7aae68e) will increase coverage by 0.00%.
The diff coverage is 61.11%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2146   +/-   ##
=======================================
  Coverage   63.31%   63.31%           
=======================================
  Files          97       97           
  Lines       19127    19138   +11     
  Branches     9713     9718    +5     
=======================================
+ Hits        12111    12118    +7     
- Misses       4748     4751    +3     
- Partials     2268     2269    +1     
Impacted Files Coverage Δ
src/value.cpp 72.61% <61.11%> (-0.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7aae68e...56c52be. Read the comment docs.

@jim-easterbrook
Copy link
Contributor

jim-easterbrook commented Mar 12, 2022

I'll wait for the nightly build incorporating this change before I test it. In #2144 I should have also tested setting dates with zeros in - this also fails with v0.27.5 and builds of 1.0.0 from a few days ago.

I'm not sure I like the setting out of range months and days to 0. I think it's libexiv2's job to read whatever is in the file and leave it to the application program to make sense of it.

(As for the title of this patch - it's not a malformed date, it's IPTC standard compliant!)

@jim-easterbrook
Copy link
Contributor

Just realised mainDatesWithZeros is not merged yet, so won't be in the next nightly build. I've checked out the branch and built it myself. It's working as I'd expect it to so I'm happy with this PR.

@piponazo piponazo merged commit 5d08bb9 into main Mar 12, 2022
@piponazo piponazo deleted the mainDatesWithZeros branch March 12, 2022 15:56
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.

Problems with IPTC dates containing zero for month or day
3 participants