diff --git a/yaml/pretty/testdata/cron.yml b/yaml/pretty/testdata/cron.yml index 9bf7360..ed78f46 100644 --- a/yaml/pretty/testdata/cron.yml +++ b/yaml/pretty/testdata/cron.yml @@ -5,7 +5,7 @@ kind: cron spec: branch: master - schedule: "1 * * * *" + schedule: 1 * * * * deployment: diff --git a/yaml/pretty/testdata/cron.yml.golden b/yaml/pretty/testdata/cron.yml.golden index aa5cdae..24baaef 100644 --- a/yaml/pretty/testdata/cron.yml.golden +++ b/yaml/pretty/testdata/cron.yml.golden @@ -3,7 +3,7 @@ version: 1 kind: cron name: nightly spec: - schedule: "1 * * * *" + schedule: 1 * * * * branch: master deployment: target: production diff --git a/yaml/pretty/testdata/manifest.yml.golden b/yaml/pretty/testdata/manifest.yml.golden index 6a55830..b38fa2e 100644 --- a/yaml/pretty/testdata/manifest.yml.golden +++ b/yaml/pretty/testdata/manifest.yml.golden @@ -45,7 +45,7 @@ data: > kind: cron name: nightly spec: - schedule: "1 * * * *" + schedule: 1 * * * * branch: master deployment: target: production diff --git a/yaml/pretty/util.go b/yaml/pretty/util.go index 202a1dc..749543b 100644 --- a/yaml/pretty/util.go +++ b/yaml/pretty/util.go @@ -14,7 +14,11 @@ package pretty -import "github.com/drone/drone-yaml/yaml" +import ( + "strings" + + "github.com/drone/drone-yaml/yaml" +) func isPrimative(v interface{}) bool { switch v.(type) { @@ -63,10 +67,8 @@ func isZero(v interface{}) bool { } } -func isQuoted(b rune) bool { +func isEscapeCode(b rune) bool { switch b { - case '#', ',', '[', ']', '{', '}', '&', '*', '!', '\'', '"', '%', '@', '`': - return true case '\a', '\b', '\f', '\n', '\r', '\t', '\v': return true default: @@ -74,6 +76,61 @@ func isQuoted(b rune) bool { } } +func isQuoted(s string) bool { + // if the string is empty it should be quoted. + if len(s) == 0 { + return true + } + + var r0, r1 byte + t := strings.TrimSpace(s) + + // if the trimmed string does not match the string, it + // has starting or tailing white space and therefore + // needs to be quoted to preserve the whitespace + if t != s { + return true + } + + if len(t) > 0 { + r0 = t[0] + } + if len(t) > 1 { + r1 = t[1] + } + + switch r0 { + // if the yaml starts with any of these characters + // the string should be quoted. + case ',', '[', ']', '{', '}', '*', '"', '\'', '%', '@', '`', '|', '>', '#': + return true + + case '&', '!', '-', ':', '?': + // if the yaml starts with any of these characters, + // followed by whitespace, the string should be quoted. + if r1 == ' ' { + return true + } + } + + var prev rune + for _, b := range s { + switch { + case isEscapeCode(b): + return true + case b == ' ' && prev == ':': + return true + case b == '#' && prev == ' ': + return true + } + prev = b + } + + // if the string ends in : it should be quoted otherwise + // it is interpreted as an object. + return strings.HasSuffix(t, ":") +} + func chunk(s string, chunkSize int) []string { if len(s) == 0 { return []string{s} diff --git a/yaml/pretty/util_test.go b/yaml/pretty/util_test.go index e202aaf..9a38ab1 100644 --- a/yaml/pretty/util_test.go +++ b/yaml/pretty/util_test.go @@ -21,6 +21,73 @@ import ( "github.com/google/go-cmp/cmp" ) +func TestQuoted(t *testing.T) { + tests := []struct{ + before, after string + }{ + {"", `""`}, + {"foo", "foo"}, + + // special characters only quoted when followed + // by whitespace. + {"&foo", "&foo"}, + {"!foo", "!foo"}, + {"-foo", "-foo"}, + {":foo", ":foo"}, + + {"& foo", `"& foo"`}, + {"! foo", `"! foo"`}, + {"- foo", `"- foo"`}, + {": foo", `": foo"`}, + + {" & foo", `" & foo"`}, + {" ! foo", `" ! foo"`}, + {" - foo", `" - foo"`}, + {" : foo", `" : foo"`}, + + // special characters only quoted when it is the + // first character in the string. + {",foo", `",foo"`}, + {"[foo", `"[foo"`}, + {"]foo", `"]foo"`}, + {"{foo", `"{foo"`}, + {"}foo", `"}foo"`}, + {"*foo", `"*foo"`}, + {`"foo`, `"\"foo"`}, + {`'foo`, `"'foo"`}, + {`%foo`, `"%foo"`}, + {`@foo`, `"@foo"`}, + {`|foo`, `"|foo"`}, + {`>foo`, `">foo"`}, + {`#foo`, `"#foo"`}, + + {`foo:bar`, `foo:bar`}, + {`foo :bar`, `foo :bar`}, + {`foo: bar`, `"foo: bar"`}, + {`foo:`, `"foo:"`}, + {`alpine:3.8`, `alpine:3.8`}, // verify docker image names are ok + + // comments should be escaped. A comment is a pound + // sybol preceded by a space. + {`foo#bar`, `foo#bar`}, + {`foo #bar`, `"foo #bar"`}, + + // strings with newlines and control characters + // should be escaped + {"foo\nbar", "\"foo\\nbar\""}, + } + + for _, test := range tests { + buf := new(baseWriter) + writeEncode(buf, test.before) + a := test.after + b := buf.String() + if b != a { + t.Errorf("Want %q, got %q", a, b) + } + } +} + func TestChunk(t *testing.T) { s := strings.Join(testChunk, "") got, want := chunk(s, 64), testChunk diff --git a/yaml/pretty/writer.go b/yaml/pretty/writer.go index 29b5b97..c0ee50a 100644 --- a/yaml/pretty/writer.go +++ b/yaml/pretty/writer.go @@ -19,7 +19,6 @@ import ( "fmt" "sort" "strconv" - "strings" "github.com/drone/drone-yaml/yaml" ) @@ -196,28 +195,11 @@ func writeEncode(w writer, v string) { w.WriteByte('"') return } - trimmed := strings.TrimSpace(v) - if strings.HasPrefix(trimmed, "| ") { + if isQuoted(v) { fmt.Fprintf(w, "%q", v) - return + } else { + w.WriteString(v) } - if strings.HasPrefix(trimmed, "> ") { - fmt.Fprintf(w, "%q", v) - return - } - var prev rune - for _, b := range v { - if isQuoted(b) { - fmt.Fprintf(w, "%q", v) - return - } - if b == ' ' && prev == ':' { - fmt.Fprintf(w, "%q", v) - return - } - prev = b - } - w.WriteString(v) } func writeValue(w writer, v interface{}) { diff --git a/yaml/pretty/writer_test.go b/yaml/pretty/writer_test.go index d68e948..08b7513 100644 --- a/yaml/pretty/writer_test.go +++ b/yaml/pretty/writer_test.go @@ -36,7 +36,9 @@ func TestWriteComplexValue(t *testing.T) { got, want := b.String(), strings.TrimSpace(testComplexValue) if got != want { t.Errorf("Unexpected block format") - print(got) + println(got) + println("---") + println(want) } }