From e666630d36be79d3beef8d150fd4f6f48bb310de Mon Sep 17 00:00:00 2001
From: Narayanan Singaram <narayanan.singaram@gmail.com>
Date: Mon, 4 Mar 2019 13:50:49 -0800
Subject: [PATCH] Simplify map conversion logic

---
 pkg/commands/edit/add/addmetadata.go      | 34 ++++++++---------------
 pkg/commands/edit/add/addmetadata_test.go | 31 ++++++++++-----------
 2 files changed, 26 insertions(+), 39 deletions(-)

diff --git a/pkg/commands/edit/add/addmetadata.go b/pkg/commands/edit/add/addmetadata.go
index cbb3bc8a7a..d9104f35d5 100644
--- a/pkg/commands/edit/add/addmetadata.go
+++ b/pkg/commands/edit/add/addmetadata.go
@@ -130,29 +130,19 @@ func (o *addMetadataOptions) convertToMap(arg string) (map[string]string, error)
 	result := make(map[string]string)
 	inputs := strings.Split(arg, ",")
 	for _, input := range inputs {
-		kv := strings.Split(input, ":")
-		if len(kv[0]) < 1 {
-			return nil, o.makeError(input, "empty key")
-		}
-		if len(kv) > 2 {
-			// more than one colon found
-			// check if value is quoted
-			qc := strings.Index(input, ":\"")
-			if qc >= 1 && input[len(input)-1:] == "\"" {
-				//value is quoted
-				result[kv[0]] = input[qc+1:]
-			} else {
-				// value is not quoted, return error
-				return nil, o.makeError(input, "too many colons, quote the values")
-			}
-		} else if len(kv) == 2 {
-			result[kv[0]] = kv[1]
+		c := strings.Index(input, ":")
+		if c == 0 {
+			// key is not passed
+			return nil, o.makeError(input, "need k:v pair where v may be quoted")
+		} else if c < 0 {
+			// only key passed
+			result[input] = ""
 		} else {
-			result[kv[0]] = ""
+			// both key and value passed
+			key := input[:c]
+			value := trimQuotes(input[c+1:])
+			result[key] = value
 		}
-
-		// remove quotes if value is quoted
-		result[kv[0]] = trimQuotes(result[kv[0]])
 	}
 	return result, nil
 }
@@ -182,7 +172,7 @@ func (o *addMetadataOptions) writeToMap(m map[string]string, kind kindOfAdd) err
 }
 
 func (o *addMetadataOptions) makeError(input string, message string) error {
-	return fmt.Errorf("invalid %s: %s (%s)", o.kind, input, message)
+	return fmt.Errorf("invalid %s: '%s' (%s)", o.kind, input, message)
 }
 
 func trimQuotes(s string) string {
diff --git a/pkg/commands/edit/add/addmetadata_test.go b/pkg/commands/edit/add/addmetadata_test.go
index 8d1041ec32..6c7f0743ae 100644
--- a/pkg/commands/edit/add/addmetadata_test.go
+++ b/pkg/commands/edit/add/addmetadata_test.go
@@ -140,23 +140,21 @@ func TestAddAnnotationNoKey(t *testing.T) {
 	if err == nil {
 		t.Errorf("expected an error")
 	}
-	if err.Error() != "invalid annotation: :nokey (empty key)" {
+	if err.Error() != "invalid annotation: ':nokey' (need k:v pair where v may be quoted)" {
 		t.Errorf("incorrect error: %v", err.Error())
 	}
 }
 
 func TestAddAnnotationTooManyColons(t *testing.T) {
 	fakeFS := fs.MakeFakeFS()
+	fakeFS.WriteTestKustomization()
 	v := validators.MakeHappyMapValidator(t)
 	cmd := newCmdAddAnnotation(fakeFS, v.Validator)
 	args := []string{"key:v1:v2"}
 	err := cmd.RunE(cmd, args)
-	v.VerifyNoCall()
-	if err == nil {
-		t.Errorf("expected an error")
-	}
-	if err.Error() != "invalid annotation: key:v1:v2 (too many colons, quote the values)" {
-		t.Errorf("incorrect error: %v", err.Error())
+	v.VerifyCall()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err.Error())
 	}
 }
 
@@ -250,23 +248,21 @@ func TestAddLabelNoKey(t *testing.T) {
 	if err == nil {
 		t.Errorf("expected an error")
 	}
-	if err.Error() != "invalid label: :nokey (empty key)" {
+	if err.Error() != "invalid label: ':nokey' (need k:v pair where v may be quoted)" {
 		t.Errorf("incorrect error: %v", err.Error())
 	}
 }
 
 func TestAddLabelTooManyColons(t *testing.T) {
 	fakeFS := fs.MakeFakeFS()
+	fakeFS.WriteTestKustomization()
 	v := validators.MakeHappyMapValidator(t)
 	cmd := newCmdAddLabel(fakeFS, v.Validator)
 	args := []string{"key:v1:v2"}
 	err := cmd.RunE(cmd, args)
-	v.VerifyNoCall()
-	if err == nil {
-		t.Errorf("expected an error")
-	}
-	if err.Error() != "invalid label: key:v1:v2 (too many colons, quote the values)" {
-		t.Errorf("incorrect error: %v", err.Error())
+	v.VerifyCall()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err.Error())
 	}
 }
 
@@ -301,11 +297,12 @@ func TestAddLabelMultipleArgs(t *testing.T) {
 
 func TestConvertToMap(t *testing.T) {
 	var o addMetadataOptions
-	args := "a:b,c:\"d\",e:\"f:g\""
+	args := "a:b,c:\"d\",e:\"f:g\",g:h:k"
 	expected := make(map[string]string)
 	expected["a"] = "b"
 	expected["c"] = "d"
 	expected["e"] = "f:g"
+	expected["g"] = "h:k"
 
 	result, err := o.convertToMap(args)
 	if err != nil {
@@ -320,13 +317,13 @@ func TestConvertToMap(t *testing.T) {
 
 func TestConvertToMapError(t *testing.T) {
 	var o addMetadataOptions
-	args := "a:b,c:\"d\",e:f:g"
+	args := "a:b,c:\"d\",:f:g"
 
 	_, err := o.convertToMap(args)
 	if err == nil {
 		t.Errorf("expected an error")
 	}
-	if err.Error() != "invalid annotation: e:f:g (too many colons, quote the values)" {
+	if err.Error() != "invalid annotation: ':f:g' (need k:v pair where v may be quoted)" {
 		t.Errorf("incorrect error: %v", err.Error())
 	}
 }