Separate partitioning from globbing in cmdutil/args package and consumers (#3)

* Separate partitioning from globbing in cmdutil/args package and consumers

In the previous commit, GlobPaths was overloaded, containing logic
specific to command use-cases. This commit removes that functionality from
GlobPaths and back into the commands that have the special use-cases.

To do this, I've introduced a new Partition util in cmdutil/args.go that
will separate a slice into two slices given a predicate. This functionality
is leveraged by both the special use-cases described above to separate the
command-specific syntax from the globable filepaths.

* Add test to validate that the order of '-' in gh gist create args doesn't matter

pkg/cmd/gist/create/create.go

@@ -105,11 +105,18 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
 }
 
 func createRun(opts *CreateOptions) error {
-	filenames, err := cmdutil.GlobPaths(opts.Filenames, nil)
+
+	readFromStdInArg, filenames := cmdutil.Partition(opts.Filenames, func(f string) bool {
+		return f == "-"
+	})
+
+	filenames, err := cmdutil.GlobPaths(filenames)
 	if err != nil {
 		return err
 	}
 
+	filenames = append(filenames, readFromStdInArg...)
+
 	if len(filenames) == 0 {
 		filenames = []string{"-"}
 	}

pkg/cmd/gist/create/create_test.go

@@ -225,7 +225,7 @@ func Test_createRun(t *testing.T) {
 			responseStatus: http.StatusOK,
 		},
 		{
-			name: "multiple files",
+			name: "when both a file and the stdin '-' are provided, it matches on all the files passed in and stdin",
 			opts: &CreateOptions{
 				Filenames: []string{fixtureFile, "-"},
 			},
@@ -248,6 +248,30 @@ func Test_createRun(t *testing.T) {
 			},
 			responseStatus: http.StatusOK,
 		},
+		{
+			name: "when both a file and the stdin '-' are provided, but '-' is not the last argument, it matches on all the files provided and stdin",
+			opts: &CreateOptions{
+				Filenames: []string{"-", fixtureFile},
+			},
+			stdin:      "cool stdin content",
+			wantOut:    "https://gist.github.com/aa5a315d61ae9438b18d\n",
+			wantStderr: "- Creating gist with multiple files\n✓ Created secret gist fixture.txt\n",
+			wantErr:    false,
+			wantParams: map[string]interface{}{
+				"description": "",
+				"updated_at":  "0001-01-01T00:00:00Z",
+				"public":      false,
+				"files": map[string]interface{}{
+					"fixture.txt": map[string]interface{}{
+						"content": "{}",
+					},
+					"gistfile1.txt": map[string]interface{}{
+						"content": "cool stdin content",
+					},
+				},
+			},
+			responseStatus: http.StatusOK,
+		},
 		{
 			name: "file with empty content",
 			opts: &CreateOptions{

pkg/cmd/release/shared/upload.go

@@ -37,12 +37,17 @@ type AssetForUpload struct {
 }
 
 func AssetsFromArgs(args []string) (assets []*AssetForUpload, err error) {
-	args, err = cmdutil.GlobPaths(args, func(pattern string) bool {
-		return strings.Contains(pattern, "#")
+	labeledArgs, unlabeledArgs := cmdutil.Partition(args, func(arg string) bool {
+		return strings.Contains(arg, "#")
 	})
+
+	args, err = cmdutil.GlobPaths(unlabeledArgs)
 	if err != nil {
 		return nil, err
 	}
+
+	args = append(args, labeledArgs...)
+
 	for _, arg := range args {
 		var label string
 		fn := arg

pkg/cmdutil/args.go

@@ -59,22 +59,41 @@ func NoArgsQuoteReminder(cmd *cobra.Command, args []string) error {
 	return FlagErrorf("%s", errMsg)
 }
 
-func GlobPaths(patterns []string, exclude func(pattern string) bool) ([]string, error) {
+// Partition takes a slice of any type T and separates it into two slices
+// of the same type based on the provided predicate function. Any item
+// that returns true for the predicate will be included in the first slice
+// returned, and any item that returns false for the predicate will be
+// included in the second slice returned.
+func Partition[T any](slice []T, predicate func(T) bool) ([]T, []T) {
+	var matching, nonMatching []T
+	for _, item := range slice {
+		if predicate(item) {
+			matching = append(matching, item)
+		} else {
+			nonMatching = append(nonMatching, item)
+		}
+	}
+	return matching, nonMatching
+}
+
+// GlobPaths expands a list of file patterns into a list of file paths.
+// If no files match a pattern, that pattern will return an error.
+// If no pattern is passed, this returns an empty list and no error.
+//
+// For information on supported glob patterns, see
+// https://pkg.go.dev/path/filepath#Match
+func GlobPaths(patterns []string) ([]string, error) {
 	expansions := []string{}
 
 	for _, pattern := range patterns {
-		if pattern == "-" || (exclude != nil && exclude(pattern)) {
-			expansions = append(expansions, pattern)
-		} else {
-			matches, err := filepath.Glob(pattern)
-			if err != nil {
-				return nil, fmt.Errorf("%s: %v", pattern, err)
-			}
-			if len(matches) == 0 {
-				return []string{}, fmt.Errorf("no matches found for `%s`", pattern)
-			}
-			expansions = append(expansions, matches...)
+		matches, err := filepath.Glob(pattern)
+		if err != nil {
+			return nil, fmt.Errorf("%s: %v", pattern, err)
 		}
+		if len(matches) == 0 {
+			return []string{}, fmt.Errorf("no matches found for `%s`", pattern)
+		}
+		expansions = append(expansions, matches...)
 	}
 
 	return expansions, nil

pkg/cmdutil/args_test.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -58,6 +57,69 @@ func TestMinimumNs_with_error(t *testing.T) {
 	}
 }
 
+func TestPartition(t *testing.T) {
+	tests := []struct {
+		name            string
+		slice           []any
+		predicate       func(any) bool
+		wantMatching    []any
+		wantNonMatching []any
+	}{
+		{
+			name:  "When the slice is empty, it returns two empty slices",
+			slice: []any{},
+			predicate: func(any) bool {
+				return true
+			},
+			wantMatching:    []any{},
+			wantNonMatching: []any{},
+		},
+		{
+			name: "when the slice has one element that satisfies the predicate, it returns a slice with that element and an empty slice",
+			slice: []any{
+				"foo",
+			},
+			predicate: func(any) bool {
+				return true
+			},
+			wantMatching:    []any{"foo"},
+			wantNonMatching: []any{},
+		},
+		{
+			name: "when the slice has one element that does not satisfy the predicate, it returns an empty slice and a slice with that element",
+			slice: []any{
+				"foo",
+			},
+			predicate: func(any) bool {
+				return false
+			},
+			wantMatching:    []any{},
+			wantNonMatching: []any{"foo"},
+		},
+		{
+			name: "when the slice has multiple elements, it returns a slice with the elements that satisfy the predicate and a slice with the elements that do not satisfy the predicate",
+			slice: []any{
+				"foo",
+				"bar",
+				"baz",
+			},
+			predicate: func(s any) bool {
+				return s.(string) != "foo"
+			},
+			wantMatching:    []any{"bar", "baz"},
+			wantNonMatching: []any{"foo"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotMatching, gotNonMatching := Partition(tt.slice, tt.predicate)
+			assert.ElementsMatch(t, tt.wantMatching, gotMatching)
+			assert.ElementsMatch(t, tt.wantNonMatching, gotNonMatching)
+		})
+	}
+}
+
 func TestGlobPaths(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -72,18 +134,6 @@ func TestGlobPaths(t *testing.T) {
 			wantOut:  []string{},
 			wantErr:  nil,
 		},
-		{
-			name:     "When - is passed, return -",
-			patterns: []string{"-"},
-			wantOut:  []string{"-"},
-			wantErr:  nil,
-		},
-		{
-			name:     "When labels are passed, return labels",
-			patterns: []string{"file.txt#Text File", "README.md#README"},
-			wantOut:  []string{"file.txt#Text File", "README.md#README"},
-			wantErr:  nil,
-		},
 		{
 			name:     "When no files match, it returns an empty expansions array with error",
 			patterns: []string{"foo"},
@@ -131,9 +181,7 @@ func TestGlobPaths(t *testing.T) {
 			cleanupFn := createTestDir(t)
 			defer cleanupFn()
 
-			got, err := GlobPaths(tt.patterns, func(pattern string) bool {
-				return strings.Contains(pattern, "#")
-			})
+			got, err := GlobPaths(tt.patterns)
 			if tt.wantErr != nil {
 				assert.EqualError(t, err, tt.wantErr.Error())
 			} else {