From d4822839a2ea5f7d5c8960c8d8479beeecbe2692 Mon Sep 17 00:00:00 2001
From: Cam Hutchison <camh@xdna.net>
Date: Tue, 2 Jun 2020 11:19:45 +1000
Subject: [PATCH] Add GetSep to simplify Tag initialisation

Factor out some repetition from `parseTag()` into a `GetSep()` method so
as to simplify `parseTag()` and formalise the semantics of a separator.

The existing behaviour in corner/error cases has been preserved, but an
appropriate error is returned (currently ignored) so a future breaking
change could be made to handle these conditions.

Tests have not been added as the test files are in a separate package to
the unit under test, making direct `Tag` construction difficult. Testing
through the exported APIs is already performed in the
`FormatPlaceHolder()` tests.
---
 tag.go | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/tag.go b/tag.go
index b0d2fe5..4c58517 100644
--- a/tag.go
+++ b/tag.go
@@ -151,26 +151,12 @@ func parseTag(fv reflect.Value, ft reflect.StructField) *Tag {
 	t.Short, _ = t.GetRune("short")
 	t.Hidden = t.Has("hidden")
 	t.Format = t.Get("format")
+	t.Sep, _ = t.GetSep("sep", ',')
+	t.MapSep, _ = t.GetSep("mapsep", ';')
 	t.Group = t.Get("group")
 	t.Xor = t.Get("xor")
 	t.Prefix = t.Get("prefix")
 	t.Embed = t.Has("embed")
-	if t.Get("sep") == "none" {
-		t.Sep = -1
-	} else {
-		t.Sep, _ = t.GetRune("sep")
-		if t.Sep == 0 {
-			t.Sep = ','
-		}
-	}
-	if t.Get("mapsep") == "none" {
-		t.MapSep = -1
-	} else {
-		t.MapSep, _ = t.GetRune("mapsep")
-		if t.MapSep == 0 {
-			t.MapSep = ';'
-		}
-	}
 	t.Vars = Vars{}
 	for _, set := range t.GetAll("set") {
 		parts := strings.SplitN(set, "=", 2)
@@ -232,3 +218,23 @@ func (t *Tag) GetRune(k string) (rune, error) {
 	}
 	return r, nil
 }
+
+// GetSep parses the given tag as a rune separator, allowing for a default or none.
+// The separator is returned, or -1 if "none" is specified. If the tag value is an
+// invalid utf8 sequence, the default rune is returned as well as an error. If the
+// tag value is more than one rune, the first rune is returned as well as an error.
+func (t *Tag) GetSep(k string, dflt rune) (rune, error) {
+	tv := t.Get(k)
+	if tv == "none" {
+		return -1, nil
+	} else if tv == "" {
+		return dflt, nil
+	}
+	r, size := utf8.DecodeRuneInString(tv)
+	if r == utf8.RuneError {
+		return dflt, fmt.Errorf(`%v:"%v" has a rune error`, k, tv)
+	} else if size != len(tv) {
+		return r, fmt.Errorf(`%v:"%v" is more than a single rune`, k, tv)
+	}
+	return r, nil
+}