From ea136e4e77259e27db0b89a88d7d57cad6062d1e Mon Sep 17 00:00:00 2001
From: mjarkk <mkopenga@gmail.com>
Date: Fri, 6 Aug 2021 21:50:53 +0200
Subject: [PATCH] Improve code quality

- Make CommandMenuEntry private
- create candidates only once we really need it
- Use only 1 buffer
- Clearify CommandMenuEntry creation fields
---
 pkg/gui/custom_commands.go | 36 ++++++++++++++++++------------------
 pkg/gui/gui_test.go        |  8 ++++----
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/pkg/gui/custom_commands.go b/pkg/gui/custom_commands.go
index bac65801d..a9795f7b6 100644
--- a/pkg/gui/custom_commands.go
+++ b/pkg/gui/custom_commands.go
@@ -33,7 +33,7 @@ type CustomCommandObjects struct {
 	PromptResponses        []string
 }
 
-type CommandMenuEntry struct {
+type commandMenuEntry struct {
 	label string
 	value string
 }
@@ -123,30 +123,30 @@ func (gui *Gui) menuPrompt(prompt config.CustomCommandPrompt, promptResponses []
 	return gui.createMenu(title, menuItems, createMenuOptions{showCancel: true})
 }
 
-func (gui *Gui) GenerateMenuCandidates(commandOutput, filter, valueFormat, labelFormat string) ([]CommandMenuEntry, error) {
-	candidates := []CommandMenuEntry{}
-
+func (gui *Gui) GenerateMenuCandidates(commandOutput, filter, valueFormat, labelFormat string) ([]commandMenuEntry, error) {
 	reg, err := regexp.Compile(filter)
 	if err != nil {
-		return candidates, gui.surfaceError(errors.New("unable to parse filter regex, error: " + err.Error()))
+		return nil, gui.surfaceError(errors.New("unable to parse filter regex, error: " + err.Error()))
 	}
 
-	valueBuff := bytes.NewBuffer(nil)
+	buff := bytes.NewBuffer(nil)
+
 	valueTemp, err := template.New("format").Parse(valueFormat)
 	if err != nil {
-		return candidates, gui.surfaceError(errors.New("unable to parse value format, error: " + err.Error()))
+		return nil, gui.surfaceError(errors.New("unable to parse value format, error: " + err.Error()))
 	}
 
-	descBuff := bytes.NewBuffer(nil)
 	descTemp, err := template.New("format").Parse(labelFormat)
 	if err != nil {
-		return candidates, gui.surfaceError(errors.New("unable to parse label format, error: " + err.Error()))
+		return nil, gui.surfaceError(errors.New("unable to parse label format, error: " + err.Error()))
 	}
 
+	candidates := []commandMenuEntry{}
 	for _, str := range strings.Split(string(commandOutput), "\n") {
 		if str == "" {
 			continue
 		}
+
 		tmplData := map[string]string{}
 		out := reg.FindAllStringSubmatch(str, -1)
 		if len(out) > 0 {
@@ -161,28 +161,28 @@ func (gui *Gui) GenerateMenuCandidates(commandOutput, filter, valueFormat, label
 			}
 		}
 
-		err = valueTemp.Execute(valueBuff, tmplData)
+		err = valueTemp.Execute(buff, tmplData)
 		if err != nil {
 			return candidates, gui.surfaceError(err)
 		}
+		entry := commandMenuEntry{
+			value: strings.TrimSpace(buff.String()),
+		}
 
 		if labelFormat != "" {
-			err = descTemp.Execute(descBuff, tmplData)
+			buff.Reset()
+			err = descTemp.Execute(buff, tmplData)
 			if err != nil {
 				return candidates, gui.surfaceError(err)
 			}
+			entry.label = strings.TrimSpace(buff.String())
 		} else {
-			descBuff.Write(valueBuff.Bytes())
+			entry.label = entry.value
 		}
 
-		entry := CommandMenuEntry{
-			strings.TrimSpace(descBuff.String()),
-			strings.TrimSpace(valueBuff.String()),
-		}
 		candidates = append(candidates, entry)
 
-		valueBuff.Reset()
-		descBuff.Reset()
+		buff.Reset()
 	}
 	return candidates, err
 }
diff --git a/pkg/gui/gui_test.go b/pkg/gui/gui_test.go
index e3989ba24..a50f093be 100644
--- a/pkg/gui/gui_test.go
+++ b/pkg/gui/gui_test.go
@@ -88,7 +88,7 @@ func TestGuiGenerateMenuCandidates(t *testing.T) {
 		filter      string
 		valueFormat string
 		labelFormat string
-		test        func([]CommandMenuEntry, error)
+		test        func([]commandMenuEntry, error)
 	}
 
 	scenarios := []scenario{
@@ -98,7 +98,7 @@ func TestGuiGenerateMenuCandidates(t *testing.T) {
 			"(?P<remote>[a-z_]+)/(?P<branch>.*)",
 			"{{ .branch }}",
 			"Remote: {{ .remote }}",
-			func(actualEntry []CommandMenuEntry, err error) {
+			func(actualEntry []commandMenuEntry, err error) {
 				assert.NoError(t, err)
 				assert.EqualValues(t, "pr-1", actualEntry[0].value)
 				assert.EqualValues(t, "Remote: upstream", actualEntry[0].label)
@@ -110,7 +110,7 @@ func TestGuiGenerateMenuCandidates(t *testing.T) {
 			"(?P<remote>[a-z]*)/(?P<branch>.*)",
 			"{{ .branch }}|{{ .remote }}",
 			"",
-			func(actualEntry []CommandMenuEntry, err error) {
+			func(actualEntry []commandMenuEntry, err error) {
 				assert.NoError(t, err)
 				assert.EqualValues(t, "pr-1|upstream", actualEntry[0].value)
 				assert.EqualValues(t, "pr-1|upstream", actualEntry[0].label)
@@ -122,7 +122,7 @@ func TestGuiGenerateMenuCandidates(t *testing.T) {
 			"(?P<remote>[a-z]*)/(?P<branch>.*)",
 			"{{ .group_2 }}|{{ .group_1 }}",
 			"Remote: {{ .group_1 }}",
-			func(actualEntry []CommandMenuEntry, err error) {
+			func(actualEntry []commandMenuEntry, err error) {
 				assert.NoError(t, err)
 				assert.EqualValues(t, "pr-1|upstream", actualEntry[0].value)
 				assert.EqualValues(t, "Remote: upstream", actualEntry[0].label)