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

Fix quotation for timestamps #515

Merged
merged 1 commit into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions encode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ func TestEncoder(t *testing.T) {
nil,
},
{
"t2: 2018-01-09T10:40:47Z\nt4: 2098-01-09T10:40:47Z\n",
"t2: \"2018-01-09T10:40:47Z\"\nt4: \"2098-01-09T10:40:47Z\"\n",
map[string]string{
"t2": "2018-01-09T10:40:47Z",
"t4": "2098-01-09T10:40:47Z",
Expand Down Expand Up @@ -717,14 +717,16 @@ func TestEncoder(t *testing.T) {
},
}
for _, test := range tests {
var buf bytes.Buffer
enc := yaml.NewEncoder(&buf, test.options...)
if err := enc.Encode(test.value); err != nil {
t.Fatalf("%+v", err)
}
if test.source != buf.String() {
t.Fatalf("expect = [%s], actual = [%s]", test.source, buf.String())
}
t.Run(test.source, func(t *testing.T) {
var buf bytes.Buffer
enc := yaml.NewEncoder(&buf, test.options...)
if err := enc.Encode(test.value); err != nil {
t.Fatalf("%+v", err)
}
if test.source != buf.String() {
t.Fatalf("expect = [%s], actual = [%s]", test.source, buf.String())
}
})
}
}

Expand Down
33 changes: 20 additions & 13 deletions token/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"strconv"
"strings"
"time"
)

// Character type for character
Expand Down Expand Up @@ -600,20 +601,26 @@ func ToNumber(value string) *NumberValue {
}
}

func looksLikeTimeValue(value string) bool {
Copy link
Collaborator Author

@shuheiktgw shuheiktgw Nov 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've combine it into the isTimestamp function

for i, c := range value {
switch c {
case ':', '1', '2', '3', '4', '5', '6', '7', '8', '9':
continue
case '0':
if i == 0 {
return false
}
continue
// This is a subset of the formats permitted by the regular expression
// defined at http://yaml.org/type/timestamp.html. Note that time.Parse
// cannot handle: "2001-12-14 21:59:43.10 -5" from the examples.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why didn't you provide a layout to handle this case ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I've actually tried supporting the layout, but Go does not seem to support the "-timezone" representation...

https://go.dev/play/p/a5UvI5cUlJy

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK ! I understand !

var timestampFormats = []string{
time.RFC3339Nano,
"2006-01-02t15:04:05.999999999Z07:00", // RFC3339Nano with lower-case "t".
time.DateTime,
time.DateOnly,

// Not in examples, but to preserve backward compatibility by quoting time values.
"15:4",
}

func isTimestamp(value string) bool {
for _, format := range timestampFormats {
if _, err := time.Parse(format, value); err == nil {
return true
}
return false
}
return true
return false
}

// IsNeedQuoted whether need quote for passed string or not
Expand All @@ -637,7 +644,7 @@ func IsNeedQuoted(value string) bool {
case ':', ' ':
return true
}
if looksLikeTimeValue(value) {
if isTimestamp(value) {
return true
}
for i, c := range value {
Expand Down
11 changes: 9 additions & 2 deletions token/token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ func TestIsNeedQuoted(t *testing.T) {
"true",
"1.234",
"1:1",
"2001-12-15T02:59:43.1Z",
"2001-12-14t21:59:43.10-05:00",
"2001-12-15 2:59:43.10",
"2002-12-14",
"hoge # comment",
"\\0",
"#a b",
Expand Down Expand Up @@ -128,15 +132,18 @@ func TestIsNeedQuoted(t *testing.T) {
}
for i, test := range needQuotedTests {
if !token.IsNeedQuoted(test) {
t.Fatalf("%d: failed to quoted judge for %s", i, test)
t.Errorf("%d: failed to quoted judge for %s", i, test)
}
}
notNeedQuotedTests := []string{
"Hello World",
// time.Parse cannot handle: "2001-12-14 21:59:43.10 -5" from the examples.
// https://yaml.org/type/timestamp.html
"2001-12-14 21:59:43.10 -5",
}
for i, test := range notNeedQuotedTests {
if token.IsNeedQuoted(test) {
t.Fatalf("%d: failed to quoted judge for %s", i, test)
t.Errorf("%d: failed to quoted judge for %s", i, test)
}
}
}
Loading