attempt to improve string escaping

yaml/pretty/testdata/cron.yml

@@ -5,7 +5,7 @@ kind: cron
 spec:
   branch: master
 
-  schedule: "1 * * * *"
+  schedule: 1 * * * *
 
   deployment:
 

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

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

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}

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

yaml/pretty/writer.go

@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"sort"
 	"strconv"
-	"strings"
 
 	"github.com/drone/drone-yaml/yaml"
 )
@@ -196,29 +195,12 @@ 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
-	}
-	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
-	}
+	} else {
 		w.WriteString(v)
 	}
+}
 
 func writeValue(w writer, v interface{}) {
 	if v == nil {

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)
 	}
 }