Fix issue #1537 "filepath.split_list requires a trailing separator"

Does `make()` with `count + 1` and appends final component (note a
  trailing separator will now result in an empty final component)
Adds test "tests/core/path/filepath/test_core_filepath.odin"

4 files changed, 140 insertions, 3 deletions

diff --git a/core/path/filepath/path.odin b/core/path/filepath/path.odin
index ba6e11044..c04bd5a11 100644
--- a/core/path/filepath/path.odin
+++ b/core/path/filepath/path.odin
@@ -1,5 +1,5 @@
 // The path/filepath package uses either forward slashes or backslashes depending on the operating system
-// To process paths usch as URLs that depend on forward slashes regardless of the OS, use the path package
+// To process paths such as URLs that depend on forward slashes regardless of the OS, use the path package
 package filepath
 
 import "core:strings"
@@ -300,6 +300,11 @@ dir :: proc(path: string, allocator := context.allocator) -> string {
 
 
 
+// Splits the PATH-like `path` string, returning an array of its separated components (delete after use).
+// For Windows the separator is `;`, for Unix it's  `:`.
+// An empty string returns nil. A non-empty string with no separators returns a 1-element array.
+// Any empty components will be included, e.g. `a::b` will return a 3-element array, as will `::`.
+// Separators within pairs of double-quotes will be ignored and stripped, e.g. `"a:b"c:d` will return []{`a:bc`, `d`}.
 split_list :: proc(path: string, allocator := context.allocator) -> []string {
 	if path == "" {
 		return nil
@@ -322,7 +327,7 @@ split_list :: proc(path: string, allocator := context.allocator) -> []string {
 	}
 
 	start, quote = 0, false
-	list := make([]string, count, allocator)
+	list := make([]string, count + 1, allocator)
 	index := 0
 	for i := 0; i < len(path); i += 1 {
 		c := path[i]
@@ -336,6 +341,7 @@ split_list :: proc(path: string, allocator := context.allocator) -> []string {
 		}
 	}
 	assert(index == count)
+	list[index] = path[start:]
 
 	for s0, i in list {
 		s, new := strings.replace_all(s0, `"`, ``, allocator)
diff --git a/tests/core/Makefile b/tests/core/Makefile
index a990c5833..4160690f9 100644
--- a/tests/core/Makefile
+++ b/tests/core/Makefile
@@ -2,7 +2,7 @@ ODIN=../../odin
 PYTHON=$(shell which python3)
 
 all: download_test_assets image_test compress_test strings_test hash_test crypto_test noise_test encoding_test \
-	 math_test linalg_glsl_math_test reflect_test
+	 math_test linalg_glsl_math_test filepath_test reflect_test
 
 download_test_assets:
 	$(PYTHON) download_assets.py
@@ -36,5 +36,8 @@ math_test:
 linalg_glsl_math_test:
 	$(ODIN) run math/linalg/glsl/test_linalg_glsl_math.odin -out=test_linalg_glsl_math -collection:tests=..
 
+filepath_test:
+	$(ODIN) run path/filepath/test_core_filepath.odin -out=test_core_filepath -collection:tests=..
+
 reflect_test:
 	$(ODIN) run reflect/test_core_reflect.odin -out=test_core_reflect -collection:tests=..
diff --git a/tests/core/build.bat b/tests/core/build.bat
index c94aac10a..2f9ba672e 100644
--- a/tests/core/build.bat
+++ b/tests/core/build.bat
@@ -55,6 +55,11 @@ echo ---
 %PATH_TO_ODIN% run math/linalg/glsl %COMMON%

 

 echo ---

+echo Running core:path/filepath tests

+echo ---

+%PATH_TO_ODIN% run path/filepath %COMMON%

+

+echo ---

 echo Running core:reflect tests

 echo ---

 %PATH_TO_ODIN% run reflect %COMMON%

diff --git a/tests/core/path/filepath/test_core_filepath.odin b/tests/core/path/filepath/test_core_filepath.odin
new file mode 100644
index 000000000..0268fb62c
--- /dev/null
+++ b/tests/core/path/filepath/test_core_filepath.odin
@@ -0,0 +1,123 @@
+// Tests "path.odin" in "core:path/filepath".
+// Must be run with `-collection:tests=` flag, e.g.
+// ./odin run tests/core/path/filepath/test_core_filepath.odin -collection:tests=tests
+package test_core_filepath
+
+import "core:fmt"
+import "core:path/filepath"
+import "core:testing"
+import tc "tests:common"
+
+main :: proc() {
+	t := testing.T{}
+
+	when ODIN_OS == .Windows {
+		test_split_list_windows(&t)
+	} else {
+		test_split_list_unix(&t)
+	}
+
+	tc.report(&t)
+}
+
+@test
+test_split_list_windows :: proc(t: ^testing.T) {
+
+	using filepath
+
+	Datum :: struct {
+		i: int,
+		v: string,
+		e: [3]string,
+	}
+	@static data := []Datum{
+		{ 0, "C:\\Odin;C:\\Visual Studio;\"C:\\Some Other\"",
+			[3]string{"C:\\Odin", "C:\\Visual Studio", "C:\\Some Other"} }, // Issue #1537
+		{ 1, "a;;b", [3]string{"a", "", "b"} },
+		{ 2, "a;b;", [3]string{"a", "b", ""} },
+		{ 3, ";a;b", [3]string{"", "a", "b"} },
+		{ 4, ";;", [3]string{"", "", ""} },
+		{ 5, "\"a;b\"c;d;\"f\"", [3]string{"a;bc", "d", "f"} },
+		{ 6, "\"a;b;c\";d\";e\";f", [3]string{"a;b;c", "d;e", "f"} },
+	}
+
+	for d, i in data {
+		assert(i == d.i, fmt.tprintf("wrong data index: i %d != d.i %d\n", i, d.i))
+		r := split_list(d.v)
+		defer delete(r)
+		tc.expect(t, len(r) == len(d.e), fmt.tprintf("i:%d %s(%s) len(r) %d != len(d.e) %d",
+													 i, #procedure, d.v, len(r), len(d.e)))
+		if len(r) == len(d.e) {
+			for _, j in r {
+				tc.expect(t, r[j] == d.e[j], fmt.tprintf("i:%d %s(%v) -> %v[%d] != %v",
+														 i, #procedure, d.v, r[j], j, d.e[j]))
+			}
+		}
+	}
+
+	{
+		v := ""
+		r := split_list(v)
+		tc.expect(t, r == nil, fmt.tprintf("%s(%s) -> %v != nil", #procedure, v, r))
+	}
+	{
+		v := "a"
+		r := split_list(v)
+		defer delete(r)
+		tc.expect(t, len(r) == 1, fmt.tprintf("%s(%s) len(r) %d != 1", #procedure, v, len(r)))
+		if len(r) == 1 {
+			tc.expect(t, r[0] == "a", fmt.tprintf("%s(%v) -> %v[0] != a", #procedure, v, r[0]))
+		}
+	}
+}
+
+@test
+test_split_list_unix :: proc(t: ^testing.T) {
+
+	using filepath
+
+	Datum :: struct {
+		i: int,
+		v: string,
+		e: [3]string,
+	}
+	@static data := []Datum{
+		{ 0, "/opt/butler:/home/fancykillerpanda/Projects/Odin/Odin:/usr/local/sbin",
+			[3]string{"/opt/butler", "/home/fancykillerpanda/Projects/Odin/Odin", "/usr/local/sbin"} }, // Issue #1537
+		{ 1, "a::b", [3]string{"a", "", "b"} },
+		{ 2, "a:b:", [3]string{"a", "b", ""} },
+		{ 3, ":a:b", [3]string{"", "a", "b"} },
+		{ 4, "::", [3]string{"", "", ""} },
+		{ 5, "\"a:b\"c:d:\"f\"", [3]string{"a:bc", "d", "f"} },
+		{ 6, "\"a:b:c\":d\":e\":f", [3]string{"a:b:c", "d:e", "f"} },
+	}
+
+	for d, i in data {
+		assert(i == d.i, fmt.tprintf("wrong data index: i %d != d.i %d\n", i, d.i))
+		r := split_list(d.v)
+		defer delete(r)
+		tc.expect(t, len(r) == len(d.e), fmt.tprintf("i:%d %s(%s) len(r) %d != len(d.e) %d",
+													 i, #procedure, d.v, len(r), len(d.e)))
+		if len(r) == len(d.e) {
+			for _, j in r {
+				tc.expect(t, r[j] == d.e[j], fmt.tprintf("i:%d %s(%v) -> %v[%d] != %v",
+														 i, #procedure, d.v, r[j], j, d.e[j]))
+			}
+		}
+	}
+
+	{
+		v := ""
+		r := split_list(v)
+		tc.expect(t, r == nil, fmt.tprintf("%s(%s) -> %v != nil", #procedure, v, r))
+	}
+	{
+		v := "a"
+		r := split_list(v)
+		defer delete(r)
+		tc.expect(t, len(r) == 1, fmt.tprintf("%s(%s) len(r) %d != 1", #procedure, v, len(r)))
+		if len(r) == 1 {
+			tc.expect(t, r[0] == "a", fmt.tprintf("%s(%v) -> %v[0] != a", #procedure, v, r[0]))
+		}
+	}
+}