Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: 5099-gh-release-create-upload-expand-glob-patterns-on-windows
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion pkg/cmd/gist/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{"-"}
}
Expand Down
26 changes: 25 additions & 1 deletion pkg/cmd/gist/create/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, "-"},
},
Expand All @@ -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{
Expand Down
9 changes: 7 additions & 2 deletions pkg/cmd/release/shared/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 31 additions & 12 deletions pkg/cmdutil/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
80 changes: 64 additions & 16 deletions pkg/cmdutil/args_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"errors"
"os"
"path/filepath"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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
Expand All @@ -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"},
Expand Down Expand Up @@ -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 {
Expand Down