Merge pull request #96 from twpayne/autotemplate-fixes

Autotemplate fixes

lib/chezmoi/autotemplate.go

@@ -1,15 +1,13 @@
 package chezmoi
 
 import (
-	"regexp"
 	"sort"
 	"strings"
 )
 
 type templateVariable struct {
-	name        string
-	value       string
-	valueRegexp *regexp.Regexp
+	name  string
+	value string
 }
 
 type byValueLength []templateVariable
@@ -33,9 +31,8 @@ func extractVariables(variables []templateVariable, parent []string, data map[st
 		switch value := value.(type) {
 		case string:
 			variables = append(variables, templateVariable{
-				name:        strings.Join(append(parent, name), "."),
-				value:       value,
-				valueRegexp: regexp.MustCompile(`\b` + regexp.QuoteMeta(value) + `\b`),
+				name:  strings.Join(append(parent, name), "."),
+				value: value,
 			})
 		case map[string]interface{}:
 			variables = extractVariables(variables, append(parent, name), value)
@@ -44,14 +41,47 @@ func extractVariables(variables []templateVariable, parent []string, data map[st
 	return variables
 }
 
-func autoTemplate(contents []byte, data map[string]interface{}) []byte {
+func autoTemplate(contents []byte, data map[string]interface{}) ([]byte, error) {
 	// FIXME this naive approach will generate incorrect templates if the
 	// variable names match variable values
+	// FIXME the algorithm here is probably O(N^2), we can do better
 	variables := extractVariables(nil, nil, data)
 	sort.Sort(sort.Reverse(byValueLength(variables)))
 	contentsStr := string(contents)
 	for _, variable := range variables {
-		contentsStr = variable.valueRegexp.ReplaceAllString(contentsStr, "{{ ."+variable.name+" }}")
+		index := strings.Index(contentsStr, variable.value)
+		for index != -1 && index != len(contentsStr) {
+			if !inWord(contentsStr, index) && !inWord(contentsStr, index+len(variable.value)) {
+				// Replace variable.value which is on word boundaries at both
+				// ends.
+				replacement := "{{ ." + variable.name + " }}"
+				contentsStr = contentsStr[:index] + replacement + contentsStr[index+len(variable.value):]
+				index += len(replacement)
+			} else {
+				// Otherwise, keep looking. Consume at least one byte so we
+				// make progress.
+				index++
+			}
+			// Look for the next occurrence of variable.value.
+			j := strings.Index(contentsStr[index:], variable.value)
+			if j == -1 {
+				// No more occurrences found, so terminate the loop.
+				break
+			} else {
+				// Advance to the next occurrence.
+				index += j
+			}
+		}
 	}
-	return []byte(contentsStr)
+	return []byte(contentsStr), nil
+}
+
+// isWord returns true if b is a word byte.
+func isWord(b byte) bool {
+	return '0' <= b && b <= '9' || 'A' <= b && b <= 'Z' || 'a' <= b && b <= 'z'
+}
+
+// inWord returns true if splitting s at position i would split a word.
+func inWord(s string, i int) bool {
+	return i > 0 && i < len(s) && isWord(s[i-1]) && isWord(s[i])
 }

lib/chezmoi/autotemplate_test.go

@@ -4,18 +4,21 @@ import "testing"
 
 func TestAutoTemplate(t *testing.T) {
 	for _, tc := range []struct {
+		name        string
 		contentsStr string
 		data        map[string]interface{}
 		wantStr     string
 	}{
 		{
+			name:        "simple",
 			contentsStr: "email = hello@example.com\n",
 			data: map[string]interface{}{
 				"email": "hello@example.com",
 			},
 			wantStr: "email = {{ .email }}\n",
 		},
 		{
+			name:        "longest_first",
 			contentsStr: "name = John Smith\nfirstName = John\n",
 			data: map[string]interface{}{
 				"name":      "John Smith",
@@ -24,6 +27,17 @@ func TestAutoTemplate(t *testing.T) {
 			wantStr: "name = {{ .name }}\nfirstName = {{ .firstName }}\n",
 		},
 		{
+			name:        "alphabetical_first",
+			contentsStr: "name = John Smith\n",
+			data: map[string]interface{}{
+				"alpha": "John Smith",
+				"beta":  "John Smith",
+				"gamma": "John Smith",
+			},
+			wantStr: "name = {{ .alpha }}\n",
+		},
+		{
+			name:        "nested_values",
 			contentsStr: "email = hello@example.com\n",
 			data: map[string]interface{}{
 				"personal": map[string]interface{}{
@@ -33,28 +47,116 @@ func TestAutoTemplate(t *testing.T) {
 			wantStr: "email = {{ .personal.email }}\n",
 		},
 		{
+			name:        "only_replace_words",
 			contentsStr: "darwinian evolution",
 			data: map[string]interface{}{
 				"os": "darwin",
 			},
 			wantStr: "darwinian evolution", // not "{{ .os }}ian evolution"
 		},
-		/*
-			// FIXME this test currently fails because we match on word
-			// boundaries and ^/ is not a word boundary.
-			{
-				contentsStr: "/home/user",
-				data: map[string]interface{}{
-					"homedir": "/home/user",
-				},
-				wantStr: "{{ .homedir }}",
+		{
+			name:        "longest_match_first",
+			contentsStr: "/home/user",
+			data: map[string]interface{}{
+				"homedir": "/home/user",
+			},
+			wantStr: "{{ .homedir }}",
+		},
+		{
+			name:        "longest_match_first_prefix",
+			contentsStr: "HOME=/home/user",
+			data: map[string]interface{}{
+				"homedir": "/home/user",
+			},
+			wantStr: "HOME={{ .homedir }}",
+		},
+		{
+			name:        "longest_match_first_suffix",
+			contentsStr: "/home/user/something",
+			data: map[string]interface{}{
+				"homedir": "/home/user",
+			},
+			wantStr: "{{ .homedir }}/something",
+		},
+		{
+			name:        "longest_match_first_prefix_and_suffix",
+			contentsStr: "HOME=/home/user/something",
+			data: map[string]interface{}{
+				"homedir": "/home/user",
 			},
-		*/
+			wantStr: "HOME={{ .homedir }}/something",
+		},
+		{
+			name:        "words_only",
+			contentsStr: "aaa aa a aa aaa aa a aa aaa",
+			data: map[string]interface{}{
+				"alpha": "a",
+			},
+			wantStr: "aaa aa {{ .alpha }} aa aaa aa {{ .alpha }} aa aaa",
+		},
+		{
+			name:        "words_only_2",
+			contentsStr: "aaa aa a aa aaa aa a aa aaa",
+			data: map[string]interface{}{
+				"alpha": "aa",
+			},
+			wantStr: "aaa {{ .alpha }} a {{ .alpha }} aaa {{ .alpha }} a {{ .alpha }} aaa",
+		},
+		{
+			name:        "words_only_3",
+			contentsStr: "aaa aa a aa aaa aa a aa aaa",
+			data: map[string]interface{}{
+				"alpha": "aaa",
+			},
+			wantStr: "{{ .alpha }} aa a aa {{ .alpha }} aa a aa {{ .alpha }}",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			got, gotErr := autoTemplate([]byte(tc.contentsStr), tc.data)
+			gotStr := string(got)
+			if gotErr != nil || gotStr != tc.wantStr {
+				t.Errorf("autoTemplate([]byte(%q), %v) == %q, %v, want %q, <nil>", tc.contentsStr, tc.data, gotStr, gotErr, tc.wantStr)
+			}
+		})
+	}
+}
+
+func TestInWord(t *testing.T) {
+	for _, tc := range []struct {
+		s    string
+		i    int
+		want bool
+	}{
+		{s: "", i: 0, want: false},
+		{s: "a", i: 0, want: false},
+		{s: "a", i: 1, want: false},
+		{s: "ab", i: 0, want: false},
+		{s: "ab", i: 1, want: true},
+		{s: "ab", i: 2, want: false},
+		{s: "abc", i: 0, want: false},
+		{s: "abc", i: 1, want: true},
+		{s: "abc", i: 2, want: true},
+		{s: "abc", i: 3, want: false},
+		{s: " abc ", i: 0, want: false},
+		{s: " abc ", i: 1, want: false},
+		{s: " abc ", i: 2, want: true},
+		{s: " abc ", i: 3, want: true},
+		{s: " abc ", i: 4, want: false},
+		{s: " abc ", i: 5, want: false},
+		{s: "/home/user", i: 0, want: false},
+		{s: "/home/user", i: 1, want: false},
+		{s: "/home/user", i: 2, want: true},
+		{s: "/home/user", i: 3, want: true},
+		{s: "/home/user", i: 4, want: true},
+		{s: "/home/user", i: 5, want: false},
+		{s: "/home/user", i: 6, want: false},
+		{s: "/home/user", i: 7, want: true},
+		{s: "/home/user", i: 8, want: true},
+		{s: "/home/user", i: 9, want: true},
+		{s: "/home/user", i: 10, want: false},
 	} {
-		got := autoTemplate([]byte(tc.contentsStr), tc.data)
-		gotStr := string(got)
-		if gotStr != tc.wantStr {
-			t.Errorf("autoTemplate([]byte(%q), %v) == %q, want %q", tc.contentsStr, tc.data, gotStr, tc.wantStr)
+		if got := inWord(tc.s, tc.i); got != tc.want {
+			t.Errorf("inWord(%q, %d) == %v, want %v", tc.s, tc.i, got, tc.want)
 		}
 	}
 }

lib/chezmoi/target_state.go

@@ -112,7 +112,10 @@ func (ts *TargetState) Add(fs vfs.FS, addOptions AddOptions, targetPath string,
 			return err
 		}
 		if addOptions.Template {
-			contents = autoTemplate(contents, ts.Data)
+			contents, err = autoTemplate(contents, ts.Data)
+			if err != nil {
+				return err
+			}
 		}
 		return ts.addFile(targetName, entries, parentDirSourceName, info, addOptions.Template, contents, mutator)
 	case info.Mode()&os.ModeType == os.ModeSymlink: