since recently, when encoding time.Time
, a prefix appears.
What would you recommend to get rid of that again?
That looks like a bug on some recent merge. I'll look to see what was changed recently.
here it shows: https://github.com/go-yaml/yaml/commit/60a2abf4e00318875a661c29b36df7a68e484bf4#diff-3df5f85b48572bebfd409c300c38f308R215
Thanks for the pointer. The IsZero check seems right, but the detail at the end is strange. I'll fix it, but will first catch up with Roger to ser why he chose to do that.
awesome, btw, here's how I ran into it: https://travis-ci.org/mro/ShaarliGo/jobs/351511259#L524
The addition of the !!timestamp
tag was deliberate so that after fixing we don't break backwards compatibility when unmarshaling timestamp-like strings, but with later yaml versions we can still unmarshal timestamps correctly into time.Time values. Backward compatibility is a fragile thing with YAML though, so I may well have got it wrong.
@mro Is this causing any specific problems for you? ISTM that the emitted YAML is still correct even though it differs from previous output. It also still works OK when unmarshaled by the previous version of the YAML. The !!timestamp
tag is technically correct, although redundant because timestamps should be given that tag automatically.
@rogpeppe It's not about being technically incorrect. We just want timestamps to look nice by default.
I'll look into this.
@niemeyer The problem is that previous versions of go-yaml did not interpret timestamps correctly, so a string that looks like a timestamp would be marshaled as something that by the YAML spec should be treated as a timestamp not a string.
For that reason, even though we now support timestamps, when we're unmarshaling something that looks like a timestamp into an interface{}, we don't want it to end up as a time.Time value, because that would break backward compatibility.
Another aspect of the changes is that if you now try to marshal a string that looks like a timestamp, it will be correctly quoted, so that it can't be be corrupted by being read as a timestamp by other YAML systems. For example, 9999-99-99
matches the timestamp regexp in http://yaml.org/type/timestamp.html, but is unlikely to be unmarshaled correctly by a parser that sees it as a timestamp.
Perhaps the right solution here is:
!!timestamp
tag.This means that unmarshaling into an interface{}
will never give you a time.Time
value unless someone has explicitly added a !!time
tag, but perhaps that's OK.
For that reason, even though we now support timestamps, when we're unmarshaling something that looks like a timestamp into an interface{}
That's irrelevant here. That's done at decoding time, and there's an explicit check for that. Which can remain there.
Another aspect of the changes is that if you now try to marshal a string that looks like a timestamp, it will be correctly quoted,
Also irrelevant here. The simple fact an implicit value resolves as a timestamp is enough to tell it's supposed to be quoted when being encoded. We do that today, and can continue doing that whether we decide to explicitly tag timestamps or not.
This means that unmarshaling into an interface{} will never give you a time.Time value unless someone has explicitly added a !!time tag, but perhaps that's OK.
Again, that's already the case today. We have explicit logic for that.
With all that said, I'm tempted to not touch this logic in v2 anymore and continue focusing on v3 which can have the proper behavior for all of that and implicitly decode as time.Time even in interface{}, which we cannot do on v2.
I'm tempted to not touch this logic in v2 anymore
@niemeyer does _not touch_ mean _restore as was before_ or _leave as is since recently_?
Annotating a RFC3339 with additional goo to explicitly express it's a RFC3339 feels overly verbose to me and doesn't go well with Go's _sensible defaults_ paradigm.
What other serialisation approach would you recommend for human-legible, machine generated, moderately nested data? v2 was perfect.
@mro The behavior was inconsistent before, depending on whether the time was a value or a pointer, and the tags were already there as you can see in the tests touched by @rogpeppe's change. The modification simply made it consistent, which is hard to argue against. So simply going back is not an option - either we change it so it never marshals the tag, and risk breaking compatibility, or keep the current behavior, which is safe.
That's why I'm considering working on v3 so we get the exact behavior we want. So what I would recommend is using that.
@niemeyer Actually, the tags were introduced by https://github.com/go-yaml/yaml/pull/310, so I think we could go back if we wanted.
Thanks for pointing that out. I will go ahead and fix v2 then so it doesn't marshal such tags out.
btw, you folks are awesome.
Most helpful comment
Thanks for pointing that out. I will go ahead and fix v2 then so it doesn't marshal such tags out.