From ed0c798a817eefe86b4522de305d9cc72f5de401 Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Fri, 21 Apr 2023 19:51:59 +0200 Subject: [PATCH] feat: Remove deprecated --autotemplate flag --- .../chezmoi.io/docs/reference/commands/add.md | 16 -- pkg/chezmoi/autotemplate.go | 119 ----------- pkg/chezmoi/autotemplate_test.go | 190 ------------------ pkg/chezmoi/sourcestate.go | 10 - pkg/chezmoi/sourcestate_test.go | 17 -- pkg/cmd/addcmd.go | 7 - .../testdata/scripts/addautotemplate.txtar | 35 ---- 7 files changed, 394 deletions(-) delete mode 100644 pkg/chezmoi/autotemplate.go delete mode 100644 pkg/chezmoi/autotemplate_test.go delete mode 100644 pkg/cmd/testdata/scripts/addautotemplate.txtar diff --git a/assets/chezmoi.io/docs/reference/commands/add.md b/assets/chezmoi.io/docs/reference/commands/add.md index c733cd93610..5d55d4005c0 100644 --- a/assets/chezmoi.io/docs/reference/commands/add.md +++ b/assets/chezmoi.io/docs/reference/commands/add.md @@ -4,22 +4,6 @@ Add *target*s to the source state. If any target is already in the source state, then its source state is replaced with its current state in the destination directory. -## `--autotemplate` (deprecated) - -Automatically generate a template by replacing strings that match variable -values from the `data` section of the config file with their respective config -names as a template string. Longer substitutions occur before shorter ones. -This implies the `--template` option. - -!!! warning - - `--autotemplate` uses a greedy algorithm which occasionally generates - templates with unwanted variable substitutions. Carefully review any - templates it generates. - - `--autotemplate` has been deprecated and will be removed in a future - release of chezmoi. - ## `--encrypt` Encrypt files using the defined encryption method. diff --git a/pkg/chezmoi/autotemplate.go b/pkg/chezmoi/autotemplate.go deleted file mode 100644 index 12a0901f34b..00000000000 --- a/pkg/chezmoi/autotemplate.go +++ /dev/null @@ -1,119 +0,0 @@ -package chezmoi - -import ( - "regexp" - "sort" - "strings" -) - -// A templateVariable is a template variable. It is used instead of a -// map[string]string so that we can control order. -type templateVariable struct { - name string - value string -} - -var templateMarkerRx = regexp.MustCompile(`\{{2,}|\}{2,}`) - -// autoTemplate converts contents into a template by escaping template markers -// and replacing values in data with their keys. It returns the template and if -// any replacements were made. -func autoTemplate(contents []byte, data map[string]any) ([]byte, bool) { - contentsStr := string(contents) - replacements := false - - // Replace template markers. - replacedTemplateMarkersStr := templateMarkerRx.ReplaceAllString(contentsStr, `{{ "$0" }}`) - if replacedTemplateMarkersStr != contentsStr { - contentsStr = replacedTemplateMarkersStr - replacements = true - } - - // Replace variables. - // - // This naive approach will generate incorrect templates if the variable - // names match variable values. The algorithm here is probably O(N^2), we - // can do better. - variables := extractVariables(data) - sort.Slice(variables, func(i, j int) bool { - valueI := variables[i].value - valueJ := variables[j].value - switch { - case len(valueI) > len(valueJ): // First sort by value length. - return true - case len(valueI) == len(valueJ): // Second sort by value name. - nameI := variables[i].name - nameJ := variables[j].name - return nameI < nameJ - default: - return false - } - }) - for _, variable := range variables { - if variable.value == "" { - continue - } - - 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) - replacements = true - } 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 - } - // Advance to the next occurrence. - index += j - } - } - - return []byte(contentsStr), replacements -} - -// extractVariables extracts all template variables from data. -func extractVariables(data map[string]any) []templateVariable { - return extractVariablesHelper(nil /* variables */, nil /* parent */, data) -} - -// extractVariablesHelper appends all template variables in data to variables -// and returns variables. data is assumed to be rooted at parent. -func extractVariablesHelper( - variables []templateVariable, parent []string, data map[string]any, -) []templateVariable { - for name, value := range data { - switch value := value.(type) { - case string: - variable := templateVariable{ - name: strings.Join(append(parent, name), "."), - value: value, - } - variables = append(variables, variable) - case map[string]any: - variables = extractVariablesHelper(variables, append(parent, name), value) - } - } - return variables -} - -// 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]) -} - -// 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' -} diff --git a/pkg/chezmoi/autotemplate_test.go b/pkg/chezmoi/autotemplate_test.go deleted file mode 100644 index 2a9ff880940..00000000000 --- a/pkg/chezmoi/autotemplate_test.go +++ /dev/null @@ -1,190 +0,0 @@ -package chezmoi - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestAutoTemplate(t *testing.T) { - for _, tc := range []struct { - name string - contentsStr string - data map[string]any - expected string - expectedReplacements bool - }{ - { - name: "simple", - contentsStr: "email = you@example.com\n", - data: map[string]any{ - "email": "you@example.com", - }, - expected: "email = {{ .email }}\n", - expectedReplacements: true, - }, - { - name: "longest_first", - contentsStr: "name = John Smith\nfirstName = John\n", - data: map[string]any{ - "name": "John Smith", - "firstName": "John", - }, - expected: "" + - "name = {{ .name }}\n" + - "firstName = {{ .firstName }}\n", - expectedReplacements: true, - }, - { - name: "alphabetical_first", - contentsStr: "name = John Smith\n", - data: map[string]any{ - "alpha": "John Smith", - "beta": "John Smith", - "gamma": "John Smith", - }, - expected: "name = {{ .alpha }}\n", - expectedReplacements: true, - }, - { - name: "nested_values", - contentsStr: "email = you@example.com\n", - data: map[string]any{ - "personal": map[string]any{ - "email": "you@example.com", - }, - }, - expected: "email = {{ .personal.email }}\n", - expectedReplacements: true, - }, - { - name: "only_replace_words", - contentsStr: "darwinian evolution", - data: map[string]any{ - "os": "darwin", - }, - expected: "darwinian evolution", // not "{{ .os }}ian evolution" - }, - { - name: "longest_match_first", - contentsStr: "/home/user", - data: map[string]any{ - "homeDir": "/home/user", - }, - expected: "{{ .homeDir }}", - expectedReplacements: true, - }, - { - name: "longest_match_first_prefix", - contentsStr: "HOME=/home/user", - data: map[string]any{ - "homeDir": "/home/user", - }, - expected: "HOME={{ .homeDir }}", - expectedReplacements: true, - }, - { - name: "longest_match_first_suffix", - contentsStr: "/home/user/something", - data: map[string]any{ - "homeDir": "/home/user", - }, - expected: "{{ .homeDir }}/something", - expectedReplacements: true, - }, - { - name: "longest_match_first_prefix_and_suffix", - contentsStr: "HOME=/home/user/something", - data: map[string]any{ - "homeDir": "/home/user", - }, - expected: "HOME={{ .homeDir }}/something", - expectedReplacements: true, - }, - { - name: "words_only", - contentsStr: "aaa aa a aa aaa aa a aa aaa", - data: map[string]any{ - "alpha": "a", - }, - expected: "aaa aa {{ .alpha }} aa aaa aa {{ .alpha }} aa aaa", - expectedReplacements: true, - }, - { - name: "words_only_2", - contentsStr: "aaa aa a aa aaa aa a aa aaa", - data: map[string]any{ - "alpha": "aa", - }, - expected: "aaa {{ .alpha }} a {{ .alpha }} aaa {{ .alpha }} a {{ .alpha }} aaa", - expectedReplacements: true, - }, - { - name: "words_only_3", - contentsStr: "aaa aa a aa aaa aa a aa aaa", - data: map[string]any{ - "alpha": "aaa", - }, - expected: "{{ .alpha }} aa a aa {{ .alpha }} aa a aa {{ .alpha }}", - expectedReplacements: true, - }, - { - name: "skip_empty", - contentsStr: "a", - data: map[string]any{ - "empty": "", - }, - expected: "a", - }, - { - name: "markers", - contentsStr: "{{}}", - expected: `{{ "{{" }}{{ "}}" }}`, - expectedReplacements: true, - }, - } { - t.Run(tc.name, func(t *testing.T) { - actualTemplate, actualReplacements := autoTemplate([]byte(tc.contentsStr), tc.data) - assert.Equal(t, tc.expected, string(actualTemplate)) - assert.Equal(t, tc.expectedReplacements, actualReplacements) - }) - } -} - -func TestInWord(t *testing.T) { - for _, tc := range []struct { - s string - i int - expected bool - }{ - {s: "", i: 0, expected: false}, - {s: "a", i: 0, expected: false}, - {s: "a", i: 1, expected: false}, - {s: "ab", i: 0, expected: false}, - {s: "ab", i: 1, expected: true}, - {s: "ab", i: 2, expected: false}, - {s: "abc", i: 0, expected: false}, - {s: "abc", i: 1, expected: true}, - {s: "abc", i: 2, expected: true}, - {s: "abc", i: 3, expected: false}, - {s: " abc ", i: 0, expected: false}, - {s: " abc ", i: 1, expected: false}, - {s: " abc ", i: 2, expected: true}, - {s: " abc ", i: 3, expected: true}, - {s: " abc ", i: 4, expected: false}, - {s: " abc ", i: 5, expected: false}, - {s: "/home/user", i: 0, expected: false}, - {s: "/home/user", i: 1, expected: false}, - {s: "/home/user", i: 2, expected: true}, - {s: "/home/user", i: 3, expected: true}, - {s: "/home/user", i: 4, expected: true}, - {s: "/home/user", i: 5, expected: false}, - {s: "/home/user", i: 6, expected: false}, - {s: "/home/user", i: 7, expected: true}, - {s: "/home/user", i: 8, expected: true}, - {s: "/home/user", i: 9, expected: true}, - {s: "/home/user", i: 10, expected: false}, - } { - assert.Equal(t, tc.expected, inWord(tc.s, tc.i)) - } -} diff --git a/pkg/chezmoi/sourcestate.go b/pkg/chezmoi/sourcestate.go index f68cf49383f..4e8f1969c46 100644 --- a/pkg/chezmoi/sourcestate.go +++ b/pkg/chezmoi/sourcestate.go @@ -292,7 +292,6 @@ type ReplaceFunc func(targetRelPath RelPath, newSourceStateEntry, oldSourceState // AddOptions are options to SourceState.Add. type AddOptions struct { - AutoTemplate bool // Automatically create templates, if possible. Create bool // Add create_ entries instead of normal entries. Encrypt bool // Encrypt files. EncryptedSuffix string // Suffix for encrypted files. @@ -1876,13 +1875,6 @@ func (s *SourceState) newSourceStateFileEntryFromFile( if err != nil { return nil, err } - if options.AutoTemplate { - var replacements bool - contents, replacements = autoTemplate(contents, s.TemplateData()) - if replacements { - fileAttr.Template = true - } - } if len(contents) == 0 { fileAttr.Empty = true } @@ -1920,8 +1912,6 @@ func (s *SourceState) newSourceStateFileEntryFromSymlink( contents := []byte(linkname) template := false switch { - case options.AutoTemplate: - contents, template = autoTemplate(contents, s.TemplateData()) case options.Template: template = true case !options.Template && options.TemplateSymlinks: diff --git a/pkg/chezmoi/sourcestate_test.go b/pkg/chezmoi/sourcestate_test.go index a6597bfd707..04a6d28e937 100644 --- a/pkg/chezmoi/sourcestate_test.go +++ b/pkg/chezmoi/sourcestate_test.go @@ -428,23 +428,6 @@ func TestSourceStateAdd(t *testing.T) { ), }, }, - { - name: "template", - destAbsPaths: []AbsPath{ - NewAbsPath("/home/user/.template"), - }, - addOptions: AddOptions{ - AutoTemplate: true, - Filter: NewEntryTypeFilter(EntryTypesAll, EntryTypesNone), - }, - tests: []any{ - vfst.TestPath("/home/user/.local/share/chezmoi/dot_template.tmpl", - vfst.TestModeIsRegular, - vfst.TestModePerm(0o666&^chezmoitest.Umask), - vfst.TestContentsString("key = {{ .variable }}\n"), - ), - }, - }, { name: "dir_and_dir_file", destAbsPaths: []AbsPath{ diff --git a/pkg/cmd/addcmd.go b/pkg/cmd/addcmd.go index 61d0a296434..9d64eecb249 100644 --- a/pkg/cmd/addcmd.go +++ b/pkg/cmd/addcmd.go @@ -10,7 +10,6 @@ import ( type addCmdConfig struct { TemplateSymlinks bool `json:"templateSymlinks" mapstructure:"templateSymlinks" yaml:"templateSymlinks"` - autoTemplate bool create bool encrypt bool exact bool @@ -40,7 +39,6 @@ func (c *Config) newAddCmd() *cobra.Command { } flags := addCmd.Flags() - flags.BoolVarP(&c.Add.autoTemplate, "autotemplate", "a", c.Add.autoTemplate, "Generate the template when adding files as templates") //nolint:lll flags.BoolVar(&c.Add.create, "create", c.Add.create, "Add files that should exist, irrespective of their contents") flags.BoolVar(&c.Add.encrypt, "encrypt", c.Add.encrypt, "Encrypt files") flags.BoolVar(&c.Add.exact, "exact", c.Add.exact, "Add directories exactly") @@ -53,10 +51,6 @@ func (c *Config) newAddCmd() *cobra.Command { flags.BoolVarP(&c.Add.template, "template", "T", c.Add.template, "Add files as templates") flags.BoolVar(&c.Add.TemplateSymlinks, "template-symlinks", c.Add.TemplateSymlinks, "Add symlinks with target in source or home dirs as templates") //nolint:lll - if err := flags.MarkDeprecated("autotemplate", "it will be removed in a future release"); err != nil { - panic(err) - } - registerExcludeIncludeFlagCompletionFuncs(addCmd) return addCmd @@ -158,7 +152,6 @@ func (c *Config) runAddCmd(cmd *cobra.Command, args []string, sourceState *chezm } return sourceState.Add(c.sourceSystem, c.persistentState, c.destSystem, destAbsPathInfos, &chezmoi.AddOptions{ - AutoTemplate: c.Add.autoTemplate, Create: c.Add.create, Encrypt: c.Add.encrypt, EncryptedSuffix: c.encryption.EncryptedSuffix(), diff --git a/pkg/cmd/testdata/scripts/addautotemplate.txtar b/pkg/cmd/testdata/scripts/addautotemplate.txtar deleted file mode 100644 index 5f5129f5e89..00000000000 --- a/pkg/cmd/testdata/scripts/addautotemplate.txtar +++ /dev/null @@ -1,35 +0,0 @@ -# test that chezmoi add --autotemplate on a file with a replacement creates a template in the source directory -exec chezmoi add --autotemplate $HOME${/}.template -stderr 'deprecated' -cmp $CHEZMOISOURCEDIR/dot_template.tmpl golden/dot_template.tmpl - -# test that chezmoi add --autotemplate on a symlink with a replacement creates a template in the source directory -symlink $HOME/.symlink -> .target-value -exec chezmoi add --autotemplate $HOME${/}.symlink -cmp $CHEZMOISOURCEDIR/symlink_dot_symlink.tmpl golden/symlink_dot_symlink.tmpl - -# test that chezmoi add --autotemplate does not create a template if no replacements occurred -exec chezmoi add --autotemplate $HOME${/}.file -cmp $CHEZMOISOURCEDIR/dot_file golden/dot_file - -# test that chezmoi add --autotemplate escapes brackets -exec chezmoi add --autotemplate $HOME${/}.vimrc -cmp $CHEZMOISOURCEDIR/dot_vimrc.tmpl golden/dot_vimrc.tmpl - --- golden/dot_file -- -# contents of .file --- golden/dot_template.tmpl -- -key = {{ .variable }} --- golden/dot_vimrc.tmpl -- -set foldmarker={{ "{{" }},{{ "}}" }} --- golden/symlink_dot_symlink.tmpl -- -.target-{{ .variable }} --- home/user/.config/chezmoi/chezmoi.toml -- -[data] - variable = "value" --- home/user/.file -- -# contents of .file --- home/user/.template -- -key = value --- home/user/.vimrc -- -set foldmarker={{,}}