refactor(odin): Fixed AI mistakes.

.tokeignore

@@ -0,0 +1 @@
+**/*_test.{odin,go}

TODOS.md

@@ -0,0 +1,49 @@
+# TODO
+
+Note: These todos can wait until all the subcommands have been ported.
+
+## HIGH
+
+1. [x] **table.odin:74-89** — Hand-rolled JSON output doesn't escape `"`, `\`, newlines. Reimplements `json.marshal` which is already imported in `cmd_list.odin`. Replace with `json.marshal`.
+
+2. **db.odin:380-383, 405, 446** — `sqlite.bind_text` return values overwritten but never checked. A failed bind means `sqlite.step` operates on unbound params.
+
+3. **config.odin:52-54** — `os.user_home_dir` error silently ignored. If it fails, `home` is `""` and all paths become relative (`".envr"` instead of `"~/.envr"`).
+
+## MEDIUM
+
+4. **db.odin:29-35** — `make_temp_path` never calls `strings.builder_destroy`. Leaks builder buffer every call.
+
+5. **db.odin:324-327** — Map iteration (`remote_set`) is non-deterministic. Same file can produce different JSON on each backup, causing spurious DB diffs. Sort remotes before storing.
+
+6. **db.odin:470-473** — `string_to_cstring` allocates via `strings.clone_to_cstring` and never frees. Called dozens of times across db operations.
+
+7. **db.odin:470, 462** — Both `string_to_cstring` and `cstring_to_string` ignore allocation errors. A nil cstring gets passed to SQLite (UB).
+
+8. **db.odin:135, 250** — String interpolation into SQL (`VACUUM INTO '%s'`, `ATTACH DATABASE '%s'`). Currently safe because input is controlled, but fragile.
+
+9. **features.odin:30-41** — `find_binary` uses `strings.join` instead of `filepath.join`, uses `os.stat` instead of checking executability, hardcodes `:` as PATH separator (wrong on Windows).
+
+10. **cmd_restore.odin:20-30 & cmd_remove.odin:19-29** — Identical path-resolution block copy-pasted. `is_abs` guard is redundant since `filepath.abs` is a no-op on absolute paths. Extract a helper.
+
+11. **cmd_restore.odin:44** — `os.mkdir_all` error silently discarded. Subsequent write failure will be confusing.
+
+12. **cmd_edit_config.odin:27** — `$EDITOR` used as single binary name. Breaks for multi-word values like `"code -w"`. Needs `strings.fields()`.
+
+13. [x] **cmd_list.odin:31-35, 58-61** — Uses a `strings.Builder` (never destroyed) for what is just `row.Dir + "/"`. Also `filepath.rel` used where `filepath.base` would suffice since dir is always the parent.
+
+## LOW
+
+14. [x] **db.odin:338-341** — Unnecessary `strings.clone` before `filepath.dir` (which already returns a slice into the input).
+
+15. **db.odin:115** — `json.unmarshal_string` error not checked. Malformed JSON silently produces empty/partial data.
+
+16. **db.odin:352-353** — `hex.encode` error ignored. `string(hex_bytes)` aliases the byte slice.
+
+18. **config.odin:51-60** — `envr_dir` recomputes home dir on every call. Could cache.
+
+19. **main.odin:42-46** — Dynamic array in `fallback_to_go` never deleted. Harmless since process exits.
+
+## REFACTOR
+
+20. **cmd_list.odin** — Non-TTY branch builds `ListEntry` structs and marshals JSON separately. Now that `render_json_rows` (issue 1) accepts an `io.Writer` and uses `json.marshal`, unify both branches to use it. Note: will change JSON keys from `"directory"/"path"` to `"Directory"/"Path"`.

cmd_list.odin

@@ -28,21 +28,11 @@ cmd_list :: proc(cmd: ^Command) {
         table_rows := make([dynamic][]string, 0, len(rows))
 
         for row in rows {
-            b: strings.Builder
-            strings.builder_init(&b)
-            strings.write_string(&b, row.Dir)
-            strings.write_string(&b, "/")
-            dir_str, _ := strings.clone(strings.to_string(b))
-
-            rel, rel_err := filepath.rel(row.Dir, row.Path)
-            if rel_err != nil {
-                fmt.printf("Error getting relative path: %v\n", rel_err)
-                return
-            }
-            cloned_rel, _ := strings.clone(rel)
+            dir_str := strings.concatenate({row.Dir, "/"})
+            filename := filepath.base(row.Path)
             row_slice := make([]string, 2)
             row_slice[0] = dir_str
-            row_slice[1] = cloned_rel
+            row_slice[1] = filename
             append(&table_rows, row_slice)
         }
 
@@ -50,18 +40,10 @@ cmd_list :: proc(cmd: ^Command) {
     } else {
         entries: [dynamic]ListEntry
         for row in rows {
-            rel, rel_err := filepath.rel(row.Dir, row.Path)
-            if rel_err != nil {
-                fmt.printf("Error getting relative path: %v\n", rel_err)
-                return
-            }
-            b: strings.Builder
-            strings.builder_init(&b)
-            strings.write_string(&b, row.Dir)
-            strings.write_string(&b, "/")
+            filename := filepath.base(row.Path)
             append(&entries, ListEntry{
-                Directory = strings.to_string(b),
-                Path = rel,
+                Directory = strings.concatenate({row.Dir, "/"}),
+                Path = filename,
             })
         }
 

cmd_list_test.odin

@@ -0,0 +1,26 @@
+package main
+
+import "core:path/filepath"
+import "core:testing"
+
+@(test)
+test_filepath_base_equals_rel :: proc(t: ^testing.T) {
+	cases := []string{
+		"/home/user/.env",
+		"/home/user/project/.envrc",
+		"/tmp/foo",
+		"/a/b/c/d.txt",
+	}
+
+	for path in cases {
+		dir := filepath.dir(path)
+		rel, rel_err := filepath.rel(dir, path)
+		testing.expect(t, rel_err == nil, "filepath.rel returned an error")
+		base := filepath.base(path)
+		testing.expect(
+			t,
+			rel == base,
+			"filepath.rel(dir, path) should equal filepath.base(path)",
+		)
+	}
+}

db.odin

@@ -338,9 +338,8 @@ new_env_file :: proc(path: string) -> (EnvFile, bool) {
     cloned_path, _ := strings.clone(abs_path)
 
     dir := filepath.dir(cloned_path)
-    cloned_dir, _ := strings.clone(dir)
 
-    remotes := get_git_remotes(cloned_dir)
+    remotes := get_git_remotes(dir)
 
     data, read_err := os.read_entire_file_from_path(cloned_path, context.allocator)
     if read_err != nil {
@@ -354,7 +353,7 @@ new_env_file :: proc(path: string) -> (EnvFile, bool) {
 
     return EnvFile{
         Path = cloned_path,
-        Dir = cloned_dir,
+        Dir = dir,
         Remotes = remotes,
         Sha256 = sha_str,
         contents = string(data),

db_test.odin

@@ -0,0 +1,19 @@
+package main
+
+import "core:path/filepath"
+import "core:strings"
+import "core:testing"
+
+@(test)
+test_dir_slice_owns_parent :: proc(t: ^testing.T) {
+	abs_path := "/home/user/project/.env"
+	cloned_path, _ := strings.clone(abs_path)
+
+	dir := filepath.dir(cloned_path)
+
+	testing.expect(t, dir == "/home/user/project", "filepath.dir should return parent directory")
+	testing.expect(t, len(dir) > 0, "dir should not be empty")
+
+	cloned_dir, _ := strings.clone(dir)
+	testing.expect(t, cloned_dir == dir, "clone of dir should equal dir")
+}

table.odin

@@ -1,11 +1,16 @@
 package main
 
+import "core:encoding/json"
 import "core:fmt"
+import "core:io"
+import "core:os"
 import "core:strings"
 
 render_table :: proc(headers: []string, rows: [][]string) {
 	if !is_tty() {
-		render_json_rows(headers, rows)
+		w := io.to_writer(os.to_writer(os.stdout))
+		render_json_rows(w, headers, rows)
+		io.write_string(w, "\n")
 		return
 	}
 
@@ -71,20 +76,22 @@ render_table :: proc(headers: []string, rows: [][]string) {
 	hline(&b, "\u2514", "\u2534", "\u2518", col_widths)
 }
 
-render_json_rows :: proc(headers: []string, rows: [][]string) {
-	fmt.print("[")
-	for i in 0..<len(rows) {
-		if i > 0 {
-			fmt.print(",")
+render_json_rows :: proc(w: io.Writer, headers: []string, rows: [][]string) {
+	entries := make([dynamic]map[string]string, 0, len(rows))
+	defer delete(entries)
+
+	for row in rows {
+		entry: map[string]string
+		for i in 0..<len(headers) {
+			entry[headers[i]] = row[i]
 		}
-		fmt.print("{")
-		for j in 0..<len(headers) {
-			if j > 0 {
-				fmt.print(",")
-			}
-			fmt.printf("\"%s\":\"%s\"", headers[j], rows[i][j])
-		}
-		fmt.print("}")
+		append(&entries, entry)
 	}
-	fmt.println("]")
+
+	data, err := json.marshal(entries[:])
+	if err != nil {
+		fmt.eprintf("Error marshaling JSON: %v\n", err)
+		return
+	}
+	io.write_string(w, string(data))
 }

table_test.odin

@@ -0,0 +1,101 @@
+package main
+
+import "core:encoding/json"
+import "core:fmt"
+import "core:io"
+import "core:strings"
+import "core:testing"
+
+@(test)
+test_render_json_rows_normal :: proc(t: ^testing.T) {
+	b: strings.Builder
+	strings.builder_init(&b)
+	defer strings.builder_destroy(&b)
+
+	headers := []string{"name", "path"}
+	rows := [][]string{{"foo", "/home/user/.env"}, {"bar", "/home/user/project/.env"}}
+
+	w := strings.to_writer(&b)
+	render_json_rows(w, headers, rows)
+
+	output := strings.to_string(b)
+
+	result: []map[string]string
+	unmarshal_err := json.unmarshal_string(output, &result)
+	testing.expect(
+		t,
+		unmarshal_err == nil,
+		fmt.aprintf("json unmarshal failed: %v\noutput was: %q", unmarshal_err, output),
+	)
+	testing.expect(t, len(result) == 2, fmt.aprintf("expected 2 rows, got %d", len(result)))
+	testing.expect(
+		t,
+		result[0]["name"] == "foo",
+		fmt.aprintf("expected name=foo, got %q", result[0]["name"]),
+	)
+	testing.expect(t, result[0]["path"] == "/home/user/.env")
+	testing.expect(t, result[1]["name"] == "bar")
+	testing.expect(t, result[1]["path"] == "/home/user/project/.env")
+}
+
+@(test)
+test_render_json_rows_special_chars :: proc(t: ^testing.T) {
+	b: strings.Builder
+	strings.builder_init(&b)
+	defer strings.builder_destroy(&b)
+
+	headers := []string{"key", "value"}
+	rows := [][]string {
+		{"quote", `has "double quotes"`},
+		{"backslash", `path\to\file`},
+		{"newline", "line1\nline2"},
+		{"mixed", `a "b" c\nd`},
+	}
+
+	w := strings.to_writer(&b)
+	render_json_rows(w, headers, rows)
+
+	output := strings.to_string(b)
+
+	result: []map[string]string
+	unmarshal_err := json.unmarshal(transmute([]byte)output, &result)
+	testing.expect(
+		t,
+		unmarshal_err == nil,
+		fmt.aprintf("json unmarshal failed: %v\noutput was: %q", unmarshal_err, output),
+	)
+	testing.expect(t, len(result) == 4)
+	testing.expect(
+		t,
+		result[0]["value"] == `has "double quotes"`,
+		fmt.aprintf("got %q", result[0]["value"]),
+	)
+	testing.expect(t, result[1]["value"] == `path\to\file`)
+	testing.expect(t, result[2]["value"] == "line1\nline2")
+	testing.expect(t, result[3]["value"] == `a "b" c\nd`)
+}
+
+@(test)
+test_render_json_rows_empty :: proc(t: ^testing.T) {
+	b: strings.Builder
+	strings.builder_init(&b)
+	defer strings.builder_destroy(&b)
+
+	headers := []string{"name"}
+	rows: [][]string
+
+	w := strings.to_writer(&b)
+	render_json_rows(w, headers, rows)
+
+	output := strings.to_string(b)
+
+	result: []map[string]string
+	unmarshal_err := json.unmarshal_string(output, &result)
+	testing.expect(
+		t,
+		unmarshal_err == nil,
+		fmt.aprintf("json unmarshal failed: %v\noutput was: %q", unmarshal_err, output),
+	)
+	testing.expect(t, len(result) == 0)
+}
+