From 22a517340a6d0c773b18bd74c1329051e84c4c8b Mon Sep 17 00:00:00 2001 From: Spencer Brower Date: Fri, 12 Jun 2026 10:32:20 -0400 Subject: [PATCH] refactor(odin): Added proper help text to all commands. --- TODOS.md | 18 +++++ cli.odin | 181 ++++++++++++++++++++++++++++++++--------------- cli_test.odin | 130 ++++++++++++++++++++++++++++++++++ cmd_backup.odin | 2 +- cmd_remove.odin | 2 +- cmd_restore.odin | 2 +- 6 files changed, 274 insertions(+), 61 deletions(-) create mode 100644 cli_test.odin diff --git a/TODOS.md b/TODOS.md index 22c1b5b..ccf64f8 100644 --- a/TODOS.md +++ b/TODOS.md @@ -10,6 +10,8 @@ Note: These todos can wait until all the subcommands have been ported. 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"`). +30. **cmd_sync.odin:46-50, 64-68** — Double `db_insert` when `BackedUp`: first insert on line 48, then `db_update_required` is also true for `BackedUp` so second insert runs on line 65. Redundant and wasteful. + ## MEDIUM 4. **db.odin:29-35** — `make_temp_path` never calls `strings.builder_destroy`. Leaks builder buffer every call. @@ -32,6 +34,12 @@ Note: These todos can wait until all the subcommands have been ported. 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. +33. **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. + +34. **table.odin:84-88** — `render_json_rows` creates `map[string]string` per row, copies into dynamic array. `delete(entries)` frees the array but not individual map internals — potential map bucket leak per row. + +35. **prompt.odin:124** — `make([dynamic]bool, len(options))` creates N zero-initialized elements. Works because `false` is the default, but same footgun as original issue 1. Should be `make([dynamic]bool, 0, len(options))`. + ## LOW 14. [x] **db.odin:338-341** — Unnecessary `strings.clone` before `filepath.dir` (which already returns a slice into the input). @@ -44,6 +52,10 @@ Note: These todos can wait until all the subcommands have been ported. 19. **main.odin:42-46** — Dynamic array in `fallback_to_go` never deleted. Harmless since process exits. +36. **cli.odin:59-76** — Single-dash multi-char flags (e.g. `-force`) silently misparse. `-force` becomes flag `f` with value `o`, then `rce` as positional arg. Only `--force` and `-f` work correctly. + +37. **cmd_sync.odin:80, cmd_list.odin:33, cmd_deps.odin:9** — `make([]string, 2)` for table rows never freed. Leaks per row. Defer to memory pass. + ## 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"`. @@ -59,3 +71,9 @@ Note: These todos can wait until all the subcommands have been ported. 25. Add tests for untested commands. 26. Add a global --config -c flag to use an alternate config. + +27. version --long Odin only prints version; Go also prints commit hash and build date + +28. 2 scan tests silently skip Low When fd isn't installed, tests pass without actually testing anything. These should use #assert to be sure that fd is in path. + +29. nushell completions? diff --git a/cli.odin b/cli.odin index 75d8163..27da44b 100644 --- a/cli.odin +++ b/cli.odin @@ -12,27 +12,29 @@ Command :: struct { } CommandInfo :: struct { - name: string, - usage: string, - short: string, - long: string, + name: string, + usage: string, + short: string, + long: string, + aliases: []string, } COMMANDS := []CommandInfo{ {"init", "envr init", "Set up envr", - "The init command generates your initial config and saves it to\n~/.envr/config in JSON format.\n\nDuring setup, you will be prompted to select one or more ssh keys with which to\nencrypt your databse. **Make 100% sure** that you have **a remote copy** of this\nkey somewhere, otherwise your data could be lost forever."}, - {"scan", "envr scan", "Find and select .env files for backup", ""}, - {"sync", "envr sync", "Update or restore your env backups", ""}, - {"backup", "envr backup ", "Import a .env file into envr", ""}, - {"add", "envr add ", "Import a .env file into envr", ""}, - {"restore", "envr restore ", "Restore a .env file from the database", ""}, - {"list", "envr list", "View your tracked files", ""}, - {"remove", "envr remove ", "Remove a .env file from your database", ""}, - {"check", "envr check [path]", "Check if files are backed up", ""}, + "The init command generates your initial config and saves it to\n~/.envr/config in JSON format.\n\nDuring setup, you will be prompted to select one or more ssh keys with which to\nencrypt your databse. **Make 100% sure** that you have **a remote copy** of this\nkey somewhere, otherwise your data could be lost forever.", + {}}, + {"scan", "envr scan", "Find and select .env files for backup", "", {}}, + {"sync", "envr sync", "Update or restore your env backups", "", {}}, + {"backup", "envr backup ", "Import a .env file into envr", "", {"add"}}, + {"restore", "envr restore ", "Restore a .env file from the database", "", {}}, + {"list", "envr list", "View your tracked files", "", {}}, + {"remove", "envr remove ", "Remove a .env file from your database", "", {}}, + {"check", "envr check [path]", "Check if files are backed up", "", {}}, {"deps", "envr deps", "Check for missing binaries", - "envr relies on external binaries for certain functionality.\n\nThe check command reports on which binaries are available and which are not."}, - {"version", "envr version", "Show envr's version", ""}, - {"edit-config", "envr edit-config", "Edit your config with your default editor", ""}, + "envr relies on external binaries for certain functionality.\n\nThe check command reports on which binaries are available and which are not.", + {}}, + {"version", "envr version", "Show envr's version", "", {}}, + {"edit-config", "envr edit-config", "Edit your config with your default editor", "", {}}, } parse_args :: proc() -> (cmd: Command, ok: bool) { @@ -102,60 +104,123 @@ find_command :: proc(name: string) -> (CommandInfo, bool) { if c.name == name { return c, true } + for a in c.aliases { + if a == name { + return c, true + } + } } return CommandInfo{}, false } -print_command_help :: proc(name: string) { +command_help_text :: proc(name: string) -> (string, bool) { info, found := find_command(name) if !found { + return "", false + } + + b: strings.Builder + strings.builder_init(&b) + + fmt.sbprintf(&b, "Usage: %s [flags]\n\n", info.usage) + fmt.sbprintf(&b, "%s\n", info.short) + + if len(info.aliases) > 0 { + fmt.sbprintf(&b, "\nAliases:\n %s", info.name) + for a in info.aliases { + fmt.sbprintf(&b, ", %s", a) + } + fmt.sbprintf(&b, "\n") + } + + if len(info.long) > 0 { + fmt.sbprintf(&b, "\n%s\n", info.long) + } + + fmt.sbprintf(&b, "\nFlags:\n -h, --help help for %s\n", info.name) + + s := strings.clone(strings.to_string(b)) + strings.builder_destroy(&b) + return s, true +} + +print_command_help :: proc(name: string) { + text, ok := command_help_text(name) + if !ok { fmt.printf("Unknown command: %s\n", name) print_usage() return } - fmt.printf("Usage: %s\n\n%s\n", info.usage, info.short) - if len(info.long) > 0 { - fmt.printf("\n%s\n", info.long) + fmt.println(text) +} + +usage_text :: proc() -> string { + b: strings.Builder + strings.builder_init(&b) + + fmt.sbprintf(&b, "envr keeps your .env synced to a local, age encrypted database.\n") + fmt.sbprintf(&b, "Is a safe and easy way to gather all your .env files in one place where they can\n") + fmt.sbprintf(&b, "easily be backed by another tool such as restic or git.\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "All your data is stored in ~/data.age\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "Getting started is easy:\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "1. Create your configuration file and set up encrypted storage:\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "> envr init\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "2. Scan for existing .env files:\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "> envr scan\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "Select the files you want to back up from the interactive list.\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "3. Verify that it worked:\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "> envr list\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "4. After changing any of your .env files, update the backup with:\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "> envr sync\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "5. If you lose a repository, after re-cloning the repo into the same path it was\n") + fmt.sbprintf(&b, "at before, restore your backup with:\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "> envr restore ~//.env\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "Usage:\n") + fmt.sbprintf(&b, " envr [command]\n") + fmt.sbprintf(&b, "\n") + fmt.sbprintf(&b, "Available Commands:\n") + + for c in COMMANDS { + name_start := len(b.buf) + fmt.sbprintf(&b, "%s", c.name) + for a in c.aliases { + fmt.sbprintf(&b, ", %s", a) + } + name_len := len(b.buf) - name_start + padding := 20 - name_len + if padding > 0 { + for _ in 0.. envr init") - fmt.println("") - fmt.println("2. Scan for existing .env files:") - fmt.println("") - fmt.println("> envr scan") - fmt.println("") - fmt.println("Select the files you want to back up from the interactive list.") - fmt.println("") - fmt.println("3. Verify that it worked:") - fmt.println("") - fmt.println("> envr list") - fmt.println("") - fmt.println("Usage: envr [args]") - fmt.println("") - fmt.println("Commands:") - fmt.println(" init Set up envr") - fmt.println(" scan Find and select .env files for backup") - fmt.println(" sync Update or restore your env backups") - fmt.println(" backup Import a .env file into envr") - fmt.println(" restore Restore a .env file from the database") - fmt.println(" list View your tracked files") - fmt.println(" remove Remove a .env file from your database") - fmt.println(" check [path] Check if files are backed up") - fmt.println(" deps Check for missing binaries") - fmt.println(" version Show envr's version") - fmt.println(" edit-config Edit your config with your default editor") + fmt.print(usage_text()) } diff --git a/cli_test.odin b/cli_test.odin new file mode 100644 index 0000000..b952850 --- /dev/null +++ b/cli_test.odin @@ -0,0 +1,130 @@ +package main + +import "core:fmt" +import "core:strings" +import "core:testing" + +@(test) +test_usage_text_contains_all_commands :: proc(t: ^testing.T) { + text := usage_text() + + for c in COMMANDS { + testing.expect( + t, + strings.contains(text, c.name), + fmt.aprintf("usage_text missing command %q", c.name), + ) + for a in c.aliases { + testing.expect( + t, + strings.contains(text, a), + fmt.aprintf("usage_text missing alias %q", a), + ) + } + } +} + +@(test) +test_usage_text_contains_steps :: proc(t: ^testing.T) { + text := usage_text() + + testing.expect(t, strings.contains(text, "1."), "missing step 1") + testing.expect(t, strings.contains(text, "2."), "missing step 2") + testing.expect(t, strings.contains(text, "3."), "missing step 3") + testing.expect(t, strings.contains(text, "4."), "missing step 4") + testing.expect(t, strings.contains(text, "5."), "missing step 5") + testing.expect(t, strings.contains(text, "> envr sync\n"), "step 4 missing 'envr sync'") + testing.expect( + t, + strings.contains(text, "> envr restore"), + "step 5 missing 'envr restore'", + ) +} + +@(test) +test_usage_text_contains_flags_and_help_hint :: proc(t: ^testing.T) { + text := usage_text() + + testing.expect(t, strings.contains(text, "Flags:"), "missing Flags section") + testing.expect(t, strings.contains(text, "--help"), "missing --help flag") + testing.expect( + t, + strings.contains(text, "Use \"envr [command] --help\""), + "missing help hint", + ) +} + +@(test) +test_command_help_backup :: proc(t: ^testing.T) { + text, ok := command_help_text("backup") + testing.expect(t, ok, "command_help_text(\"backup\") returned false") + + testing.expect(t, strings.contains(text, "Usage:"), "missing Usage line") + testing.expect( + t, + strings.contains(text, "envr backup "), + "missing usage pattern", + ) + testing.expect( + t, + strings.contains(text, "Aliases:"), + "missing Aliases section", + ) + testing.expect(t, strings.contains(text, "add"), "missing 'add' alias") + testing.expect(t, strings.contains(text, "Flags:"), "missing Flags section") + testing.expect( + t, + strings.contains(text, "--help"), + "missing --help in flags", + ) +} + +@(test) +test_command_help_add_alias :: proc(t: ^testing.T) { + text, ok := command_help_text("add") + testing.expect(t, ok, "command_help_text(\"add\") returned false") + testing.expect( + t, + strings.contains(text, "envr backup "), + "'add' alias should resolve to backup usage", + ) + testing.expect(t, strings.contains(text, "Aliases:"), "missing Aliases section") +} + +@(test) +test_command_help_init_no_aliases :: proc(t: ^testing.T) { + text, ok := command_help_text("init") + testing.expect(t, ok, "command_help_text(\"init\") returned false") + + testing.expect(t, strings.contains(text, "Usage:"), "missing Usage line") + testing.expect( + t, + !strings.contains(text, "Aliases:"), + "init should not have Aliases section", + ) + testing.expect(t, strings.contains(text, "Flags:"), "missing Flags section") + testing.expect( + t, + strings.contains(text, "help for init"), + "missing 'help for init'", + ) +} + +@(test) +test_command_help_unknown :: proc(t: ^testing.T) { + text, ok := command_help_text("nonexistent") + testing.expect(t, !ok, "command_help_text(\"nonexistent\") should return false") + testing.expect(t, len(text) == 0, "text should be empty for unknown command") +} + +@(test) +test_command_help_version :: proc(t: ^testing.T) { + text, ok := command_help_text("version") + testing.expect(t, ok, "command_help_text(\"version\") returned false") + testing.expect(t, strings.contains(text, "Usage:"), "missing Usage line") + testing.expect( + t, + !strings.contains(text, "Aliases:"), + "version should not have Aliases section", + ) +} diff --git a/cmd_backup.odin b/cmd_backup.odin index 5604a89..0be0f3f 100644 --- a/cmd_backup.odin +++ b/cmd_backup.odin @@ -5,7 +5,7 @@ import "core:strings" cmd_backup :: proc(cmd: ^Command) { if len(cmd.args) != 1 { - fmt.println("Usage: envr backup ") + print_command_help("backup") return } diff --git a/cmd_remove.odin b/cmd_remove.odin index 46f9d07..febbe76 100644 --- a/cmd_remove.odin +++ b/cmd_remove.odin @@ -6,7 +6,7 @@ import "core:strings" cmd_remove :: proc(cmd: ^Command) { if len(cmd.args) != 1 { - fmt.println("Usage: envr remove ") + print_command_help("remove") return } diff --git a/cmd_restore.odin b/cmd_restore.odin index dc92517..9e27400 100644 --- a/cmd_restore.odin +++ b/cmd_restore.odin @@ -7,7 +7,7 @@ import "core:strings" cmd_restore :: proc(cmd: ^Command) { if len(cmd.args) != 1 { - fmt.println("Usage: envr restore ") + print_command_help("restore") return }