pkg/eval: Move EachExternal to the fsutil package.

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 {

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 {

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())
-			}
-		}
-	}
-}

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

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:":

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), ":")
+}

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)
+	}
+}