'ParsePatchDate' is broken :/

[sevki]

time.Parse isn't guaranteed to throw errors, when it sees elements that it doesn't recognize it zeros them. https://play.golang.org/p/4kbScfG56Ic

Elements omitted from the value are assumed to be zero or, when zero is impossible, one, so parsing "3:04pm" returns the time corresponding to Jan 1, year 0, 15:04:00 UTC (note that because the year is 0, this time is before the zero Time). Years must be in the range 0000..9999. The day of the week is checked for syntax but it is otherwise ignored.

• https://golang.org/pkg/time/#example_Parse

It might be worth checking the time to see if the year is 1 in addition to checking if err is nil, seeing that git was released in 2005 :D I know this would make it unusable for time travelers but 🤷

Personally I would just use something like https://github.com/kierdavis/dateparser https://play.golang.org/p/-yRXt4qPAZo

[bluekeyes]

Thanks for reporting this with a playground example. After looking at it, time.Parse returns errors as I expected, but my parse string was too strict. It required zero-padded days, which are not necessary according to Git. That is fixed by #14.

The general behavior about zero values is intentional. The supported formats all include the year, month, and day, and zero is an appropriate default for the one format missing the time. While this may change in the future as people use it more, the contract for now is that if Parsed is the zero-value (i.e. IsParsed() returns false), then the format is unknown and clients should either implement their own parsing on the Raw field or use a different format for input patches. My thought is that supporting a strict set of formats output by Git (assuming I actually get them correct!) is less confusing than trying to parse a range of inputs and then finding workarounds or adding options when the parser does the wrong thing.

[sevki]

ah nice. Had I tried this on tried today's date it would have parsed correctly ha 😄

My personal experience with contracts that may change in the future has not been great, as codified by Hyrum's law

With a sufficient number of users of an API,
it does not matter what you promise in the contract:
all observable behaviors of your system
will be depended on by somebody.

It does not look as tho the strict set of outputs are that hard to get correct

DATE FORMATS
------------

The `GIT_AUTHOR_DATE`, `GIT_COMMITTER_DATE` environment variables
ifdef::git-commit[]
and the `--date` option
endif::git-commit[]
support the following date formats:

Git internal format::
	It is `<unix timestamp> <time zone offset>`, where `<unix
	timestamp>` is the number of seconds since the UNIX epoch.
	`<time zone offset>` is a positive or negative offset from UTC.
	For example CET (which is 1 hour ahead of UTC) is `+0100`.

RFC 2822::
	The standard email format as described by RFC 2822, for example
	`Thu, 07 Apr 2005 22:13:13 +0200`.

ISO 8601::
	Time and date specified by the ISO 8601 standard, for example
	`2005-04-07T22:13:13`. The parser accepts a space instead of the
	`T` character as well. Fractional parts of a second will be ignored,
	for example `2005-04-07T22:13:13.019` will be treated as
	`2005-04-07T22:13:13`.
+
NOTE: In addition, the date part is accepted in the following formats:
`YYYY.MM.DD`, `MM/DD/YYYY` and `DD.MM.YYYY`.

Would you be happy if I sent a PR that removes IsParsed and only parses the spec dates?

[bluekeyes]

The problem I ran into is that the set of output date formats allowed by git diff and git show is more varied, including the possibility of custom formats. ParsePatchDate supports what I judged were the most common and well-formed of those formats.

That said, I wasn't very attached to the PatchDate type and you've convinced me that something simpler is better. I put up #15, which switches to plain time.Time and returns an error for unknown formats. Is something like that what you had in mind?

[sevki]

Yeah, exactly what I had in mind! Thank you kindly sir!