Put double quotes in error messages with resources (#1365)

* Put single quotes in error messages with resources

* Remove stray print.

* Change single quotes to double.

* Avoid escaping double quotes.

internal/cmdfix/cmdfix.go

@@ -68,7 +68,7 @@ func (r *Runner) preRunE(c *cobra.Command, args []string) error {
 			"kpt packages must be tracked by git before making fix to revert unwanted changes: %v", err)
 	}
 	if strings.TrimSpace(g.Stdout.String()) != "" {
-		return errors.Errorf("must commit package changes to git %s before attempting to fix it",
+		return errors.Errorf("must commit package changes to git %q before attempting to fix it",
 			args[0])
 	}
 	return nil

internal/cmdfix/cmdfix_test.go

@@ -54,11 +54,11 @@ spec:
 kind: Kustomization`,
 
 			expectedOut: `processing resource configs to identify possible fixes... 
-created setter with name cluster
-created setter with name profile
-created setter with name project
+created setter with name "cluster"
+created setter with name "profile"
+created setter with name "project"
 created 3 setters in total
-created substitution with name project-cluster-54235872
+created substitution with name "project-cluster-54235872"
 created 1 substitution in total
 `,
 
@@ -120,11 +120,11 @@ spec:
   cluster: "someproj/someclus" # {"type":"string","x-kustomize":{"partialSetters":[{"name":"project","value":"someproj"},{"name":"cluster","value":"someclus"}]}}
 `,
 			expectedOut: `processing resource configs to identify possible fixes...  (dry-run)
-created setter with name cluster (dry-run)
-created setter with name profile (dry-run)
-created setter with name project (dry-run)
+created setter with name "cluster" (dry-run)
+created setter with name "profile" (dry-run)
+created setter with name "project" (dry-run)
 created 3 setters in total (dry-run)
-created substitution with name project-cluster-54235872 (dry-run)
+created substitution with name "project-cluster-54235872" (dry-run)
 created 1 substitution in total (dry-run)
 `,
 			expectedOutput: `apiVersion: install.istio.io/v1alpha2

internal/cmdget/cmdget.go

@@ -80,7 +80,7 @@ func (r *Runner) runE(c *cobra.Command, args []string) error {
 		return getioreader.Get(args[1], r.FilenamePattern, c.InOrStdin())
 	}
 
-	fmt.Fprintf(c.OutOrStdout(), "fetching package %s from %s to %s\n",
+	fmt.Fprintf(c.OutOrStdout(), "fetching package %q from %q to %q\n",
 		r.Get.Directory, r.Get.Repo, r.Get.Destination)
 	if err := r.Get.Run(); err != nil {
 		return err

internal/cmdinit/cmdinit.go

@@ -86,11 +86,11 @@ func (r *Runner) preRunE(_ *cobra.Command, args []string) error {
 func (r *Runner) runE(c *cobra.Command, args []string) error {
 	var err error
 	if _, err = os.Stat(args[0]); os.IsNotExist(err) {
-		return errors.Errorf("%s does not exist", err)
+		return errors.Errorf("%q does not exist", err)
 	}
 
 	if _, err = os.Stat(filepath.Join(args[0], "Kptfile")); os.IsNotExist(err) {
-		fmt.Fprintf(c.OutOrStdout(), "writing %s\n", filepath.Join(args[0], "Kptfile"))
+		fmt.Fprintf(c.OutOrStdout(), "writing %q\n", filepath.Join(args[0], "Kptfile"))
 		k := kptfile.KptFile{
 			ResourceMeta: yaml.ResourceMeta{
 				ObjectMeta: yaml.ObjectMeta{
@@ -127,7 +127,7 @@ func (r *Runner) runE(c *cobra.Command, args []string) error {
 	}
 
 	if _, err = os.Stat(filepath.Join(args[0], man.ManFilename)); os.IsNotExist(err) {
-		fmt.Fprintf(c.OutOrStdout(), "writing %s\n", filepath.Join(args[0], man.ManFilename))
+		fmt.Fprintf(c.OutOrStdout(), "writing %q\n", filepath.Join(args[0], man.ManFilename))
 		buff := &bytes.Buffer{}
 		t, err := template.New("man").Parse(manTemplate)
 		if err != nil {

internal/cmdupdate/cmdupdate.go

@@ -94,10 +94,10 @@ func (r *Runner) preRunE(c *cobra.Command, args []string) error {
 
 func (r *Runner) runE(c *cobra.Command, args []string) error {
 	if len(r.Update.Ref) > 0 {
-		fmt.Fprintf(c.ErrOrStderr(), "updating package %s to %s\n",
+		fmt.Fprintf(c.ErrOrStderr(), "updating package %q to %s\n",
 			r.Update.Path, r.Update.Ref)
 	} else {
-		fmt.Fprintf(c.ErrOrStderr(), "updating package %s\n",
+		fmt.Fprintf(c.ErrOrStderr(), "updating package %q\n",
 			r.Update.Path)
 	}
 	if err := r.Update.Run(); err != nil {

internal/gitutil/gitutil.go

@@ -69,7 +69,7 @@ func branchExists(repo, branch string) (bool, error) {
 	if err != nil {
 		// stdErr contains the error message for os related errors, git permission errors
 		// and if repo doesn't exist
-		return false, errors.Errorf("failed to lookup master(or main) branch %v: %s", err, strings.TrimSpace(stdErr.String()))
+		return false, errors.Errorf("failed to lookup master(or main) branch %q: %s", err, strings.TrimSpace(stdErr.String()))
 	}
 	// stdOut contains the branch information if the branch is present in remote repo
 	// stdOut is empty if the repo doesn't have the input branch
@@ -219,7 +219,7 @@ func (g *GitRunner) cacheRepo(uri, dir string,
 			// verify we got the commit
 			if err = gitRunner.Run("show", s); err != nil {
 				return "", errors.Errorf(
-					"failed to clone git repo: trouble fetching origin %s: %v, "+
+					"failed to clone git repo: trouble fetching origin %q: %v, "+
 						"please run 'git clone <REPO>; stat <DIR/SUBDIR>' to verify credentials", s, err)
 			}
 		}

internal/testutil/testdata/updateMergeConflict/java/java-deployment.resource.yaml

@@ -40,7 +40,7 @@ spec:
         - containerPort: 8081
 =======
         - containerPort: 80
->>>>>>> update from master to master
+>>>>>>> update from "master" to "master"
         envFrom:
         - configMapRef:
             name: app-config

internal/testutil/testdata/updateMergeConflict/java/java-service.resource.yaml

@@ -30,4 +30,4 @@ spec:
   - name: "80"
     port: 80
     targetPort: 80
->>>>>>> update from master to master
+>>>>>>> update from "master" to "master"

internal/util/argutil/argutil.go

@@ -77,7 +77,7 @@ func ParseFieldPath(path string) ([]string, error) {
 		}
 		p := strings.Split(parts[i], "[")
 		if len(p) != 2 {
-			return nil, errors.Errorf("unrecognized path element: %s.  "+
+			return nil, errors.Errorf("unrecognized path element: %q.  "+
 				"Should be of the form 'list[field=value]'", parts[i])
 		}
 		p[1] = "[" + p[1]

internal/util/fix/fix.go

@@ -59,21 +59,21 @@ func (c Command) fixV1Setters() error {
 	}
 
 	for _, setter := range sfr.CreatedSetters {
-		printFunc("created setter with name %s", setter)
+		printFunc("created setter with name %q", setter)
 	}
 	printFunc("created %d setters in total", len(sfr.CreatedSetters))
 
 	for _, subst := range sfr.CreatedSubst {
-		printFunc("created substitution with name %s", subst)
+		printFunc("created substitution with name %q", subst)
 	}
 	printFunc("created %d substitution in total", len(sfr.CreatedSubst))
 
 	for setter, err := range sfr.FailedSetters {
-		printFunc("failed to create setter with name %s: %v", setter, err)
+		printFunc("failed to create setter with name %q: %v", setter, err)
 	}
 
 	for subst, err := range sfr.FailedSubst {
-		printFunc("failed to create substitution with name %s: %v", subst, err)
+		printFunc("failed to create substitution with name %q: %v", subst, err)
 	}
 
 	return err

internal/util/get/get.go

@@ -57,7 +57,7 @@ func (c Command) Run() error {
 	}
 
 	if _, err := os.Stat(c.Destination); !c.Clean && !os.IsNotExist(err) {
-		return errors.Errorf("destination directory %s already exists", c.Destination)
+		return errors.Errorf("destination directory %q already exists", c.Destination)
 	}
 
 	// normalize path to a filepath
@@ -92,7 +92,7 @@ func (c Command) Run() error {
 	// copy the git sub directory to the destination
 	err = copyutil.CopyDir(r.AbsPath(), c.Destination)
 	if err != nil {
-		return errors.WrapPrefixf(err, "missing subdirectory %s in repo %s at ref %s\n",
+		return errors.WrapPrefixf(err, "missing subdirectory %q in repo %q at ref %q\n",
 			r.Path, r.OrgRepo, r.Ref)
 	}
 
@@ -154,7 +154,7 @@ func clonerUsingGitExec(repoSpec *git.RepoSpec) error {
 	err = cmd.Run()
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "Error initializing empty git repo: %s", out.String())
-		return errors.WrapPrefixf(err, "trouble initializing empty git repo in %s",
+		return errors.WrapPrefixf(err, "trouble initializing empty git repo in %q",
 			repoSpec.Dir)
 	}
 
@@ -167,7 +167,7 @@ func clonerUsingGitExec(repoSpec *git.RepoSpec) error {
 		fmt.Fprintf(os.Stderr, "Error setting git remote: %s", out.String())
 		return errors.WrapPrefixf(
 			err,
-			"trouble adding remote %s",
+			"trouble adding remote %q",
 			repoSpec.CloneSpec())
 	}
 	if repoSpec.Ref == "" {
@@ -184,7 +184,7 @@ func clonerUsingGitExec(repoSpec *git.RepoSpec) error {
 		cmd.Dir = repoSpec.Dir
 		err = cmd.Run()
 		if err != nil {
-			return errors.WrapPrefixf(err, "trouble fetching %s, "+
+			return errors.WrapPrefixf(err, "trouble fetching %q, "+
 				"please run 'git clone <REPO>; stat <DIR/SUBDIR>' to verify credentials", repoSpec.Ref)
 		}
 		cmd = exec.Command(gitProgram, "reset", "--hard", "FETCH_HEAD")
@@ -194,7 +194,7 @@ func clonerUsingGitExec(repoSpec *git.RepoSpec) error {
 		err = cmd.Run()
 		if err != nil {
 			return errors.WrapPrefixf(
-				err, "trouble hard resetting empty repository to %s", repoSpec.Ref)
+				err, "trouble hard resetting empty repository to %q", repoSpec.Ref)
 		}
 		return nil
 	}()
@@ -213,7 +213,7 @@ func clonerUsingGitExec(repoSpec *git.RepoSpec) error {
 		cmd.Dir = repoSpec.Dir
 		if err = cmd.Run(); err != nil {
 			return errors.WrapPrefixf(
-				err, "trouble hard resetting empty repository to %s, "+
+				err, "trouble hard resetting empty repository to %q, "+
 					"please run 'git clone <REPO>; stat <DIR/SUBDIR>' to verify credentials", repoSpec.Ref)
 		}
 	}
@@ -223,7 +223,7 @@ func clonerUsingGitExec(repoSpec *git.RepoSpec) error {
 	cmd.Dir = repoSpec.Dir
 	err = cmd.Run()
 	if err != nil {
-		return errors.WrapPrefixf(err, "trouble fetching submodules for %s, "+
+		return errors.WrapPrefixf(err, "trouble fetching submodules for %q, "+
 			"please run 'git clone <REPO>; stat <DIR/SUBDIR>' to verify credentials", repoSpec.Ref)
 	}
 

internal/util/get/get_test.go

@@ -571,7 +571,7 @@ func TestCommand_Run_failExistingDir(t *testing.T) {
 	err = Command{
 		Git:         kptfile.Git{Repo: g.RepoDirectory, Ref: "master", Directory: "/"},
 		Destination: filepath.Base(g.RepoDirectory)}.Run()
-	assert.EqualError(t, err, fmt.Sprintf("destination directory %s already exists", g.RepoName))
+	assert.EqualError(t, err, fmt.Sprintf("destination directory %q already exists", g.RepoName))
 
 	// verify files are unchanged
 	g.AssertEqual(t, filepath.Join(g.DatasetDirectory, testutil.Dataset1), r)

internal/util/man/man.go

@@ -64,7 +64,7 @@ func (m Command) Run() error {
 	if k.PackageMeta.Man == "" {
 		_, err := os.Stat(filepath.Join(m.Path, ManFilename))
 		if err != nil {
-			return errors.Errorf("no manual entry for %s", m.Path)
+			return errors.Errorf("no manual entry for %q", m.Path)
 		}
 		k.PackageMeta.Man = filepath.Join(ManFilename)
 	}
@@ -83,7 +83,7 @@ func (m Command) Run() error {
 		return err
 	}
 	if !strings.HasPrefix(apMan, apPkg) {
-		return errors.Errorf("invalid manual location for %s", m.Path)
+		return errors.Errorf("invalid manual location for %q", m.Path)
 	}
 
 	// write the formatted manual to a tmp file so it can be displayed

internal/util/man/man_test.go

@@ -214,7 +214,7 @@ packageMetadata:
 		StdOut:         b,
 	}
 	err = instance.Run()
-	if !assert.EqualError(t, err, fmt.Sprintf("no manual entry for %s", d)) {
+	if !assert.EqualError(t, err, fmt.Sprintf("no manual entry for %q", d)) {
 		return
 	}
 	if !assert.Equal(t, ``, b.String()) {
@@ -275,7 +275,7 @@ packageMetadata:
 		StdOut:         b,
 	}
 	err = instance.Run()
-	assert.EqualError(t, err, fmt.Sprintf("invalid manual location for %s", d))
+	assert.EqualError(t, err, fmt.Sprintf("invalid manual location for %q", d))
 	assert.Equal(t, ``, b.String())
 }
 

internal/util/openapi/openapi.go

@@ -44,14 +44,14 @@ func ConfigureOpenAPI(factory util.Factory, k8sSchemaSource, k8sSchemaPath strin
 	case SchemaSourceFile:
 		openAPISchema, err := ReadOpenAPISchemaFromDisk(k8sSchemaPath)
 		if err != nil {
-			return fmt.Errorf("error reading file at path %s: %v",
+			return fmt.Errorf("error reading file at path %q: %v",
 				k8sSchemaPath, err)
 		}
 		return ConfigureOpenAPISchema(openAPISchema)
 	case SchemaSourceBuiltin:
 		return nil
 	default:
-		return fmt.Errorf("unknown schema source %s. Must be one of file, cluster, builtin",
+		return fmt.Errorf("unknown schema source %q. Must be one of file, cluster, builtin",
 			k8sSchemaSource)
 	}
 }

internal/util/parse/parse.go

@@ -153,7 +153,7 @@ func getDest(v, repo, subdir string) (string, error) {
 		parent := filepath.Dir(v)
 		if _, err := os.Stat(parent); os.IsNotExist(err) {
 			// error -- fetch to directory where parent does not exist
-			return "", errors.Errorf("parent directory %s does not exist", parent)
+			return "", errors.Errorf("parent directory %q does not exist", parent)
 		}
 		// fetch to a specific directory -- don't default the name
 		return v, nil
@@ -171,7 +171,7 @@ func getDest(v, repo, subdir string) (string, error) {
 
 	// make sure the destination directory does not yet exist yet
 	if _, err := os.Stat(v); !os.IsNotExist(err) {
-		return "", errors.Errorf("destination directory %s already exists", v)
+		return "", errors.Errorf("destination directory %q already exists", v)
 	}
 	return v, nil
 }

internal/util/sync/sync.go

@@ -49,12 +49,12 @@ type Command struct {
 func (c Command) Run() error {
 	b, err := ioutil.ReadFile(filepath.Join(c.Dir, kptfile.KptFileName))
 	if err != nil {
-		return errors.WrapPrefixf(err, "failed to read Kptfile under %s", c.Dir)
+		return errors.WrapPrefixf(err, "failed to read Kptfile under %q", c.Dir)
 	}
 	k := &kptfile.KptFile{}
 
 	if err := yaml.Unmarshal(b, k); err != nil {
-		return errors.WrapPrefixf(err, "failed to unmarshal Kptfile under %s", c.Dir)
+		return errors.WrapPrefixf(err, "failed to unmarshal Kptfile under %q", c.Dir)
 	}
 
 	// validate dependencies are well formed
@@ -124,12 +124,12 @@ func (c Command) sync(dependency kptfile.Dependency) error {
 	// verify the dep is well formed
 	if !f.IsDir() {
 		// place where dep should be fetched exists and is not a directory
-		return errors.Errorf("cannot sync to %s, non-directory file exists", path)
+		return errors.Errorf("cannot sync to %q, non-directory file exists", path)
 	}
 	_, err = os.Stat(filepath.Join(path, kptfile.KptFileName))
 	if os.IsNotExist(err) {
 		// dep doesn't have a Kptfile -- something is wrong
-		return errors.Errorf("expected Kptfile under dependency %s", path)
+		return errors.Errorf("expected Kptfile under dependency %q", path)
 	}
 	if err != nil {
 		return errors.Wrap(err)
@@ -156,7 +156,7 @@ func (c Command) sync(dependency kptfile.Dependency) error {
 // get fetches the dependency
 func (c Command) get(dependency kptfile.Dependency) error {
 	path := filepath.Join(c.Dir, dependency.Name)
-	fmt.Fprintf(c.StdOut, "fetching %s (%s)\n", dependency.Name, path)
+	fmt.Fprintf(c.StdOut, "fetching %q from %q\n", dependency.Name, path)
 	if c.DryRun {
 		return nil
 	}
@@ -171,7 +171,7 @@ func (c Command) get(dependency kptfile.Dependency) error {
 // update updates the version of the fetched dependency to match
 func (c Command) update(dependency kptfile.Dependency, k *kptfile.KptFile) error {
 	path := filepath.Join(c.Dir, dependency.Name)
-	fmt.Fprintf(c.StdOut, "updating %s (%s) from %s to %s\n",
+	fmt.Fprintf(c.StdOut, "updating %q (%s) from %q to %q\n",
 		dependency.Name, path, k.Upstream.Git.Ref, dependency.Git.Ref)
 	if c.DryRun {
 		return nil
@@ -194,7 +194,7 @@ func (c Command) update(dependency kptfile.Dependency, k *kptfile.KptFile) error
 // delete removes the dependency if it exists
 func (c Command) delete(dependency kptfile.Dependency) error {
 	path := filepath.Join(c.Dir, dependency.Name)
-	fmt.Fprintf(c.StdOut, "deleting %s (%s)\n", dependency.Name, path)
+	fmt.Fprintf(c.StdOut, "deleting %q from %q\n", dependency.Name, path)
 	if c.DryRun {
 		return nil
 	}