diff --git a/commands/internal/arguments/arguments.go b/commands/internal/arguments/arguments.go index 465bd73..fdddb01 100644 --- a/commands/internal/arguments/arguments.go +++ b/commands/internal/arguments/arguments.go @@ -9,31 +9,48 @@ import ( "strconv" ) -var expandArgDigitMatcher = regexp.MustCompile("^[0-9]{0,4}$") -var expandArgRangeMatcher = regexp.MustCompile("^([0-9]+)-([0-9]+)$") +var ( + expandArgDigitMatcher = regexp.MustCompile("^[0-9]{0,4}$") + expandArgRangeMatcher = regexp.MustCompile("^([0-9]+)-([0-9]+)$") + managedEnvVar = regexp.MustCompile(`^\$e\d+$`) +) -// EvaluateEnvironment evaluates a string of arguments and expands environment variables. +// EvaluateEnvironment evaluates a single arguments and expands environment +// variables. +// +// For scmpuff-managed position variables only (e.g. $e1, etc), the variable is +// expanded into a locatable file path, and expandRelative is true, it will be +// converted into a relative path when possible. func EvaluateEnvironment(arg string, expandRelative bool) string { - expandedArg := os.ExpandEnv(arg) - if expandRelative { - return convertToRelativeIfFilePath(expandedArg) + expanded := os.ExpandEnv(arg) + wasChanged := (expanded != arg) + if wasChanged && expandRelative && managedEnvVar.MatchString(arg) { + relPath, err := convertToRelativeIfFilePath(expanded) + if err == nil { + return relPath + } } - return expandedArg + return expanded } // For a given arg, try to determine if it represents a file, and if so, convert // it to a relative filepath. // // Otherwise (or if any error conditions occur) return it unmolested. -func convertToRelativeIfFilePath(arg string) string { - if _, err := os.Stat(arg); err == nil { - wd, err1 := os.Getwd() - relPath, err2 := filepath.Rel(wd, arg) - if err1 == nil && err2 == nil { - return relPath - } +func convertToRelativeIfFilePath(arg string) (string, error) { + _, err := os.Stat(arg) + if err != nil { + return arg, err + } + wd, err := os.Getwd() + if err != nil { + return arg, err + } + relPath, err := filepath.Rel(wd, arg) + if err != nil { + return arg, err } - return arg + return relPath, nil } // Expand takes the list of arguments received from the command line and expands diff --git a/commands/internal/arguments/arguments_test.go b/commands/internal/arguments/arguments_test.go index 4ca8570..f8a5df9 100644 --- a/commands/internal/arguments/arguments_test.go +++ b/commands/internal/arguments/arguments_test.go @@ -1,6 +1,8 @@ package arguments import ( + "os" + "path/filepath" "reflect" "strings" "testing" @@ -46,3 +48,54 @@ func TestExpandArg(t *testing.T) { } } } + +func TestEvaluateEnvironment(t *testing.T) { + // It would be wonderful to use fstest.MapFS here and have the function rely + // upon fs.StatFS, however MapFS currently does not work with absolute paths + // at all, which makes it useless for our testing here. Bummer. So we use a + // testdata fixture instead. + /* var mockFS = fstest.MapFS{ + "/foo/a.txt": &fstest.MapFile{Mode: 0644}, + "/foo/b.txt": &fstest.MapFile{Mode: 0644}, + "/foo/bar/c.txt": &fstest.MapFile{Mode: 0644}, + "/usr/local/bin/git": &fstest.MapFile{Mode: 0777}, + } */ + wd, err := os.Getwd() + if err != nil { + t.Skip("failed to get wd, cannot test") + } + + fakegitAbsPath := filepath.Join(wd, "testdata", "bin", "fakegit") + t.Setenv("SCMPUFF_GIT_CMD", fakegitAbsPath) + t.Setenv("e1", filepath.Join(wd, "testdata", "a.txt")) + t.Setenv("e2", filepath.Join(wd, "testdata", "b.txt")) + t.Setenv("FOO_USER", "not_a_file") + + t.Logf("$CWD=%v", wd) + t.Logf("$SCMPUFF_GIT_CMD=%v", os.Getenv("SCMPUFF_GIT_CMD")) + t.Logf("$e1=%v", os.Getenv("e1")) + t.Logf("$e2=%v", os.Getenv("e2")) + + tests := []struct { + name string + arg string + expandRelative bool + want string + }{ + {name: "not an env var", arg: "eee", expandRelative: false, want: "eee"}, + {name: "not file absolute", arg: "$FOO_USER", expandRelative: false, want: "not_a_file"}, + {name: "not file relative", arg: "$FOO_USER", expandRelative: true, want: "not_a_file"}, + {name: "absolute file", arg: "$e1", expandRelative: false, want: filepath.Join(wd, "testdata", "a.txt")}, + {name: "relative file", arg: "$e1", expandRelative: true, want: filepath.FromSlash("testdata/a.txt")}, + {name: "path binary dont convert relative - abs", arg: "$SCMPUFF_GIT_CMD", expandRelative: false, want: fakegitAbsPath}, + {name: "path binary dont convert relative - rel", arg: "$SCMPUFF_GIT_CMD", expandRelative: true, want: fakegitAbsPath}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := EvaluateEnvironment(tt.arg, tt.expandRelative); got != tt.want { + t.Errorf("EvaluateEnvironment(%v, %v) = %v, want %v", tt.arg, tt.expandRelative, got, tt.want) + } + }) + } +} diff --git a/commands/internal/arguments/testdata/a.txt b/commands/internal/arguments/testdata/a.txt new file mode 100644 index 0000000..7898192 --- /dev/null +++ b/commands/internal/arguments/testdata/a.txt @@ -0,0 +1 @@ +a diff --git a/commands/internal/arguments/testdata/b.txt b/commands/internal/arguments/testdata/b.txt new file mode 100644 index 0000000..6178079 --- /dev/null +++ b/commands/internal/arguments/testdata/b.txt @@ -0,0 +1 @@ +b diff --git a/features/command_expand.feature b/features/command_expand.feature index 5ee0a99..8435ed7 100644 --- a/features/command_expand.feature +++ b/features/command_expand.feature @@ -69,16 +69,33 @@ Feature: command expansion at command line Then the stderr should not contain anything And the output should match /git\tcommit\t-m\tfoo\\;\\ bar/ - Scenario: Allow user to specify --relative paths + Scenario Outline: Allow user to specify --relative paths Given a directory named "foo" And a directory named "foo/bar" And an empty file named "xxx.jpg" And I cd to "foo/bar" Given I override environment variable "e1" to the absolute path of "xxx.jpg" - When I successfully run `scmpuff expand 1` - Then the stdout from "scmpuff expand 1" should contain "/tmp/aruba/xxx.jpg" - When I successfully run `scmpuff expand -r -- 1` - Then the stdout from "scmpuff expand -r -- 1" should contain "../../xxx.jpg" + When I successfully run `` + Then the output should match + Examples: + | cmd | output_regex | + | scmpuff expand 1 | %r<^\S*/tmp/aruba/xxx\.jpg$> | + | scmpuff expand -r -- 1 | %r<\.\./\.\./xxx\.jpg> | + + Scenario Outline: Do not convert anything other than special variables to --relative paths + Given a directory named "foo" + And a directory named "foo/bar" + And an empty file named "fakegit" + And an empty file named "xxx.jpg" + And I cd to "foo/bar" + Given I override environment variable "SCMPUFF_GIT_CMD" to the absolute path of "fakegit" + And I override environment variable "e1" to the absolute path of "xxx.jpg" + When I successfully run `` + Then the output should match + Examples: + | cmd | output_regex | + | scmpuff expand -- $SCMPUFF_GIT_CMD checkout 1 | %r<^\S*/tmp/aruba/fakegit\tcheckout\t\S*/tmp/aruba/xxx\.jpg$> | + | scmpuff expand -r -- $SCMPUFF_GIT_CMD checkout 1 | %r<^\S*/tmp/aruba/fakegit\tcheckout\t\.\./\.\./xxx\.jpg$> | Scenario: Don't trim empty string args when expanding command There are certain situations where someone would want to actually pass an