commands: simplify CompletePath

This function is overly complex, has code duplication and is not
preserving user input (converting all paths to absolute paths).

Simplify it and avoid converting relative paths to absolute ones.

Add unit tests to ensure it works as expected.

Signed-off-by: Robin Jarry <robin@jarry.cc>
Tested-by: Bence Ferdinandy <bence@ferdinandy.com>

7 files changed, 81 insertions, 50 deletions

diff --git a/commands/testdata/.hidden/foo.conf b/commands/testdata/.hidden/foo.conf
new file mode 100644
index 00000000..190a1803
--- /dev/null
+++ b/commands/testdata/.hidden/foo.conf
@@ -0,0 +1 @@
+123
diff --git a/commands/testdata/.keep-me b/commands/testdata/.keep-me
new file mode 100644
index 00000000..b35a57ea
--- /dev/null
+++ b/commands/testdata/.keep-me
@@ -0,0 +1 @@
+This file is used by unit tests
diff --git a/commands/testdata/baz/unused.txt b/commands/testdata/baz/unused.txt
new file mode 100644
index 00000000..573541ac
--- /dev/null
+++ b/commands/testdata/baz/unused.txt
@@ -0,0 +1 @@
+0
diff --git a/commands/testdata/foo.ini b/commands/testdata/foo.ini
new file mode 100644
index 00000000..9d632e56
--- /dev/null
+++ b/commands/testdata/foo.ini
@@ -0,0 +1 @@
+# x
diff --git a/commands/testdata/foo/bar.json b/commands/testdata/foo/bar.json
new file mode 100644
index 00000000..0967ef42
--- /dev/null
+++ b/commands/testdata/foo/bar.json
@@ -0,0 +1 @@
+{}
diff --git a/commands/util.go b/commands/util.go
index 6247f579..e7940017 100644
--- a/commands/util.go
+++ b/commands/util.go
@@ -72,65 +72,37 @@ func QuickTerm(args []string, stdin io.Reader) (*app.Terminal, error) {
 
 // CompletePath provides filesystem completions given a starting path.
 func CompletePath(path string) []string {
-	if path == "" {
-		// default to cwd
-		cwd, err := os.Getwd()
-		if err != nil {
-			return nil
-		}
-		path = cwd
+	if path == ".." || strings.HasSuffix(path, "/..") {
+		return []string{path + "/"}
+	}
+	if path == "~" || strings.HasPrefix(path, "~/") {
+		path = xdg.HomeDir() + strings.TrimPrefix(path, "~")
+	}
+	includeHidden := path == "."
+	if i := strings.LastIndex(path, "/"); i != -1 && i < len(path)-1 {
+		includeHidden = strings.HasPrefix(path[i+1:], ".")
 	}
 
-	// strip trailing slashes, etc.
-	path = filepath.Clean(xdg.ExpandHome(path))
-
-	if _, err := os.Stat(path); os.IsNotExist(err) {
-		// if the path doesn't exist, it is likely due to it being a partial path
-		// in this case, we want to return possible matches (ie /hom* should match
-		// /home)
-		matches, err := filepath.Glob(fmt.Sprintf("%s*", path))
-		if err != nil {
-			return nil
-		}
-
-		if !strings.HasPrefix(path, ".") && !strings.Contains(path, "/.") {
-			log.Tracef("removing hidden files from glob results")
-			for i := len(matches) - 1; i >= 0; i-- {
-				if strings.HasPrefix(filepath.Base(matches[i]), ".") {
-					if i == len(matches)-1 {
-						matches = matches[:i]
-						continue
-					}
-					matches = append(matches[:i], matches[i+1:]...)
-				}
-			}
-		}
-
-		for i, m := range matches {
-			if isDir(m) {
-				m += "/"
-			}
-			matches[i] = opt.QuoteArg((xdg.TildeHome(m)))
-		}
-
-		sort.Strings(matches)
-
-		return matches
+	matches, err := filepath.Glob(path + "*")
+	if err != nil || matches == nil {
+		return nil
 	}
 
-	files := listDir(path, false)
+	results := make([]string, 0, len(matches))
 
-	for i, f := range files {
-		f = filepath.Join(path, f)
-		if isDir(f) {
-			f += "/"
+	for _, m := range matches {
+		if isDir(m) {
+			m += "/"
+		}
+		if strings.HasPrefix(filepath.Base(m), ".") && !includeHidden {
+			continue
 		}
-		files[i] = opt.QuoteArg((xdg.TildeHome(f)))
+		results = append(results, opt.QuoteArg(xdg.TildeHome(m)))
 	}
 
-	sort.Strings(files)
+	sort.Strings(results)
 
-	return files
+	return results
 }
 
 func isDir(path string) bool {
diff --git a/commands/util_test.go b/commands/util_test.go
new file mode 100644
index 00000000..80113fd5
--- /dev/null
+++ b/commands/util_test.go
@@ -0,0 +1,54 @@
+package commands_test
+
+import (
+	"os"
+	"testing"
+
+	"git.sr.ht/~rjarry/aerc/commands"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCompletePath(t *testing.T) {
+	os.Chdir("testdata")
+	defer os.Chdir("..")
+
+	vectors := []struct {
+		arg      string
+		expected []string
+	}{
+		{
+			arg:      "",
+			expected: []string{"baz/", "foo.ini", "foo/"},
+		},
+		{
+			arg:      ".",
+			expected: []string{".hidden/", ".keep-me"},
+		},
+		{
+			arg:      "fo",
+			expected: []string{"foo.ini", "foo/"},
+		},
+		{
+			arg:      "..",
+			expected: []string{"../"},
+		},
+		{
+			arg:      "../..",
+			expected: []string{"../../"},
+		},
+		{
+			arg: "../testdata/",
+			expected: []string{
+				"../testdata/baz/",
+				"../testdata/foo.ini",
+				"../testdata/foo/",
+			},
+		},
+	}
+	for _, vec := range vectors {
+		t.Run(vec.arg, func(t *testing.T) {
+			res := commands.CompletePath(vec.arg)
+			assert.Equal(t, vec.expected, res)
+		})
+	}
+}