Return error from Isformat() and include in DoesNotMatchFormatError ErrorDetails #300
Conversation
Now -run is more useful and the logs look like this:
--- PASS: TestFormats (0.06s)
--- PASS: TestFormats/draft4 (0.01s)
--- PASS: TestFormats/draft4/validation_of_date-time_strings (0.00s)
--- PASS: TestFormats/draft4/validation_of_date-time_strings/a_valid_date-time_string (0.00s)
--- PASS: TestFormats/draft4/validation_of_date-time_strings/an_invalid_date-time_string (0.00s)
--- PASS: TestFormats/draft4/validation_of_date-time_strings/only_RFC3339_not_all_of_ISO_8601_are_valid (0.00s)
--- PASS: TestFormats/draft4/validation_of_URIs (0.00s)
--- PASS: TestFormats/draft4/validation_of_URIs/a_valid_URL_with_anchor_tag (0.00s)
--- PASS: TestFormats/draft4/validation_of_URIs/a_valid_URL_with_anchor_tag_and_parantheses (0.00s)
Update README.md err.Type() to include "format" (and sort the list). Add comment on ErrBadFormat. Update comment on FormatChecker IsFormat.
There was a problem hiding this comment.
Nice work @timbunce ! Man, it's so much simpler if we just make things not backwards compatible lol. Way less code change and more user-friendly than what I suggested.
Obviously, still wait on @johandorland 's input.
| if err := FormatCheckers.IsFormat(currentSubSchema.format, value); err != nil { | ||
| details := ErrorDetails{"format": currentSubSchema.format} | ||
| if err != ErrBadFormat { | ||
| details["error"] = err |
There was a problem hiding this comment.
NIT but can we potentially change error to fmtError? Right now, ErrorDetails is overloaded with a lot of other key names like expected, reference, format, etc. so error becomes extremely unclear in what context this key should be used or overloaded.
Or maybe call it customMsg or customError if we want this custom error injection in other validation checks in the future.
There was a problem hiding this comment.
ErrorDetails holds "a map of details specific to each error". In this case it's a DoesNotMatchFormatError type error.
I think the error key fits in very well with the existing general key names like expected, reference, format, etc. @johandorland would be better able to judge though.
In looking into this I noticed that I hadn't updated the relevant comment in errors.go, so I've done that now in 4a6c4b6.
…mment Also add `-` to another ErrorDetails comment to be consistent with others.
|
@johandorland The tests are failing due to maps being unordered in %v in Go v1.11. I could work around that if you're like, let me know, but it would be simpler to just drop Go 1.11 from the Travis config (and add 1.14). |
|
Ok, that's not a problem. It could be a while before I have the time to look through this thorougly, but don't worry, it's on the list. |
|
Anything I can do to help, @johandorland? |
|
Hey @johandorland, anything I can do to help? |
|
Hello, |
Perhaps best to review the commits individually. (Ignoring whitespace helps with the indentation change in fd8e6d1.)
The first three commits are backwards compatible:
The last few commits comprise the desired changes:
This is based on @Ferix9288's idea and work in #254.
I've just simplified it after @johandorland confirmed he was ok with tagging a v2.