From b432b88c229c0131b344c75b7aae4797b8b56661 Mon Sep 17 00:00:00 2001
From: Gajesh Bhat <gajeshbht@gmail.com>
Date: Sat, 23 Aug 2025 16:06:52 -0700
Subject: [PATCH] Fix semicolon-separated parameter parsing for git
 count-contributors (#3052)

- Enhanced GetStringsArrFlagValue() to properly handle semicolon-separated values with trimming and empty string filtering
- Enhanced overrideArrayIfSet() with improved parsing and error handling
- Added comprehensive tests for semicolon parsing utility functions

This fixes the issue where git count-contributors --repo-name parameter fails with multiple repositories due to
shell parsing of unquoted semicolons and improves the underlying utility functions used by platform services commands.
---
 utils/cliutils/codegangstautils.go       |  14 ++-
 utils/cliutils/semicolon_parsing_test.go | 106 +++++++++++++++++++++++
 utils/cliutils/utils.go                  |  16 +++-
 3 files changed, 134 insertions(+), 2 deletions(-)
 create mode 100644 utils/cliutils/semicolon_parsing_test.go

diff --git a/utils/cliutils/codegangstautils.go b/utils/cliutils/codegangstautils.go
index 036936332..b4d2ed29c 100644
--- a/utils/cliutils/codegangstautils.go
+++ b/utils/cliutils/codegangstautils.go
@@ -20,9 +20,21 @@ func GetIntFlagValue(c *cli.Context, flagName string, defValue int) (int, error)
 	return defValue, nil
 }
 
+// GetStringsArrFlagValue parses semicolon-separated flag values into a string slice.
 func GetStringsArrFlagValue(c *cli.Context, flagName string) (resultArray []string) {
 	if c.IsSet(flagName) {
-		resultArray = append(resultArray, strings.Split(c.String(flagName), ";")...)
+		flagValue := c.String(flagName)
+		if flagValue == "" {
+			return []string{}
+		}
+
+		parts := strings.Split(flagValue, ";")
+		for _, part := range parts {
+			trimmed := strings.TrimSpace(part)
+			if trimmed != "" {
+				resultArray = append(resultArray, trimmed)
+			}
+		}
 	}
 	return
 }
diff --git a/utils/cliutils/semicolon_parsing_test.go b/utils/cliutils/semicolon_parsing_test.go
new file mode 100644
index 000000000..f4d15f4b8
--- /dev/null
+++ b/utils/cliutils/semicolon_parsing_test.go
@@ -0,0 +1,106 @@
+package cliutils
+
+import (
+	"flag"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/urfave/cli"
+)
+
+func TestGetStringsArrFlagValue(t *testing.T) {
+	tests := []struct {
+		name     string
+		flagValue string
+		expected []string
+	}{
+		{
+			name:     "Single value",
+			flagValue: "repo1",
+			expected: []string{"repo1"},
+		},
+		{
+			name:     "Multiple values",
+			flagValue: "repo1;repo2;repo3",
+			expected: []string{"repo1", "repo2", "repo3"},
+		},
+		{
+			name:     "Values with spaces",
+			flagValue: "repo1; repo2 ; repo3",
+			expected: []string{"repo1", "repo2", "repo3"},
+		},
+		{
+			name:     "Empty value",
+			flagValue: "",
+			expected: []string{},
+		},
+		{
+			name:     "Values with empty parts",
+			flagValue: "repo1;;repo2;",
+			expected: []string{"repo1", "repo2"},
+		},
+		{
+			name:     "Complex repository names",
+			flagValue: "my-org/repo1;another-org/repo-with-dashes;third_org/repo_with_underscores",
+			expected: []string{"my-org/repo1", "another-org/repo-with-dashes", "third_org/repo_with_underscores"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			flagSet := flag.NewFlagSet("test", flag.ContinueOnError)
+			flagSet.String("test-flag", tt.flagValue, "")
+			err := flagSet.Parse([]string{"--test-flag", tt.flagValue})
+			assert.NoError(t, err)
+
+			c := cli.NewContext(nil, flagSet, nil)
+
+			result := GetStringsArrFlagValue(c, "test-flag")
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestOverrideArrayIfSet(t *testing.T) {
+	tests := []struct {
+		name     string
+		flagValue string
+		expected []string
+	}{
+		{
+			name:     "Single repository",
+			flagValue: "my-repo",
+			expected: []string{"my-repo"},
+		},
+		{
+			name:     "Multiple repositories",
+			flagValue: "repo1;repo2;repo3",
+			expected: []string{"repo1", "repo2", "repo3"},
+		},
+		{
+			name:     "Repositories with spaces",
+			flagValue: " repo1 ; repo2 ; repo3 ",
+			expected: []string{"repo1", "repo2", "repo3"},
+		},
+		{
+			name:     "Empty flag value",
+			flagValue: "",
+			expected: []string{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			flagSet := flag.NewFlagSet("test", flag.ContinueOnError)
+			flagSet.String("test-flag", tt.flagValue, "")
+			err := flagSet.Parse([]string{"--test-flag", tt.flagValue})
+			assert.NoError(t, err)
+
+			c := cli.NewContext(nil, flagSet, nil)
+
+			var result []string
+			overrideArrayIfSet(&result, c, "test-flag")
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
\ No newline at end of file
diff --git a/utils/cliutils/utils.go b/utils/cliutils/utils.go
index d73aec8a9..abf1a72b4 100644
--- a/utils/cliutils/utils.go
+++ b/utils/cliutils/utils.go
@@ -385,7 +385,21 @@ func overrideStringIfSet(field *string, c *cli.Context, fieldName string) {
 // If `fieldName` exist in the cli args, read it to `field` as an array split by `;`.
 func overrideArrayIfSet(field *[]string, c *cli.Context, fieldName string) {
 	if c.IsSet(fieldName) {
-		*field = append([]string{}, strings.Split(c.String(fieldName), ";")...)
+		flagValue := c.String(fieldName)
+		if flagValue == "" {
+			*field = []string{}
+			return
+		}
+
+		parts := strings.Split(flagValue, ";")
+		result := []string{}
+		for _, part := range parts {
+			trimmed := strings.TrimSpace(part)
+			if trimmed != "" {
+				result = append(result, trimmed)
+			}
+		}
+		*field = result
 	}
 }