From b986d80a49ecd08cc64bc58bbf83bf8c20537d1e Mon Sep 17 00:00:00 2001 From: Qi Xiao Date: Fri, 4 Sep 2020 22:04:20 +0100 Subject: [PATCH] pkg/eval: Move EachExternal to the fsutil package. --- pkg/edit/completion.go | 3 ++- pkg/eval/eval.go | 6 ----- pkg/eval/external_cmd.go | 17 ------------- pkg/eval/external_cmd_unix_test.go | 38 ------------------------------ pkg/eval/resolve.go | 3 ++- pkg/fsutil/search.go | 26 ++++++++++++++++++++ pkg/fsutil/search_unix_test.go | 38 ++++++++++++++++++++++++++++++ 7 files changed, 68 insertions(+), 63 deletions(-) create mode 100644 pkg/fsutil/search_unix_test.go diff --git a/pkg/edit/completion.go b/pkg/edit/completion.go index 77eeb21e..129e3bc1 100644 --- a/pkg/edit/completion.go +++ b/pkg/edit/completion.go @@ -13,6 +13,7 @@ import ( "github.com/elves/elvish/pkg/edit/complete" "github.com/elves/elvish/pkg/eval" "github.com/elves/elvish/pkg/eval/vals" + "github.com/elves/elvish/pkg/fsutil" "github.com/elves/elvish/pkg/parse" "github.com/elves/elvish/pkg/strutil" "github.com/xiaq/persistent/hash" @@ -496,7 +497,7 @@ func lookupFn(m vals.Map, ctxName string) (eval.Callable, bool) { type pureEvaler struct{ ev *eval.Evaler } -func (pureEvaler) EachExternal(f func(string)) { eval.EachExternal(f) } +func (pureEvaler) EachExternal(f func(string)) { fsutil.EachExternal(f) } func (pureEvaler) EachSpecial(f func(string)) { for name := range eval.IsBuiltinSpecial { diff --git a/pkg/eval/eval.go b/pkg/eval/eval.go index 6a4de1e8..771c1d34 100644 --- a/pkg/eval/eval.go +++ b/pkg/eval/eval.go @@ -7,10 +7,8 @@ import ( "io" "os" "strconv" - "strings" "github.com/elves/elvish/pkg/daemon" - "github.com/elves/elvish/pkg/env" "github.com/elves/elvish/pkg/eval/mods/bundled" "github.com/elves/elvish/pkg/eval/vals" "github.com/elves/elvish/pkg/eval/vars" @@ -248,10 +246,6 @@ func (ev *Evaler) SetLibDir(libDir string) { ev.libDir = libDir } -func searchPaths() []string { - return strings.Split(os.Getenv(env.PATH), ":") -} - // growPorts makes the size of ec.ports at least n, adding nil's if necessary. func (fm *Frame) growPorts(n int) { if len(fm.ports) >= n { diff --git a/pkg/eval/external_cmd.go b/pkg/eval/external_cmd.go index 88cc0768..d96dfbde 100644 --- a/pkg/eval/external_cmd.go +++ b/pkg/eval/external_cmd.go @@ -2,7 +2,6 @@ package eval import ( "errors" - "io/ioutil" "os" "os/exec" "syscall" @@ -97,19 +96,3 @@ func (e ExternalCmd) Call(fm *Frame, argVals []interface{}, opts map[string]inte } return NewExternalCmdExit(e.Name, state.Sys().(syscall.WaitStatus), proc.Pid) } - -// EachExternal calls f for each name that can resolve to an external command. -// -// TODO(xiaq): Windows support. See https://golang.org/pkg/os/#Chmod for why -// this doesn't work on Windows as currently written. -func EachExternal(f func(string)) { - for _, dir := range searchPaths() { - // TODO(xiaq): Ignore error. - infos, _ := ioutil.ReadDir(dir) - for _, info := range infos { - if !info.IsDir() && (info.Mode()&0111 != 0) { - f(info.Name()) - } - } - } -} diff --git a/pkg/eval/external_cmd_unix_test.go b/pkg/eval/external_cmd_unix_test.go index 8db404e2..bac2bb28 100644 --- a/pkg/eval/external_cmd_unix_test.go +++ b/pkg/eval/external_cmd_unix_test.go @@ -3,47 +3,9 @@ package eval_test import ( - "reflect" - "sort" "syscall" - "testing" - - . "github.com/elves/elvish/pkg/eval" - - "github.com/elves/elvish/pkg/testutil" ) -// TODO: When EachExternal is modified to work on Windows either fold this -// test into external_cmd_test.go or create an external_cmd_windows_test.go -// that performs an equivalent test on Windows. -func TestEachExternal(t *testing.T) { - binPath, cleanup := testutil.InTestDir() - defer cleanup() - - restorePath := testutil.WithTempEnv("PATH", "/foo:"+binPath+":/bar") - defer restorePath() - - testutil.ApplyDir(testutil.Dir{ - "dir": testutil.Dir{}, - "file": "", - "cmdx": "#!/bin/sh", - "cmd1": testutil.File{Perm: 0755, Content: "#!/bin/sh"}, - "cmd2": testutil.File{Perm: 0755, Content: "#!/bin/sh"}, - "cmd3": testutil.File{Perm: 0755, Content: ""}, - }) - - wantCmds := []string{"cmd1", "cmd2", "cmd3"} - gotCmds := []string{} - EachExternal(func(filename string) { - gotCmds = append(gotCmds, filename) - }) - - sort.Strings(gotCmds) - if !reflect.DeepEqual(wantCmds, gotCmds) { - t.Errorf("EachExternal want %q got %q", wantCmds, gotCmds) - } -} - func exitWaitStatus(exit uint32) syscall.WaitStatus { // The exit<<8 is gross but I can't find any exported symbols that would // allow us to construct WaitStatus. So assume legacy UNIX encoding diff --git a/pkg/eval/resolve.go b/pkg/eval/resolve.go index 735f7af8..a89de961 100644 --- a/pkg/eval/resolve.go +++ b/pkg/eval/resolve.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/elves/elvish/pkg/eval/vars" + "github.com/elves/elvish/pkg/fsutil" ) // Resolution and iteration of variables and namespaces. @@ -25,7 +26,7 @@ func (ev *evalerScopes) EachVariableInTop(ns string, f func(s string)) { f(name) } case "e:": - EachExternal(func(cmd string) { + fsutil.EachExternal(func(cmd string) { f(cmd + FnSuffix) }) case "E:": diff --git a/pkg/fsutil/search.go b/pkg/fsutil/search.go index 185962fa..d53023a9 100644 --- a/pkg/fsutil/search.go +++ b/pkg/fsutil/search.go @@ -1,9 +1,12 @@ package fsutil import ( + "io/ioutil" "os" "path/filepath" "strings" + + "github.com/elves/elvish/pkg/env" ) // DontSearch determines whether the path to an external command should be @@ -22,3 +25,26 @@ func IsExecutable(path string) bool { fm := fi.Mode() return !fm.IsDir() && (fm&0111 != 0) } + +// EachExternal calls f for each name that can resolve to an external command. +// +// BUG: EachExternal may generate the same command multiple command it it +// appears in multiple directories in PATH. +// +// BUG: EachExternal doesn't work on Windows since it relies on the execution +// permission bit, which doesn't exist on Windows. +func EachExternal(f func(string)) { + for _, dir := range searchPaths() { + // TODO(xiaq): Ignore error. + infos, _ := ioutil.ReadDir(dir) + for _, info := range infos { + if !info.IsDir() && (info.Mode()&0111 != 0) { + f(info.Name()) + } + } + } +} + +func searchPaths() []string { + return strings.Split(os.Getenv(env.PATH), ":") +} diff --git a/pkg/fsutil/search_unix_test.go b/pkg/fsutil/search_unix_test.go new file mode 100644 index 00000000..2432e573 --- /dev/null +++ b/pkg/fsutil/search_unix_test.go @@ -0,0 +1,38 @@ +package fsutil + +import ( + "reflect" + "sort" + "testing" + + "github.com/elves/elvish/pkg/testutil" +) + +// TODO: When EachExternal is modified to work on Windows either fold this +// test into external_cmd_test.go or create an external_cmd_windows_test.go +// that performs an equivalent test on Windows. +func TestEachExternal(t *testing.T) { + binPath, cleanup := testutil.InTestDir() + defer cleanup() + + restorePath := testutil.WithTempEnv("PATH", "/foo:"+binPath+":/bar") + defer restorePath() + + testutil.ApplyDir(testutil.Dir{ + "dir": testutil.Dir{}, + "file": "", + "cmdx": "#!/bin/sh", + "cmd1": testutil.File{Perm: 0755, Content: "#!/bin/sh"}, + "cmd2": testutil.File{Perm: 0755, Content: "#!/bin/sh"}, + "cmd3": testutil.File{Perm: 0755, Content: ""}, + }) + + wantCmds := []string{"cmd1", "cmd2", "cmd3"} + gotCmds := []string{} + EachExternal(func(cmd string) { gotCmds = append(gotCmds, cmd) }) + + sort.Strings(gotCmds) + if !reflect.DeepEqual(wantCmds, gotCmds) { + t.Errorf("EachExternal want %q got %q", wantCmds, gotCmds) + } +}