Repository: twpayne/chezmoi
Pull request #132 (2019-01-16 00:32:08)

Fix bugs relating to ignoring files and add --prompt flag to add command


Patch:
diff --git a/cmd/add.go b/cmd/add.go
index ec70d73d7..ba5f39d91 100644
--- a/cmd/add.go
+++ b/cmd/add.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/spf13/cobra"
 	"github.com/twpayne/chezmoi/lib/chezmoi"
@@ -19,6 +20,7 @@ var addCommand = &cobra.Command{
 
 type addCommandConfig struct {
 	recursive bool
+	prompt    bool
 	options   chezmoi.AddOptions
 }
 
@@ -28,11 +30,12 @@ func init() {
 	persistentFlags := addCommand.PersistentFlags()
 	persistentFlags.BoolVarP(&config.add.options.Empty, "empty", "e", false, "add empty files")
 	persistentFlags.BoolVarP(&config.add.options.Exact, "exact", "x", false, "add directories exactly")
+	persistentFlags.BoolVarP(&config.add.prompt, "prompt", "p", false, "prompt before adding")
 	persistentFlags.BoolVarP(&config.add.recursive, "recursive", "r", false, "recurse in to subdirectories")
 	persistentFlags.BoolVarP(&config.add.options.Template, "template", "T", false, "add files as templates")
 }
 
-func (c *Config) runAddCommand(fs vfs.FS, args []string) error {
+func (c *Config) runAddCommand(fs vfs.FS, args []string) (err error) {
 	ts, err := c.getTargetState(fs)
 	if err != nil {
 		return err
@@ -51,10 +54,21 @@ func (c *Config) runAddCommand(fs vfs.FS, args []string) error {
 			return err
 		}
 	case err == nil:
-		return fmt.Errorf("%s: is not a directory", c.SourceDir)
+		return fmt.Errorf("%s: not a directory", c.SourceDir)
 	default:
 		return err
 	}
+	destDirPrefix := ts.DestDir + "/"
+	var quit int // quit is an int with a unique address
+	defer func() {
+		if r := recover(); r != nil {
+			if p, ok := r.(*int); ok && p == &quit {
+				err = nil
+			} else {
+				panic(r)
+			}
+		}
+	}()
 	for _, arg := range args {
 		path, err := filepath.Abs(arg)
 		if err != nil {
@@ -65,11 +79,47 @@ func (c *Config) runAddCommand(fs vfs.FS, args []string) error {
 				if err != nil {
 					return err
 				}
+				if ts.TargetIgnore.Match(strings.TrimPrefix(path, destDirPrefix)) {
+					return nil
+				}
+				if c.add.prompt {
+					choice, err := prompt(fmt.Sprintf("Add %s", path), "ynqa")
+					if err != nil {
+						return err
+					}
+					switch choice {
+					case 'y':
+					case 'n':
+						return nil
+					case 'q':
+						panic(&quit) // abort vfs.Walk by panicking
+					case 'a':
+						c.add.prompt = false
+					}
+				}
 				return ts.Add(fs, c.add.options, path, info, mutator)
 			}); err != nil {
 				return err
 			}
 		} else {
+			if ts.TargetIgnore.Match(strings.TrimPrefix(path, destDirPrefix)) {
+				continue
+			}
+			if c.add.prompt {
+				choice, err := prompt(fmt.Sprintf("Add %s", path), "ynqa")
+				if err != nil {
+					return err
+				}
+				switch choice {
+				case 'y':
+				case 'n':
+					continue
+				case 'q':
+					return nil
+				case 'a':
+					c.add.prompt = false
+				}
+			}
 			if err := ts.Add(fs, c.add.options, path, nil, mutator); err != nil {
 				return err
 			}
diff --git a/cmd/chattr.go b/cmd/chattr.go
index 5fa80a4e4..f5619a40a 100644
--- a/cmd/chattr.go
+++ b/cmd/chattr.go
@@ -135,7 +135,7 @@ func parseAttributeModifiers(s string) (*attributeModifiers, error) {
 		case "template", "t":
 			ams.template = modifier
 		default:
-			return nil, fmt.Errorf("unknown attribute: %s", attribute)
+			return nil, fmt.Errorf("%s: unknown attribute", attribute)
 		}
 	}
 	return ams, nil
diff --git a/cmd/data.go b/cmd/data.go
index 68208ee7a..a3ab00a74 100644
--- a/cmd/data.go
+++ b/cmd/data.go
@@ -29,7 +29,7 @@ func init() {
 func (c *Config) runDataCommand(fs vfs.FS, args []string) error {
 	format, ok := formatMap[strings.ToLower(c.data.format)]
 	if !ok {
-		return fmt.Errorf("unknown format: %s", c.data.format)
+		return fmt.Errorf("%s: unknown format", c.data.format)
 	}
 	ts, err := c.getTargetState(fs)
 	if err != nil {
diff --git a/cmd/dump.go b/cmd/dump.go
index 30383b9f4..c88b9bd15 100644
--- a/cmd/dump.go
+++ b/cmd/dump.go
@@ -31,7 +31,7 @@ func init() {
 func (c *Config) runDumpCommand(fs vfs.FS, args []string) error {
 	format, ok := formatMap[strings.ToLower(c.dump.format)]
 	if !ok {
-		return fmt.Errorf("unknown format: %s", c.dump.format)
+		return fmt.Errorf("%s: unknown format", c.dump.format)
 	}
 	ts, err := c.getTargetState(fs)
 	if err != nil {
diff --git a/go.mod b/go.mod
index 89b497fd5..c797b17bb 100644
--- a/go.mod
+++ b/go.mod
@@ -13,7 +13,7 @@ require (
 	github.com/spf13/viper v1.3.1
 	github.com/stretchr/objx v0.1.1 // indirect
 	github.com/twpayne/go-shell v0.0.1
-	github.com/twpayne/go-vfs v1.0.2
+	github.com/twpayne/go-vfs v1.0.3
 	github.com/twpayne/go-xdg v1.0.0
 	github.com/zalando/go-keyring v0.0.0-20180221093347-6d81c293b3fb
 	golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9
diff --git a/go.sum b/go.sum
index c24c1d9c9..d9881f0a3 100644
--- a/go.sum
+++ b/go.sum
@@ -49,8 +49,8 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf
 github.com/twpayne/go-shell v0.0.1 h1:Ako3cUeuULhWadYk37jM3FlJ8lkSSW4INBjYj9K60Gw=
 github.com/twpayne/go-shell v0.0.1/go.mod h1:QCjEvdZndTuPObd+11NYAI1UeNLSuGZVxJ+67Wl+IU4=
 github.com/twpayne/go-vfs v0.1.5/go.mod h1:OIXA6zWkcn7Jk46XT7ceYqBMeIkfzJ8WOBhGJM0W4y8=
-github.com/twpayne/go-vfs v1.0.2 h1:6auHbiujxMUlqIAEVmJPXYC8/+27P9hBaUKnZujg7nk=
-github.com/twpayne/go-vfs v1.0.2/go.mod h1:OIXA6zWkcn7Jk46XT7ceYqBMeIkfzJ8WOBhGJM0W4y8=
+github.com/twpayne/go-vfs v1.0.3 h1:kmJe688Hu21rp7jz+2QOl4l3YrnZrr7kPsBsjv04PU4=
+github.com/twpayne/go-vfs v1.0.3/go.mod h1:OIXA6zWkcn7Jk46XT7ceYqBMeIkfzJ8WOBhGJM0W4y8=
 github.com/twpayne/go-xdg v1.0.0 h1:k+QM2LL00/zx/gvxKsCMRBJ8nxCBWDBe2LU9y3Xo7x0=
 github.com/twpayne/go-xdg v1.0.0/go.mod h1:SHOoqWXTQT0rcQM4isbONZ6bH6uwIBgXuymEVgTc+Ao=
 github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0=
diff --git a/lib/chezmoi/dir.go b/lib/chezmoi/dir.go
index 414307596..f164f7046 100644
--- a/lib/chezmoi/dir.go
+++ b/lib/chezmoi/dir.go
@@ -124,6 +124,9 @@ func (d *Dir) Apply(fs vfs.FS, destDir string, ignore func(string) bool, umask o
 		for _, info := range infos {
 			name := info.Name()
 			if _, ok := d.Entries[name]; !ok {
+				if ignore(filepath.Join(d.targetName, name)) {
+					continue
+				}
 				if err := mutator.RemoveAll(filepath.Join(targetPath, name)); err != nil {
 					return err
 				}
diff --git a/lib/chezmoi/target_state.go b/lib/chezmoi/target_state.go
index 6cd3052c8..da505fb3f 100644
--- a/lib/chezmoi/target_state.go
+++ b/lib/chezmoi/target_state.go
@@ -250,7 +250,8 @@ func (ts *TargetState) Populate(fs vfs.FS) error {
 		// Treat all files and directories beginning with "." specially.
 		if _, name := filepath.Split(relPath); strings.HasPrefix(name, ".") {
 			if info.Name() == ".chezmoiignore" {
-				return ts.addSourceIgnore(fs, path, relPath)
+				dns := dirNames(parseDirNameComponents(splitPathList(relPath)))
+				return ts.addSourceIgnore(fs, path, filepath.Join(dns...))
 			}
 			// Ignore all other files and directories.
 			if info.IsDir() {
@@ -316,11 +317,11 @@ func (ts *TargetState) Populate(fs vfs.FS) error {
 					evaluateLinkname: evaluateLinkname,
 				}
 			default:
-				return fmt.Errorf("%v: unsupported mode: %d", path, psfp.Mode&os.ModeType)
+				return fmt.Errorf("%v: unsupported mode 0%o", path, psfp.Mode&os.ModeType)
 			}
 			entries[psfp.Name] = entry
 		default:
-			return fmt.Errorf("unsupported file type: %s", path)
+			return fmt.Errorf("%s: unsupported file type", path)
 		}
 		return nil
 	})
@@ -328,12 +329,11 @@ func (ts *TargetState) Populate(fs vfs.FS) error {
 
 func (ts *TargetState) addDir(targetName string, entries map[string]Entry, parentDirSourceName string, exact bool, perm os.FileMode, empty bool, mutator Mutator) error {
 	name := filepath.Base(targetName)
-	var existingDir *Dir
 	if entry, ok := entries[name]; ok {
-		existingDir, ok = entry.(*Dir)
-		if !ok {
+		if _, ok = entry.(*Dir); !ok {
 			return fmt.Errorf("%s: already added and not a directory", targetName)
 		}
+		return nil
 	}
 	sourceName := DirAttributes{
 		Name:  name,
@@ -344,12 +344,6 @@ func (ts *TargetState) addDir(targetName string, entries map[string]Entry, paren
 		sourceName = filepath.Join(parentDirSourceName, sourceName)
 	}
 	dir := newDir(sourceName, targetName, exact, perm)
-	if existingDir != nil {
-		if existingDir.sourceName == dir.sourceName {
-			return nil
-		}
-		return mutator.Rename(filepath.Join(ts.SourceDir, existingDir.sourceName), filepath.Join(ts.SourceDir, dir.sourceName))
-	}
 	if err := mutator.Mkdir(filepath.Join(ts.SourceDir, sourceName), 0777&^ts.Umask); err != nil {
 		return err
 	}


Test patch:
diff --git a/cmd/add_test.go b/cmd/add_test.go
index 897f89ec2..41120132d 100644
--- a/cmd/add_test.go
+++ b/cmd/add_test.go
@@ -248,6 +248,54 @@ func TestAddCommand(t *testing.T) {
 				),
 			},
 		},
+		{
+			name: "dont_add_ignored_file",
+			args: []string{"/home/user/foo"},
+			root: map[string]interface{}{
+				"/home/user": &vfst.Dir{Perm: 0755},
+				"/home/user/.chezmoi": &vfst.Dir{
+					Perm: 0700,
+					Entries: map[string]interface{}{
+						".chezmoiignore": "foo\n",
+					},
+				},
+				"/home/user/foo": "bar",
+			},
+			tests: []vfst.Test{
+				vfst.TestPath("/home/user/.chezmoi/foo",
+					vfst.TestDoesNotExist,
+				),
+			},
+		},
+		{
+			name: "dont_add_ignored_file_recursive",
+			args: []string{"/home/user/foo"},
+			add: addCommandConfig{
+				recursive: true,
+			},
+			root: map[string]interface{}{
+				"/home/user": &vfst.Dir{Perm: 0755},
+				"/home/user/.chezmoi": &vfst.Dir{
+					Perm: 0700,
+					Entries: map[string]interface{}{
+						"exact_foo/.chezmoiignore": "bar/qux\n",
+					},
+				},
+				"/home/user/foo/bar": map[string]interface{}{
+					"baz": "baz",
+					"qux": "quz",
+				},
+			},
+			tests: []vfst.Test{
+				vfst.TestPath("/home/user/.chezmoi/exact_foo/bar/baz",
+					vfst.TestModeIsRegular,
+					vfst.TestContentsString("baz"),
+				),
+				vfst.TestPath("/home/user/.chezmoi/exact_foo/bar/qux",
+					vfst.TestDoesNotExist,
+				),
+			},
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			c := &Config{
diff --git a/lib/chezmoi/target_state_test.go b/lib/chezmoi/target_state_test.go
index 24094965a..5edd3c7ed 100644
--- a/lib/chezmoi/target_state_test.go
+++ b/lib/chezmoi/target_state_test.go
@@ -28,20 +28,22 @@ func TestEndToEnd(t *testing.T) {
 					"dir": map[string]interface{}{
 						"foo": "foo",
 						"bar": "bar",
+						"qux": "qux",
 					},
 					"replace_symlink": &vfst.Symlink{Target: "foo"},
 				},
 				"/home/user/.chezmoi": map[string]interface{}{
-					".git/HEAD":               "HEAD",
-					".chezmoiignore":          "{{ .ignore }} # comment\n",
-					"README.md":               "contents of README.md\n",
-					"dot_bashrc":              "bar",
-					"dot_hgrc.tmpl":           "[ui]\nusername = {{ .name }} <{{ .email }}>\n",
-					"empty.tmpl":              "{{ if false }}foo{{ end }}",
-					"empty_foo":               "",
-					"exact_dir/foo":           "foo",
-					"symlink_bar":             "empty",
-					"symlink_replace_symlink": "bar",
+					".git/HEAD":                "HEAD",
+					".chezmoiignore":           "{{ .ignore }} # comment\n",
+					"README.md":                "contents of README.md\n",
+					"dot_bashrc":               "bar",
+					"dot_hgrc.tmpl":            "[ui]\nusername = {{ .name }} <{{ .email }}>\n",
+					"empty.tmpl":               "{{ if false }}foo{{ end }}",
+					"empty_foo":                "",
+					"exact_dir/foo":            "foo",
+					"exact_dir/.chezmoiignore": "qux\n",
+					"symlink_bar":              "empty",
+					"symlink_replace_symlink":  "bar",
 				},
 			},
 			sourceDir: "/home/user/.chezmoi",
@@ -76,6 +78,10 @@ func TestEndToEnd(t *testing.T) {
 				vfst.TestPath("/home/user/dir/bar",
 					vfst.TestDoesNotExist,
 				),
+				vfst.TestPath("/home/user/dir/qux",
+					vfst.TestModeIsRegular,
+					vfst.TestContentsString("qux"),
+				),
 				vfst.TestPath("/home/user/README.md",
 					vfst.TestDoesNotExist,
 				),