perf: remotes are now stored as a newline delimited list.

Previously they were saved as json.

TODOS.md

@@ -1,6 +1,6 @@
 # TODOs
 
-1. Commands are still leaking.
+1. Commands are still leaking. (Do 15. first)
 
 2. Add color flag and support non colored output.
 
@@ -10,33 +10,31 @@
 
 5. Json may be an expensive encoding for remotes. Confirm with spall, and use null terminated strings if necessary.
 
-6. Make sure official path separators are used when appropriate, rather than '/'.
+6. Consistently ignore allocator errors
 
-7. Consistently ignore allocator errors
+7. Check for prealloc opportunities. i.e. `make([dynamic]string)` -> `make([dynamic]string, 5)`.
 
-8. Check for prealloc opportunities. i.e. `make([dynamic]string)` -> `make([dynamic]string, 5)`.
+8. Add a text filter to the multi_select.
 
-9. Add a text filter to the multi_select.
+9. Add tests for untested commands.
 
-10. Add tests for untested commands.
+10. add --format -f flag to commands that draw tables.
 
-11. add --format -f flag to commands that draw tables.
+11. Replace `testing.expect` calls with `testing.expect_value` calls where appropriate.
 
-12. Replace `testing.expect` calls with `testing.expect_value` calls where appropriate.
+12. procedures should be ordered by use, main at the top, then in the order they are called from main.
 
-13. procedures should be ordered by use, main at the top, then in the order they are called from main.
+13. Shell completion
 
-14. Shell completion
+14. Bring back windows support / cross-compilation.
 
-15. Bring back windows support / cross-compilation.
+15. Test all cmds / terminal branches.
 
-16. Test all cmds / terminal branches.
+16. Fix error messages to use fmt.eprintf (stderr) instead of fmt.printf (stdout)
 
-17. Fix error messages to use fmt.eprintf (stderr) instead of fmt.printf (stdout)
+17. Pass allocator to findr?
 
-18. Pass allocator to findr?
-
-19. Update `read_wire_string` to use a slice.
+18. Update `read_wire_string` to use a slice.
 
 ## Double-check AI output
 

cmd_list.odin

@@ -57,12 +57,16 @@ cmd_list :: proc(cmd: ^Command) {
 			append(
 				&entries,
 				ListEntry {
-					dir = strings.concatenate({row.dir, "/"}, context.temp_allocator),
+					dir = strings.concatenate(
+						{row.dir, os.Path_Separator_String},
+						context.temp_allocator,
+					),
 					path = filename,
 				},
 			)
 		}
 
+
 		data, marshal_err := json.marshal(entries[:], allocator = context.temp_allocator)
 		if marshal_err != nil {
 			fmt.wprintf(cmd.err, "Error marshaling JSON: %v\n", marshal_err, flush = false)

db.odin

@@ -222,6 +222,7 @@ db_list :: proc(db: ^Db) -> ([]EnvFile, bool) {
 	allocator := db_allocator(db)
 	results := make([dynamic]EnvFile, 0, 10, allocator)
 
+	migrate := false
 	for {
 		rc = sqlite.step(stmt)
 		if rc == sqlite.DONE {
@@ -232,12 +233,22 @@ db_list :: proc(db: ^Db) -> ([]EnvFile, bool) {
 			#no_bounds_check return results[:], false
 		}
 
-		remotes_json := string(sqlite.column_text(stmt, 1))
+		// TODO: Remove json support after next major release
 		remotes: [dynamic]string = ---
-		if len(remotes_json) > 0 {
-			err := json.unmarshal_string(remotes_json, &remotes, allocator = allocator)
-			if err != nil {
-				fmt.eprintf("Warning: malformed remotes JSON: %v\n", err)
+		remotes_raw := string(sqlite.column_text(stmt, 1))
+		if len(remotes_raw) > 0 {
+			if remotes_raw[0] == '[' {
+				err := json.unmarshal_string(remotes_raw, &remotes, allocator = allocator)
+				if err != nil {
+					fmt.eprintf("Warning: malformed remotes JSON: %v\n", err)
+				}
+				migrate = true
+			} else {
+				split := strings.split_lines(remotes_raw, context.temp_allocator)
+				remotes = make([dynamic]string, 0, len(split), allocator = allocator)
+				for s in split {
+					append(&remotes, strings.clone(s, allocator))
+				}
 			}
 		}
 		path := clone_cstring(sqlite.column_text(stmt, 0), allocator)
@@ -254,16 +265,16 @@ db_list :: proc(db: ^Db) -> ([]EnvFile, bool) {
 		)
 	}
 
+	if migrate {
+		migrate_remotes(db)
+	}
+
 	#no_bounds_check return results[:], true
 }
 
 // TODO: Should we use context.temp_allocator for proc scoped lifetimes?
 db_insert :: proc(db: ^Db, file: EnvFile) -> bool {
-	remotes_json, marshal_err := json.marshal(file.remotes, allocator = context.temp_allocator)
-	if marshal_err != nil {
-		fmt.printf("Error marshaling remotes: %v\n", marshal_err)
-		return false
-	}
+	remotes := strings.join(file.remotes[:], "\n", allocator = context.temp_allocator)
 
 	sql: cstring =
 		"INSERT OR REPLACE INTO " +
@@ -285,7 +296,7 @@ db_insert :: proc(db: ^Db, file: EnvFile) -> bool {
 		return false
 	}
 
-	cremotes := to_cstring(string(remotes_json))
+	cremotes := to_cstring(remotes)
 	defer delete(cremotes)
 	rc = sqlite.bind_text(stmt, 2, cremotes, -1, nil)
 	if rc != sqlite.OK {
@@ -353,17 +364,33 @@ db_fetch :: proc(db: ^Db, path: string) -> (EnvFile, bool) {
 		return EnvFile{}, false
 	}
 
-	remotes_json := string(sqlite.column_text(stmt, 1))
+	// TODO: Remove json support after next major release
+	migrate := false
 	remotes: [dynamic]string = ---
-	if len(remotes_json) > 0 {
-		err := json.unmarshal_string(remotes_json, &remotes, allocator = allocator)
-		if err != nil {
-			fmt.eprintf("Warning: malformed remotes JSON: %v\n", err)
+	remotes_raw := string(sqlite.column_text(stmt, 1))
+	if len(remotes_raw) > 0 {
+		if remotes_raw[0] == '[' {
+			err := json.unmarshal_string(remotes_raw, &remotes, allocator = allocator)
+			if err != nil {
+				fmt.eprintf("Warning: malformed remotes JSON: %v\n", err)
+			}
+
+			migrate = true
+		} else {
+			split := strings.split_lines(remotes_raw, context.temp_allocator)
+			remotes = make([dynamic]string, 0, len(split), allocator = allocator)
+			for s in split {
+				append(&remotes, strings.clone(s, allocator))
+			}
 		}
 	}
 
 	file_path := clone_cstring(sqlite.column_text(stmt, 0), allocator)
 
+	if migrate {
+		migrate_remotes(db)
+	}
+
 	return EnvFile {
 			path = file_path,
 			dir = filepath.dir(file_path),
@@ -498,6 +525,27 @@ db_persist :: proc(db: ^Db, f: ^EnvFile, old_path: string) -> bool {
 	return db_insert(db, f^)
 }
 
+// TODO: Remove after the next major release
+migrate_remotes :: proc(db: ^Db) {
+	sql ::
+		"UPDATE envr_env_files " +
+		"SET remotes = COALESCE((" +
+		"  SELECT group_concat(atom, char(10)) " +
+		"  FROM json_each(envr_env_files.remotes)" +
+		"), '') " +
+		"WHERE remotes LIKE '[%'"
+
+	rc := sqlite.exec(db.conn, sql, nil, nil, nil)
+	if rc != sqlite.OK {
+		fmt.eprintf("Warning: failed to migrate remotes: %s\n", sqlite.errmsg(db.conn))
+		return
+	}
+
+	if sqlite.changes(db.conn) > 0 {
+		db.changed = true
+	}
+}
+
 try_move_dir :: proc(db: ^Db, f: ^EnvFile, allocator: mem.Allocator) -> (bool, SyncError) {
 	roots, ok := find_git_roots(db.cfg)
 	if !ok {

findr/findr_test.odin

@@ -21,9 +21,7 @@ test_basic_gitignored :: proc(t: ^testing.T) {
 	create_file(env, "repo/secrets.env")
 	create_file(env, "repo/normal.txt")
 
-	assert_output(t, env, nil, {}, {
-		"repo/.env", "repo/secrets.env",
-	})
+	assert_output(t, env, nil, {}, {"repo/.env", "repo/secrets.env"})
 }
 
 @(test)
@@ -49,9 +47,7 @@ test_negation_pattern :: proc(t: ^testing.T) {
 	create_file(env, "repo/secrets.env")
 	create_file(env, "repo/prod.env")
 
-	assert_output(t, env, nil, {}, {
-		"repo/.env", "repo/secrets.env",
-	})
+	assert_output(t, env, nil, {}, {"repo/.env", "repo/secrets.env"})
 }
 
 @(test)
@@ -67,9 +63,7 @@ test_multiple_repos :: proc(t: ^testing.T) {
 	create_file(env, "repo2/.gitignore", "*.key\n")
 	create_file(env, "repo2/secret.key")
 
-	assert_output(t, env, nil, {}, {
-		"repo1/a.env", "repo2/secret.key",
-	})
+	assert_output(t, env, nil, {}, {"repo1/a.env", "repo2/secret.key"})
 }
 
 @(test)
@@ -85,9 +79,7 @@ test_nested_repos :: proc(t: ^testing.T) {
 	create_file(env, "parent/child/.gitignore", "*.key\n")
 	create_file(env, "parent/child/api.key")
 
-	assert_output(t, env, nil, {}, {
-		"parent/top.env", "parent/child/api.key",
-	})
+	assert_output(t, env, nil, {}, {"parent/top.env", "parent/child/api.key"})
 }
 
 @(test)
@@ -102,9 +94,7 @@ test_nested_gitignore_read :: proc(t: ^testing.T) {
 	create_file(env, "repo/sub/secret.txt")
 	create_file(env, "repo/sub/.env")
 
-	assert_output(t, env, nil, {}, {
-		"repo/sub/secret.txt", "repo/sub/.env",
-	})
+	assert_output(t, env, nil, {}, {"repo/sub/secret.txt", "repo/sub/.env"})
 }
 
 @(test)
@@ -119,9 +109,7 @@ test_nested_gitignore_negation :: proc(t: ^testing.T) {
 	create_file(env, "repo/sub/important.log")
 	create_file(env, "repo/sub/debug.log")
 
-	assert_output(t, env, nil, {}, {
-		"repo/sub/debug.log",
-	})
+	assert_output(t, env, nil, {}, {"repo/sub/debug.log"})
 }
 
 @(test)
@@ -136,9 +124,7 @@ test_multisegment_pattern :: proc(t: ^testing.T) {
 	create_file(env, "repo/build/other.txt")
 	create_file(env, "repo/output.txt")
 
-	assert_output(t, env, nil, {}, {
-		"repo/build/output.txt",
-	})
+	assert_output(t, env, nil, {}, {"repo/build/output.txt"})
 }
 
 @(test)
@@ -200,7 +186,7 @@ test_multiple_search_dirs :: proc(t: ^testing.T) {
 		stripped := r
 		if strings.has_prefix(stripped, env.temp_dir) {
 			stripped = stripped[len(env.temp_dir):]
-			if len(stripped) > 0 && stripped[0] == '/' {
+			if len(stripped) > 0 && stripped[0] == os.Path_Separator {
 				stripped = stripped[1:]
 			}
 		}
@@ -234,9 +220,7 @@ test_ignored_dir_descended :: proc(t: ^testing.T) {
 	create_file(env, "repo/secrets/api.key")
 
 	// Ignored dir's contents are emitted AND descended into
-	assert_output(t, env, nil, {}, {
-		"repo/secrets/", "repo/secrets/.env", "repo/secrets/api.key",
-	})
+	assert_output(t, env, nil, {}, {"repo/secrets/", "repo/secrets/.env", "repo/secrets/api.key"})
 }
 
 @(test)
@@ -251,10 +235,13 @@ test_nested_ignored_dir :: proc(t: ^testing.T) {
 	create_file(env, "repo/build/output.txt")
 	create_file(env, "repo/build/sub/deep.env")
 
-	assert_output(t, env, nil, {}, {
-		"repo/build/", "repo/build/output.txt",
-		"repo/build/sub/", "repo/build/sub/deep.env",
-	})
+	assert_output(
+		t,
+		env,
+		nil,
+		{},
+		{"repo/build/", "repo/build/output.txt", "repo/build/sub/", "repo/build/sub/deep.env"},
+	)
 }
 
 // ============================================================================
@@ -272,10 +259,7 @@ test_excludes_prune_dirs :: proc(t: ^testing.T) {
 	create_dir(env, "repo/vendor")
 	create_file(env, "repo/vendor/lib.env")
 
-	assert_output(t, env, nil,
-		{excludes = {"vendor"}},
-		{"repo/.env"},
-	)
+	assert_output(t, env, nil, {excludes = {"vendor"}}, {"repo/.env"})
 }
 
 @(test)
@@ -289,10 +273,7 @@ test_pattern_filters_results :: proc(t: ^testing.T) {
 	create_file(env, "repo/secrets.env")
 	create_file(env, "repo/master.key")
 
-	assert_output(t, env, nil,
-		{pattern = "\\.env$"},
-		{"repo/.env", "repo/secrets.env"},
-	)
+	assert_output(t, env, nil, {pattern = "\\.env$"}, {"repo/.env", "repo/secrets.env"})
 }
 
 // ============================================================================
@@ -313,8 +294,6 @@ test_fifo_emitted :: proc(t: ^testing.T) {
 	defer delete(cpath)
 	linux.mknod(cpath, linux.S_IFIFO | linux.Mode{.IRUSR, .IWUSR}, 0)
 
-	assert_output(t, env, nil,
-		{pattern = "\\.fifo$"},
-		{"repo/test.fifo"},
-	)
+	assert_output(t, env, nil, {pattern = "\\.fifo$"}, {"repo/test.fifo"})
 }
+

findr/test_env.odin

@@ -37,7 +37,7 @@ create_file :: proc(env: TestEnv, path: string, content: string = "") {
 	full := join_path(env.temp_dir, path)
 	defer delete(full)
 
-	dir_end := strings.last_index(full, "/")
+	dir_end := strings.last_index(full, os.Path_Separator_String)
 	if dir_end >= 0 {
 		dir_path := full[:dir_end]
 		os.mkdir_all(dir_path, os.Permissions_Default_Directory)
@@ -105,12 +105,7 @@ assert_output :: proc(
 	}
 }
 
-assert_output_empty :: proc(
-	t: ^testing.T,
-	env: TestEnv,
-	args: []string,
-	opts: WalkOptions,
-) {
+assert_output_empty :: proc(t: ^testing.T, env: TestEnv, args: []string, opts: WalkOptions) {
 	results := collect_results(env, args, opts)
 	defer {
 		for r in results {delete(r)}
@@ -139,7 +134,7 @@ collect_results :: proc(env: TestEnv, args: []string, opts: WalkOptions) -> [dyn
 		r := results[i]
 		if strings.has_prefix(r, env.temp_dir) {
 			stripped := r[len(env.temp_dir):]
-			if len(stripped) > 0 && stripped[0] == '/' {
+			if len(stripped) > 0 && stripped[0] == os.Path_Separator {
 				stripped = stripped[1:]
 			}
 			new_r, _ := strings.clone(stripped)
@@ -150,3 +145,4 @@ collect_results :: proc(env: TestEnv, args: []string, opts: WalkOptions) -> [dyn
 
 	return results
 }
+

findr/walker.odin

@@ -189,7 +189,7 @@ flush_buf :: proc(ch: chan.Chan([]u8), local: ^[dynamic]u8) {
 }
 
 append_path :: proc(buf: ^[dynamic]u8, parent, name: string, trailing_slash: bool) {
-	need_sep := len(parent) > 0 && parent[len(parent) - 1] != '/'
+	need_sep := len(parent) > 0 && parent[len(parent) - 1] != os.Path_Separator
 	size := len(parent) + len(name) + 1
 	if need_sep do size += 1
 	if trailing_slash do size += 1
@@ -200,9 +200,9 @@ append_path :: proc(buf: ^[dynamic]u8, parent, name: string, trailing_slash: boo
 
 	pos := old_len
 	pos += copy(buf[pos:], parent)
-	if need_sep {buf[pos] = '/'; pos += 1}
+	if need_sep {buf[pos] = os.Path_Separator; pos += 1}
 	pos += copy(buf[pos:], name)
-	if trailing_slash {buf[pos] = '/'; pos += 1}
+	if trailing_slash {buf[pos] = os.Path_Separator; pos += 1}
 	buf[pos] = '\n'
 }
 
@@ -362,6 +362,7 @@ check_chain :: proc(ctx: ^GIContext, entry_rel: string, is_dir: bool) -> bool {
 	return false
 }
 
+// TODO: Is this a copy of something in the core packages?
 relative_to :: proc(entry_rel, base_rel: string) -> string {
 	if len(base_rel) == 0 do return entry_rel
 	prefix_len := len(base_rel)
@@ -442,14 +443,15 @@ load_ignore_patterns :: proc(dir_path: string, in_repo: bool) -> ^Gitignore {
 	return gi
 }
 
+// TODO: Is this a copy of core package behavior?
 join_path :: proc(parent, child: string) -> string {
-	need_sep := len(parent) == 0 || parent[len(parent) - 1] != '/'
+	need_sep := len(parent) == 0 || parent[len(parent) - 1] != os.Path_Separator
 	total := len(parent) + len(child)
 	if need_sep do total += 1
 	buf := make([]u8, total, context.allocator)
 	pos := copy(buf, parent)
 	if need_sep {
-		buf[pos] = '/'
+		buf[pos] = os.Path_Separator
 		pos += 1
 	}
 	copy(buf[pos:], child)