From 3331a40053aa38dbc3d805b13abf5d433826c0ad Mon Sep 17 00:00:00 2001 From: Spencer Brower Date: Wed, 24 Jun 2026 14:05:35 -0400 Subject: [PATCH] refactor: Simplified absolute path resolution code. --- TODOS.md | 2 -- cmd_check.odin | 27 ++++++++++----------------- cmd_remove.odin | 15 ++++----------- cmd_restore.odin | 19 +++++++------------ db.odin | 4 ++++ 5 files changed, 25 insertions(+), 42 deletions(-) diff --git a/TODOS.md b/TODOS.md index 9f4af3b..dec9737 100644 --- a/TODOS.md +++ b/TODOS.md @@ -14,8 +14,6 @@ 7. Make sure official path separators are used when appropriate, rather than '/'. -8. **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. - 10. **config.odin:178** — `search_paths` silently ignores `os.user_home_dir` error. If home is empty, `~` isn't expanded. Same class of bug as issue 3. 12. Consistently ignore allocator errors diff --git a/cmd_check.odin b/cmd_check.odin index a90bf71..5b5d741 100644 --- a/cmd_check.odin +++ b/cmd_check.odin @@ -7,28 +7,21 @@ import "core:path/filepath" // TODO: What happens if you pass a non existent path to cmd_check? // TODO: UX could be improved, so "run envr add ." if file not exists. cmd_check :: proc(cmd: ^Command) { - check_path: string + _check_path: string if len(cmd.args) > 0 { - check_path = cmd.args[0] + _check_path = cmd.args[0] } else { cwd, cwd_err := os.get_working_directory(context.temp_allocator) if cwd_err != nil { fmt.wprintf(cmd.err, "Error getting current directory: %v\n", cwd_err, flush = false) return } - check_path = cwd + _check_path = cwd } - - abs_path: string - if filepath.is_abs(check_path) { - abs_path = check_path - } else { - resolved, abs_err := filepath.abs(check_path) - if abs_err != nil { - fmt.wprintf(cmd.err, "Error getting absolute path: %v\n", abs_err, flush = false) - return - } - abs_path = resolved + check_path, abs_err := filepath.abs(_check_path, context.temp_allocator) + if abs_err != nil { + fmt.wprintf(cmd.err, "Error getting absolute path: %v\n", abs_err, flush = false) + return } db, db_ok := db_open(cmd.config_path) @@ -37,20 +30,20 @@ cmd_check :: proc(cmd: ^Command) { } defer db_close(&db) - is_dir := os.is_directory(abs_path) + is_dir := os.is_directory(check_path) // TODO: set a reasonable default files_in_path := make([dynamic]string, context.temp_allocator) if is_dir { - scanned, scan_ok := scan_path(abs_path, db.cfg) + scanned, scan_ok := scan_path(check_path, db.cfg) if !scan_ok { fmt.wprintln(cmd.err, "Error scanning directory for .env files", flush = false) return } files_in_path = scanned } else { - append(&files_in_path, abs_path) + append(&files_in_path, check_path) } db_files, list_ok := db_list(&db) diff --git a/cmd_remove.odin b/cmd_remove.odin index 7657cde..181adb0 100644 --- a/cmd_remove.odin +++ b/cmd_remove.odin @@ -16,17 +16,10 @@ cmd_remove :: proc(cmd: ^Command) { return } - // TODO: Is this the best way to do it? - abs_path: string - if filepath.is_abs(path) { - abs_path = path - } else { - resolved, abs_err := filepath.abs(path) - if abs_err != nil { - fmt.wprintf(cmd.err, "Error getting absolute path: %v\n", abs_err, flush = false) - return - } - abs_path = resolved + abs_path, abs_err := filepath.abs(path, context.temp_allocator) + if abs_err != nil { + fmt.wprintf(cmd.err, "Error getting absolute path: %v\n", abs_err, flush = false) + return } db, db_ok := db_open(cmd.config_path) diff --git a/cmd_restore.odin b/cmd_restore.odin index 925ae2c..a79a25b 100644 --- a/cmd_restore.odin +++ b/cmd_restore.odin @@ -16,18 +16,11 @@ cmd_restore :: proc(cmd: ^Command) { fmt.wprintln(cmd.err, "Error: No path provided", flush = false) return } + abs_path, abs_err := filepath.abs(path, context.temp_allocator) + if abs_err != nil { + fmt.wprintf(cmd.err, "Error getting absolute path: %v\n", abs_err, flush = false) - // TODO: Is this the right way to handle this? - abs_path: string - if filepath.is_abs(path) { - abs_path = path - } else { - resolved, abs_err := filepath.abs(path) - if abs_err != nil { - fmt.wprintf(cmd.err, "Error getting absolute path: %v\n", abs_err, flush = false) - return - } - abs_path = resolved + return } db, db_ok := db_open(cmd.config_path) @@ -43,13 +36,15 @@ cmd_restore :: proc(cmd: ^Command) { dir := filepath.dir(file.Path) if err := os.mkdir_all(dir); err != nil { - fmt.wprintf(cmd.err, "failed to create directory: %s\n", err) + fmt.wprintf(cmd.err, "Failed to create directory: %v\n", err, flush = false) + return } write_err := os.write_entire_file(file.Path, file.contents) if write_err != nil { fmt.wprintf(cmd.err, "Error writing file: %v\n", write_err, flush = false) + return } diff --git a/db.odin b/db.odin index 184d145..592702e 100644 --- a/db.odin +++ b/db.odin @@ -305,7 +305,11 @@ db_insert :: proc(db: ^Db, file: EnvFile) -> bool { } // Result will be freed when `db_close` is called. +// +// Expects an absolute path db_fetch :: proc(db: ^Db, path: string) -> (EnvFile, bool) { + assert(os.is_absolute_path(path)) + sql: cstring = "SELECT path, remotes, sha256, contents FROM envr_env_files WHERE path = ?" stmt: sqlite.Stmt rc := sqlite.prepare_v2(db.conn, sql, -1, &stmt, nil)