From a4f4b10a7b47029e24c69d61274b7d39c0dea2ee Mon Sep 17 00:00:00 2001 From: Spencer Brower Date: Fri, 26 Jun 2026 13:36:01 -0400 Subject: [PATCH] refactor: Used RTTI for more sophisticated flag parsing. --- TODOS.md | 3 + cli.odin | 96 ++++++++--------------------- cli_test.odin | 142 +++++++++++++------------------------------ cmd_backup.odin | 2 +- cmd_check.odin | 2 +- cmd_edit_config.odin | 2 +- cmd_init.odin | 24 +++++--- cmd_list.odin | 4 +- cmd_list_test.odin | 8 +-- cmd_remove.odin | 2 +- cmd_restore.odin | 2 +- cmd_scan.odin | 2 +- cmd_sync.odin | 4 +- flags.odin | 134 ++++++++++++++++++++++++++++++++++++++++ 14 files changed, 236 insertions(+), 191 deletions(-) create mode 100644 flags.odin diff --git a/TODOS.md b/TODOS.md index 8714e4c..009b1d5 100644 --- a/TODOS.md +++ b/TODOS.md @@ -28,6 +28,8 @@ 14. Add a text filter to the multi_select. +15. init -h doesn't show --force flag. + ## Double-check AI output - [ ] cli.odin @@ -54,6 +56,7 @@ - [ ] db.odin - [ ] db_integration_test.odin - [ ] db_test.odin +- [ ] flags.odin - [x] main.odin - [x] prompt.odin - [x] scan.odin diff --git a/cli.odin b/cli.odin index 8aafece..85ee1ef 100644 --- a/cli.odin +++ b/cli.odin @@ -9,17 +9,24 @@ import "core:terminal" import "core:text/table" Command :: struct { - name: string, - args: [dynamic]string, - flags: map[string]string, - bool_set: map[string]bool, - config_path: string, - out_buf: ^bufio.Writer, - out: io.Writer, - err: io.Writer, + name: string, + args: [dynamic]string, + flags: Flags, + out_buf: ^bufio.Writer, + out: io.Writer, + err: io.Writer, +} + +// TODO: Put help test in usage:"whatever" tag. +Flags :: struct { + help: bool `args:"short=h"`, + config_file: string `args:"name=config-file,short=c"`, + output: Output_Format `args:"short=o"`, + force: bool `args:"short=f"`, } Output_Format :: enum { + Auto, Table, JSON, } @@ -77,53 +84,26 @@ parse_args :: proc(args: []string, out: io.Stream, err: io.Stream) -> (cmd: Comm } cmd.name = args[1] - cmd.args = make([dynamic]string) - cmd.flags = make(map[string]string) - cmd.bool_set = make(map[string]bool) - // TODO: Optimize loop? - i := 2 - for i < len(args) { - arg := args[i] - if strings.starts_with(arg, "--") { - key := arg[2:] - if i + 1 < len(args) && !strings.starts_with(args[i + 1], "-") { - cmd.flags[key] = args[i + 1] - i += 2 - } else { - cmd.bool_set[key] = true - i += 1 - } - } else if strings.starts_with(arg, "-") && len(arg) == 2 { - key_slice := arg[1:2] - if i + 1 < len(args) && !strings.starts_with(args[i + 1], "-") { - cmd.flags[key_slice] = args[i + 1] - i += 2 - } else { - cmd.bool_set[key_slice] = true - i += 1 - } - } else { - append(&cmd.args, arg) - i += 1 - } + overflow := parse_flags(&cmd.flags, args[2:]) + for arg in overflow { + append(&cmd.args, arg) } - val: string = --- - if val, ok = cmd.flags["config-file"]; ok { - cmd.config_path = val - } else if val, ok = cmd.flags["c"]; ok { - cmd.config_path = val - } else { + if cmd.flags.output == .Auto { + cmd.flags.output = terminal.is_terminal(os.stdout) ? .Table : .JSON + } + + if cmd.flags.config_file == "" { // FIXME: Handle err // TODO: Is this right? home, _ := os.user_home_dir(context.temp_allocator) // TODO: should we copy out of the temp_allocator? - cmd.config_path = default_config_path(home, context.temp_allocator) + cmd.flags.config_file = default_config_path(home, context.temp_allocator) } - if has_flag(&cmd, "help") || has_flag(&cmd, "h") { + if cmd.flags.help { print_command_help(&cmd) return cmd, false } @@ -296,8 +276,8 @@ at before, restore your backup with: ) table.row( &tbl, - COLOR_FLAGS + "-f, --format" + ANSI_RESET + " 'json'|'table'", - `the format of output data. (default 'table', unless piping)`, + COLOR_FLAGS + "-o, --output" + ANSI_RESET + " 'table'|'json'", + `the format of output data. (default 'table')`, ) write_borderless_table(w, &tbl) @@ -310,31 +290,9 @@ at before, restore your backup with: ) } -has_flag :: proc(cmd: ^Command, name: string) -> bool { - return name in cmd.flags || name in cmd.bool_set -} - -get_format :: proc(cmd: ^Command) -> Output_Format { - flags :: []string{"format", "f"} - for name in flags { - if val, ok := cmd.flags[name]; ok { - switch val { - case "json": - return .JSON - case "table": - return .Table - } - } - } - - return terminal.is_terminal(os.stdout) ? .Table : .JSON -} - delete_command :: proc(cmd: ^Command) { bufio.writer_flush(cmd.out_buf) delete(cmd.args) - delete(cmd.flags) - delete(cmd.bool_set) bufio.writer_destroy(cmd.out_buf) free(cmd.out_buf) } diff --git a/cli_test.odin b/cli_test.odin index 06ce6fc..413fd4e 100644 --- a/cli_test.odin +++ b/cli_test.odin @@ -144,53 +144,6 @@ test_command_help_version :: proc(t: ^testing.T) { ) } -@(test) -test_has_flag_bool_set :: proc(t: ^testing.T) { - cmd := Command { - name = "test", - bool_set = map[string]bool{"force" = true}, - } - defer delete(cmd.bool_set) - - testing.expect(t, has_flag(&cmd, "force"), "should find flag in bool_set") - testing.expect(t, !has_flag(&cmd, "verbose"), "should not find missing flag") -} - -@(test) -test_has_flag_value_map :: proc(t: ^testing.T) { - cmd := Command { - name = "test", - flags = map[string]string{"output" = "/tmp/out"}, - } - defer delete(cmd.flags) - - testing.expect(t, has_flag(&cmd, "output"), "should find flag in flags map") - testing.expect(t, !has_flag(&cmd, "force"), "should not find missing flag") -} - -@(test) -test_has_flag_both_maps :: proc(t: ^testing.T) { - cmd := Command { - name = "test", - flags = map[string]string{"output" = "/tmp/out"}, - bool_set = map[string]bool{"force" = true}, - } - defer delete(cmd.flags) - defer delete(cmd.bool_set) - - testing.expect(t, has_flag(&cmd, "output"), "should find in flags") - testing.expect(t, has_flag(&cmd, "force"), "should find in bool_set") - testing.expect(t, !has_flag(&cmd, "verbose"), "should not find missing flag") -} - -@(test) -test_has_flag_empty_command :: proc(t: ^testing.T) { - cmd := Command { - name = "test", - } - testing.expect(t, !has_flag(&cmd, "anything"), "empty command should have no flags") -} - test_parse_args :: proc( args: []string, ) -> ( @@ -226,8 +179,6 @@ test_parse_args_bare_command :: proc(t: ^testing.T) { testing.expect_value(t, cmd.name, "list") testing.expect_value(t, len(cmd.args), 0) - testing.expect_value(t, len(cmd.flags), 0) - testing.expect_value(t, len(cmd.bool_set), 0) } @(test) @@ -242,43 +193,45 @@ test_parse_args_positional :: proc(t: ^testing.T) { } @(test) -test_parse_args_long_flag_with_value :: proc(t: ^testing.T) { - cmd, ok, _, _ := test_parse_args([]string{"envr", "sync", "--config", "x.json"}) +test_parse_args_config_file_long_flag :: proc(t: ^testing.T) { + cmd, ok, _, _ := test_parse_args( + []string{"envr", "sync", "--config-file", "x.json"}, + ) testing.expect(t, ok, "should succeed") if !ok do return defer delete_command(&cmd) - testing.expect_value(t, cmd.flags["config"], "x.json") + testing.expect_value(t, cmd.flags.config_file, "x.json") } @(test) -test_parse_args_short_flag_with_value :: proc(t: ^testing.T) { +test_parse_args_config_file_short_flag :: proc(t: ^testing.T) { cmd, ok, _, _ := test_parse_args([]string{"envr", "sync", "-c", "x.json"}) testing.expect(t, ok, "should succeed") if !ok do return defer delete_command(&cmd) - testing.expect_value(t, cmd.flags["c"], "x.json") + testing.expect_value(t, cmd.flags.config_file, "x.json") } @(test) -test_parse_args_long_bool_flag :: proc(t: ^testing.T) { +test_parse_args_force_long_flag :: proc(t: ^testing.T) { cmd, ok, _, _ := test_parse_args([]string{"envr", "init", "--force"}) testing.expect(t, ok, "should succeed") if !ok do return defer delete_command(&cmd) - testing.expect_value(t, cmd.bool_set["force"], true) + testing.expect_value(t, cmd.flags.force, true) } @(test) -test_parse_args_short_bool_flag :: proc(t: ^testing.T) { - cmd, ok, _, _ := test_parse_args([]string{"envr", "version", "-l"}) +test_parse_args_force_short_flag :: proc(t: ^testing.T) { + cmd, ok, _, _ := test_parse_args([]string{"envr", "init", "-f"}) testing.expect(t, ok, "should succeed") if !ok do return defer delete_command(&cmd) - testing.expect_value(t, cmd.bool_set["l"], true) + testing.expect_value(t, cmd.flags.force, true) } @(test) @@ -300,7 +253,7 @@ test_parse_args_mixed_flags_and_positionals :: proc(t: ^testing.T) { if !ok do return defer delete_command(&cmd) - testing.expect_value(t, cmd.bool_set["force"], true) + testing.expect_value(t, cmd.flags.force, true) testing.expect_value(t, len(cmd.args), 1) testing.expect_value(t, cmd.args[0], "/project/.env") } @@ -314,90 +267,77 @@ test_parse_args_no_args :: proc(t: ^testing.T) { @(test) test_parse_args_flag_then_positional_then_flag :: proc(t: ^testing.T) { - cmd, ok, _, _ := test_parse_args([]string{"envr", "backup", "a.env", "--force", "--verbose"}) + cmd, ok, _, _ := test_parse_args([]string{"envr", "backup", "--force", "a.env", "--output", "json"}) defer delete_command(&cmd) testing.expect(t, ok, "should succeed") - testing.expect_value(t, cmd.bool_set["force"], true) - testing.expect_value(t, cmd.bool_set["verbose"], true) + testing.expect_value(t, cmd.flags.force, true) + testing.expect_value(t, cmd.flags.output, Output_Format.JSON) testing.expect_value(t, len(cmd.args), 1) testing.expect_value(t, cmd.args[0], "a.env") } @(test) -test_parse_args_config_file_long_flag :: proc(t: ^testing.T) { - cmd, ok, _, _ := test_parse_args( - []string{"envr", "list", "--config-file", "/custom/config.json"}, - ) - testing.expect(t, ok, "should succeed") - if !ok do return - defer delete_command(&cmd) - - testing.expect_value(t, cmd.config_path, "/custom/config.json") -} - -@(test) -test_parse_args_config_file_short_flag :: proc(t: ^testing.T) { - cmd, ok, _, _ := test_parse_args([]string{"envr", "list", "-c", "/custom/config.json"}) - testing.expect(t, ok, "should succeed") - if !ok do return - defer delete_command(&cmd) - - testing.expect_value(t, cmd.config_path, "/custom/config.json") -} - -@(test) -test_parse_args_config_file_defaults :: proc(t: ^testing.T) { +test_parse_args_config_file_default :: proc(t: ^testing.T) { cmd, ok, _, _ := test_parse_args([]string{"envr", "list"}) testing.expect(t, ok, "should succeed") if !ok do return defer delete_command(&cmd) - testing.expect(t, len(cmd.config_path) > 0, "config_path should default to non-empty path") + testing.expect(t, len(cmd.flags.config_file) > 0, "config_file should default to non-empty path") testing.expect( t, - strings.contains(cmd.config_path, ".envr"), - "default config_path should contain .envr dir, got %s", + strings.contains(cmd.flags.config_file, ".envr"), + "default config_file should contain .envr dir, got %s", ) } @(test) -test_get_format_long_json :: proc(t: ^testing.T) { - cmd, ok, _, _ := test_parse_args([]string{"envr", "list", "--format", "json"}) +test_parse_args_output_long_json :: proc(t: ^testing.T) { + cmd, ok, _, _ := test_parse_args([]string{"envr", "list", "--output", "json"}) testing.expect(t, ok, "should succeed") if !ok do return defer delete_command(&cmd) - testing.expect_value(t, get_format(&cmd), Output_Format.JSON) + testing.expect_value(t, cmd.flags.output, Output_Format.JSON) } @(test) -test_get_format_short_json :: proc(t: ^testing.T) { - cmd, ok, _, _ := test_parse_args([]string{"envr", "list", "-f", "json"}) +test_parse_args_output_short_json :: proc(t: ^testing.T) { + cmd, ok, _, _ := test_parse_args([]string{"envr", "list", "-o", "json"}) testing.expect(t, ok, "should succeed") if !ok do return defer delete_command(&cmd) - testing.expect_value(t, get_format(&cmd), Output_Format.JSON) + testing.expect_value(t, cmd.flags.output, Output_Format.JSON) } @(test) -test_get_format_long_table :: proc(t: ^testing.T) { - cmd, ok, _, _ := test_parse_args([]string{"envr", "list", "--format", "table"}) +test_parse_args_output_long_table :: proc(t: ^testing.T) { + cmd, ok, _, _ := test_parse_args([]string{"envr", "list", "--output", "table"}) testing.expect(t, ok, "should succeed") if !ok do return defer delete_command(&cmd) - testing.expect_value(t, get_format(&cmd), Output_Format.Table) + testing.expect_value(t, cmd.flags.output, Output_Format.Table) } @(test) -test_get_format_short_table :: proc(t: ^testing.T) { - cmd, ok, _, _ := test_parse_args([]string{"envr", "list", "-f", "table"}) +test_parse_args_output_short_table :: proc(t: ^testing.T) { + cmd, ok, _, _ := test_parse_args([]string{"envr", "list", "-o", "table"}) testing.expect(t, ok, "should succeed") if !ok do return defer delete_command(&cmd) - testing.expect_value(t, get_format(&cmd), Output_Format.Table) + testing.expect_value(t, cmd.flags.output, Output_Format.Table) } +@(test) +test_parse_args_output_equals_syntax :: proc(t: ^testing.T) { + cmd, ok, _, _ := test_parse_args([]string{"envr", "list", "--output=json"}) + testing.expect(t, ok, "should succeed") + if !ok do return + defer delete_command(&cmd) + + testing.expect_value(t, cmd.flags.output, Output_Format.JSON) +} diff --git a/cmd_backup.odin b/cmd_backup.odin index fd2f7fa..146973d 100644 --- a/cmd_backup.odin +++ b/cmd_backup.odin @@ -23,7 +23,7 @@ cmd_backup :: proc(cmd: ^Command) { return } - db, db_ok := db_open(cmd.config_path) + db, db_ok := db_open(cmd.flags.config_file) if !db_ok { return } diff --git a/cmd_check.odin b/cmd_check.odin index 5b5d741..9118ae5 100644 --- a/cmd_check.odin +++ b/cmd_check.odin @@ -24,7 +24,7 @@ cmd_check :: proc(cmd: ^Command) { return } - db, db_ok := db_open(cmd.config_path) + db, db_ok := db_open(cmd.flags.config_file) if !db_ok { return } diff --git a/cmd_edit_config.odin b/cmd_edit_config.odin index bd39d87..d66ec23 100644 --- a/cmd_edit_config.odin +++ b/cmd_edit_config.odin @@ -10,7 +10,7 @@ cmd_edit_config :: proc(cmd: ^Command) { return } - config_path := cmd.config_path + config_path := cmd.flags.config_file if !os.exists(config_path) { fmt.wprintf( diff --git a/cmd_init.odin b/cmd_init.odin index c9e1d99..9322fdb 100644 --- a/cmd_init.odin +++ b/cmd_init.odin @@ -4,11 +4,12 @@ import "core:fmt" import "core:terminal/ansi" cmd_init :: proc(cmd: ^Command) { - force := has_flag(cmd, "force") || has_flag(cmd, "f") + force := cmd.flags.force + config_file := cmd.flags.config_file - fmt.wprintln(cmd.out, cmd.config_path, flush = false) + fmt.wprintln(cmd.out, cmd.flags.config_file, flush = false) - _, cfg_exists := load_config(cmd.config_path) + _, cfg_exists := load_config(config_file) if cfg_exists && !force { fmt.wprintln( cmd.out, @@ -25,15 +26,23 @@ Run again with the --force flag if you want to reinitialize.`, } if len(keys) == 0 { - fmt.wprintln(cmd.err, `No ssh-ed25519 keys found in ~/.ssh -Generate one with: ssh-keygen -t ed25519`, flush = false) + fmt.wprintln( + cmd.err, + `No ssh-ed25519 keys found in ~/.ssh +Generate one with: ssh-keygen -t ed25519`, + flush = false, + ) return } selected, result := multi_select("Select SSH private keys:", keys[:]) defer delete(selected) if result == .Cancel { - fmt.wprintln(cmd.out, ansi.CSI + ansi.FAINT + ansi.SGR + "Cancelled." + ANSI_RESET, flush = false) + fmt.wprintln( + cmd.out, + ansi.CSI + ansi.FAINT + ansi.SGR + "Cancelled." + ANSI_RESET, + flush = false, + ) return } @@ -49,7 +58,7 @@ Generate one with: ssh-keygen -t ed25519`, flush = false) return } - cfg := new_config(selected_paths[:], cmd.config_path) + cfg := new_config(selected_paths[:], config_file) if !save_config(cfg, force = force) { return } @@ -61,3 +70,4 @@ Generate one with: ssh-keygen -t ed25519`, flush = false) flush = false, ) } + diff --git a/cmd_list.odin b/cmd_list.odin index 56720e4..c3ba9f9 100644 --- a/cmd_list.odin +++ b/cmd_list.odin @@ -14,7 +14,7 @@ ListEntry :: struct { // TODO: Improve table rendering cmd_list :: proc(cmd: ^Command) { - db, db_ok := db_open(cmd.config_path) + db, db_ok := db_open(cmd.flags.config_file) if !db_ok { return } @@ -25,7 +25,7 @@ cmd_list :: proc(cmd: ^Command) { return } - if get_format(cmd) == .Table { + if cmd.flags.output == .Table { t: table.Table table.init(&t, context.temp_allocator, context.temp_allocator) table.padding(&t, 1, 1) diff --git a/cmd_list_test.odin b/cmd_list_test.odin index 8ecefe1..f309b56 100644 --- a/cmd_list_test.odin +++ b/cmd_list_test.odin @@ -22,7 +22,7 @@ test_filepath_base_equals_rel :: proc(t: ^testing.T) { } @(test) -test_cmd_list_format_json :: proc(t: ^testing.T) { +test_cmd_list_output_json :: proc(t: ^testing.T) { base := test_temp_dir(t, "envr-test-list-json-*") defer os.remove_all(base) @@ -47,7 +47,7 @@ test_cmd_list_format_json :: proc(t: ^testing.T) { defer strings.builder_destroy(&err_b) cmd, ok := parse_args( - []string{"envr", "list", "--format", "json", "--config-file", cfg_path}, + []string{"envr", "list", "--output", "json", "--config-file", cfg_path}, strings.to_stream(&out_b), strings.to_stream(&err_b), ) @@ -68,7 +68,7 @@ test_cmd_list_format_json :: proc(t: ^testing.T) { } @(test) -test_cmd_list_format_table :: proc(t: ^testing.T) { +test_cmd_list_output_table :: proc(t: ^testing.T) { base := test_temp_dir(t, "envr-test-list-table-*") defer os.remove_all(base) @@ -93,7 +93,7 @@ test_cmd_list_format_table :: proc(t: ^testing.T) { defer strings.builder_destroy(&err_b) cmd, ok := parse_args( - []string{"envr", "list", "--format", "table", "--config-file", cfg_path}, + []string{"envr", "list", "--output", "table", "--config-file", cfg_path}, strings.to_stream(&out_b), strings.to_stream(&err_b), ) diff --git a/cmd_remove.odin b/cmd_remove.odin index 181adb0..7b3c3a0 100644 --- a/cmd_remove.odin +++ b/cmd_remove.odin @@ -22,7 +22,7 @@ cmd_remove :: proc(cmd: ^Command) { return } - db, db_ok := db_open(cmd.config_path) + db, db_ok := db_open(cmd.flags.config_file) if !db_ok { return } diff --git a/cmd_restore.odin b/cmd_restore.odin index e43fb63..438a6bf 100644 --- a/cmd_restore.odin +++ b/cmd_restore.odin @@ -23,7 +23,7 @@ cmd_restore :: proc(cmd: ^Command) { return } - db, db_ok := db_open(cmd.config_path) + db, db_ok := db_open(cmd.flags.config_file) if !db_ok { return } diff --git a/cmd_scan.odin b/cmd_scan.odin index 1549317..437c09d 100644 --- a/cmd_scan.odin +++ b/cmd_scan.odin @@ -7,7 +7,7 @@ import "core:terminal" import "core:terminal/ansi" cmd_scan :: proc(cmd: ^Command) { - db, db_ok := db_open(cmd.config_path) + db, db_ok := db_open(cmd.flags.config_file) if !db_ok { return } diff --git a/cmd_sync.odin b/cmd_sync.odin index 420bb16..dbd3e51 100644 --- a/cmd_sync.odin +++ b/cmd_sync.odin @@ -11,7 +11,7 @@ SyncEntry :: struct { // TODO: Check for quiet failures. cmd_sync :: proc(cmd: ^Command) { - db, db_ok := db_open(cmd.config_path) + db, db_ok := db_open(cmd.flags.config_file) if !db_ok { return } @@ -46,7 +46,7 @@ cmd_sync :: proc(cmd: ^Command) { } } - if get_format(cmd) == .Table { + if cmd.flags.output == .Table { t: table.Table table.init(&t, context.temp_allocator, context.temp_allocator) table.padding(&t, 1, 1) diff --git a/flags.odin b/flags.odin new file mode 100644 index 0000000..87fc28c --- /dev/null +++ b/flags.odin @@ -0,0 +1,134 @@ +package main + +import "base:runtime" +import "core:reflect" +import "core:strings" + +get_subtag :: proc(tag: string, id: string) -> (value: string, ok: bool) { + parts := strings.split(tag, ",", context.temp_allocator) + for part in parts { + trimmed := strings.trim_space(part) + if strings.has_prefix(trimmed, id) && len(trimmed) > len(id) && trimmed[len(id)] == '=' { + return trimmed[len(id) + 1:], true + } + if trimmed == id { + return "", true + } + } + return "", false +} + +is_bool_type :: proc(field: reflect.Struct_Field) -> bool { + base_ti := runtime.type_info_base(field.type) + _, is_bool := base_ti.variant.(runtime.Type_Info_Boolean) + return is_bool +} + +set_field :: proc(model: rawptr, field: reflect.Struct_Field, value: string) -> bool { + ptr := rawptr(uintptr(model) + field.offset) + base_ti := runtime.type_info_base(field.type) + + if _, is_bool := base_ti.variant.(runtime.Type_Info_Boolean); is_bool { + (cast(^bool)ptr)^ = true + return true + } + + if _, is_string := base_ti.variant.(runtime.Type_Info_String); is_string { + (cast(^string)ptr)^ = value + return true + } + + if enum_ti, is_enum := base_ti.variant.(runtime.Type_Info_Enum); is_enum { + for name, i in enum_ti.names { + if strings.equal_fold(value, name) { + v := enum_ti.values[i] + switch base_ti.size { + case 1: (cast(^u8)ptr)^ = cast(u8)v + case 2: (cast(^u16)ptr)^ = cast(u16)v + case 4: (cast(^u32)ptr)^ = cast(u32)v + case 8: (cast(^u64)ptr)^ = cast(u64)v + } + return true + } + } + } + + return false +} + +parse_flags :: proc(model: ^$T, args: []string) -> (overflow: []string) { + field_count := reflect.struct_field_count(T) + long_map := make(map[string]reflect.Struct_Field, field_count, context.temp_allocator) + short_map := make(map[string]reflect.Struct_Field, field_count, context.temp_allocator) + + for i in 0..