diff --git a/.kiro/specs/xce-test-cli-ux/tasks.md b/.kiro/specs/xce-test-cli-ux/tasks.md index 3f45f53a..a5039ea7 100644 --- a/.kiro/specs/xce-test-cli-ux/tasks.md +++ b/.kiro/specs/xce-test-cli-ux/tasks.md @@ -9,7 +9,7 @@ - _Requirements: 2.1, 2.2, 2.3, 2.4, 3.1_ - _Boundary: Flag Parsing Layer_ -- [ ] 1.2 Build the flag typo-suggestion utility and the generated help-flag-block renderer +- [x] 1.2 Build the flag typo-suggestion utility and the generated help-flag-block renderer - Add a small edit-distance comparison so an unrecognized flag that closely matches exactly one flag in the descriptor table is named as the likely intended one in the rejection message - Add a renderer that produces the flag-listing portion of help text (short form, long form, required/optional, default) directly from the descriptor table - Observable completion: the unit test suite shows a single close-match flag typo producing a "did you mean" suggestion, a non-close-match producing no suggestion, and the renderer producing a correctly formatted block for a multi-flag test table diff --git a/src/controller/testsuite/flagspec.c b/src/controller/testsuite/flagspec.c index 56b35404..0302727b 100644 --- a/src/controller/testsuite/flagspec.c +++ b/src/controller/testsuite/flagspec.c @@ -3,6 +3,7 @@ #include #include +#include #include /* True iff `token` (a raw command-line argument, e.g. "-h", "--host", or @@ -80,6 +81,139 @@ int flagspec_build_opts(const flagspec_t* spec, size_t n, return 0; } +/* Levenshtein edit distance between two NUL-terminated strings -- the + * minimum number of single-character insertions/deletions/substitutions + * that turns `a` into `b`. Small and self-contained: research.md confirms + * no edit-distance utility exists anywhere in the codebase yet. Standard + * two-row rolling DP, O(len(a)*len(b)) time, O(min(len(a),len(b))+1) + * space -- more than fast enough for flag-name-sized strings. + * + * Returns (size_t)-1 on allocation failure, which callers must treat as + * "not a close match" rather than a false zero-distance/exact-match + * signal. */ +static size_t levenshtein(const char* a, const char* b) +{ + size_t la = strlen(a), lb = strlen(b); + + /* Keep the rolling row sized to the SHORTER string. */ + if (lb > la) { + const char* tmp = a; + a = b; + b = tmp; + size_t t = la; + la = lb; + lb = t; + } + + size_t* row = calloc(lb + 1, sizeof(size_t)); + if (!row) + return (size_t)-1; + + for (size_t j = 0; j <= lb; j++) + row[j] = j; + + for (size_t i = 1; i <= la; i++) { + size_t prev_diag = row[0]; + row[0] = i; + for (size_t j = 1; j <= lb; j++) { + size_t saved = row[j]; + size_t cost = (a[i - 1] == b[j - 1]) ? 0 : 1; + size_t del = row[j] + 1; + size_t ins = row[j - 1] + 1; + size_t sub = prev_diag + cost; + size_t best = del < ins ? del : ins; + if (sub < best) + best = sub; + row[j] = best; + prev_diag = saved; + } + } + + size_t result = row[lb]; + free(row); + return result; +} + +/* True iff `candidate` (one flag's literal spelling, e.g. "-h" or + * "--host") "closely matches" `bad_flag` for typo-suggestion purposes + * (Requirement 10.1). + * + * Threshold, a deliberate design choice (design.md/research.md do not + * mandate a specific algorithm or cutoff): floor(len(candidate)/4), + * clamped to [1,3]. A fixed absolute threshold would be wrong at both + * ends -- distance 1 from a 2-character short flag ("-h" vs "-z") is a + * near-total rewrite, while distance 1 from an 8+ character long flag is + * a single typo -- so the tolerance scales with the spelling's own + * length instead. Exact matches (distance 0) are never "close" here: + * flagspec_reject() only calls this from the already-unrecognized branch, + * where an exact-spelling candidate would be a contradiction, and this + * keeps the helper correct standalone too. */ +static int is_close_match(const char* bad_flag, const char* candidate) +{ + size_t dist = levenshtein(bad_flag, candidate); + if (dist == 0 || dist == (size_t)-1) + return 0; + + size_t len = strlen(candidate); + size_t threshold = len / 4; + if (threshold < 1) + threshold = 1; + if (threshold > 3) + threshold = 3; + + return dist <= threshold; +} + +/* Writes `fs`'s display label ("-h/--host" or, for a long-only flag, + * "--json") into `buf`. */ +static void format_flag_label(char* buf, size_t cap, const flagspec_t* fs) +{ + if (fs->short_opt) + snprintf(buf, cap, "-%c/--%s", fs->short_opt, fs->long_opt); + else + snprintf(buf, cap, "--%s", fs->long_opt); +} + +/* Returns the single flagspec_t in `spec` whose short or long spelling + * closely matches `bad_flag`, or NULL when zero or more than one entry + * qualifies -- Requirement 10.1 only promises a guess when exactly one + * flag is a plausible intended target; an ambiguous typo (two or more + * equally-plausible candidates) gets no guess at all, the same as a typo + * that matches nothing (Requirement 10.2). */ +static const flagspec_t* find_typo_suggestion(const flagspec_t* spec, size_t n, + const char* bad_flag) +{ + if (!spec || !bad_flag || !*bad_flag) + return NULL; + + const flagspec_t* found = NULL; + + for (size_t i = 0; i < n; i++) { + const flagspec_t* fs = &spec[i]; + int is_close = 0; + + if (fs->short_opt) { + char short_form[3] = { '-', fs->short_opt, '\0' }; + if (is_close_match(bad_flag, short_form)) + is_close = 1; + } + if (!is_close && fs->long_opt && *fs->long_opt) { + char long_form[128]; + snprintf(long_form, sizeof(long_form), "--%s", fs->long_opt); + if (is_close_match(bad_flag, long_form)) + is_close = 1; + } + + if (is_close) { + if (found) + return NULL; /* ambiguous -- more than one candidate */ + found = fs; + } + } + + return found; +} + int flagspec_reject(const char* cmd_name, const flagspec_t* spec, size_t n, const char* bad_flag, const char* usage_text) { @@ -103,8 +237,18 @@ int flagspec_reject(const char* cmd_name, const flagspec_t* spec, size_t n, fprintf(stderr, "xce_test: %s: flag '%s' does not take a value\n", who, display); } else { - fprintf(stderr, "xce_test: %s: unrecognized flag '%s'\n", who, - display); + const flagspec_t* suggestion = find_typo_suggestion(spec, n, bad_flag); + if (suggestion) { + char label[144]; + format_flag_label(label, sizeof(label), suggestion); + fprintf(stderr, + "xce_test: %s: unrecognized flag '%s' (did you mean " + "'%s'?)\n", + who, display, label); + } else { + fprintf(stderr, "xce_test: %s: unrecognized flag '%s'\n", who, + display); + } } if (usage_text && *usage_text) { @@ -117,3 +261,38 @@ int flagspec_reject(const char* cmd_name, const flagspec_t* spec, size_t n, return -EINVAL; } + +char* flagspec_render_help(const flagspec_t* spec, size_t n) +{ + if (!spec || n == 0) + return NULL; + + char* buf = NULL; + size_t size = 0; + FILE* f = open_memstream(&buf, &size); + if (!f) + return NULL; + + for (size_t i = 0; i < n; i++) { + const flagspec_t* fs = &spec[i]; + + if (fs->short_opt) + fprintf(f, " -%c, --%s", fs->short_opt, fs->long_opt); + else + fprintf(f, " --%s", fs->long_opt); + + fprintf(f, " %s", fs->required ? "required" : "optional"); + + if (fs->default_desc && *fs->default_desc) + fprintf(f, " (default: %s)", fs->default_desc); + + fprintf(f, "\n"); + } + + if (fclose(f) != 0) { + free(buf); + return NULL; + } + + return buf; +} diff --git a/src/controller/testsuite/flagspec.h b/src/controller/testsuite/flagspec.h index f56058be..fadbcdb1 100644 --- a/src/controller/testsuite/flagspec.h +++ b/src/controller/testsuite/flagspec.h @@ -1,24 +1,26 @@ #pragma once /* - * flagspec.h -- flag-descriptor table format and getopt_long-based parsing - * core for xce_test's command-line surface (xce-test-cli-ux spec, task 1.1; - * Requirements 2.1-2.4, 3.1; the flagspec_t shape is a stable contract also - * relied on by task 1.2 (typo suggestion, flagspec_render_help) and task - * 3.1 (migrating every existing cmd_* command onto this table)). + * flagspec.h -- flag-descriptor table format, getopt_long-based parsing + * core, typo suggestion, and help-flag-block rendering for xce_test's + * command-line surface (xce-test-cli-ux spec, tasks 1.1/1.2; Requirements + * 2.1-2.4, 3.1, 6.1-6.4, 8.2, 10.1-10.3; the flagspec_t shape is a stable + * contract also relied on by task 3.1 (migrating every existing cmd_* + * command onto this table)). * * bug-1711's root cause was that xce_test's flag parsing had no shared * helper at all: a plain getopt() loop per command, each with its own * hand-written switch/default, drifted independently from its * hand-authored USAGE_* string. This module gives every flag-accepting * command ONE descriptor table to build its getopt_long() option table - * from and ONE shared rejection path to call on any parse failure, so the - * table is the single source of truth for both. + * from, ONE shared rejection path (with typo suggestion) to call on any + * parse failure, and ONE renderer for the flag-listing portion of its + * help text, so the table is the single source of truth for all three. * - * THIS FILE/TASK (1.1) declares and implements only flagspec_build_opts() - * and flagspec_reject(). flagspec_render_help() (the help-text flag-block - * renderer) and flagspec_reject()'s typo-suggestion behavior are task - * 1.2 and are intentionally NOT declared here yet -- task 1.2 extends - * this header when it adds them. + * Task 1.1 declared/implemented flagspec_build_opts() and + * flagspec_reject() (without typo suggestion). THIS TASK (1.2) adds + * flagspec_render_help() and extends flagspec_reject()'s rejection + * message with a "did you mean" suggestion (Requirement 10) when exactly + * one flag in `spec` closely matches the unrecognized token. */ #include @@ -116,8 +118,16 @@ int flagspec_build_opts(const flagspec_t* spec, size_t n, * 2.4) to stderr only (Requirement 3.1 -- never stdout) and returns a * negative value; it never partially executes the command. * - * Typo suggestion ("did you mean --host?", Requirement 10) is task 1.2, - * not implemented by this function yet. + * Typo suggestion (Requirement 10): when `bad_flag` matches no entry in + * `spec` (the "unrecognized flag" case above) and exactly one entry's + * short or long spelling closely matches it by edit distance (see + * flagspec.c's is_close_match()), the rejection message names that flag + * as the likely intended one (Requirement 10.1). When zero or more than + * one entry qualifies as a close match, no guess is made (Requirement + * 10.2) -- an ambiguous typo is treated the same as a typo that matches + * nothing at all. Suggestion is never attempted for the "requires a + * value"/"does not take a value" cases, since those already identify the + * exact intended flag. * * `usage_text` is expected to be the command's one-line usage summary * (e.g. "jobkill -j [-s ]"), not its full help block; it @@ -128,3 +138,17 @@ int flagspec_build_opts(const flagspec_t* spec, size_t n, * case. */ int flagspec_reject(const char* cmd_name, const flagspec_t* spec, size_t n, const char* bad_flag, const char* usage_text); + +/* Renders the flag-listing portion of a command's help text (Requirement + * 8.2) directly from `spec`: one line per flag, each stating its short + * form (when it has one), its long form, whether it is required or + * optional, and its default-value description (when `default_desc` is + * non-NULL). This is the flag-block ONLY -- the surrounding description, + * example, and related-command prose (Requirement 8.1/8.3/8.4) stay + * hand-authored, per design.md's "Flag-descriptor table as the single + * source of truth" decision. + * + * Returns a heap-allocated, NUL-terminated string the caller must free(). + * Returns NULL if `spec` is NULL or n == 0, or if the underlying + * allocation fails; it never returns a partially-written buffer. */ +char* flagspec_render_help(const flagspec_t* spec, size_t n); diff --git a/src/controller/testsuite/test_flagspec.c b/src/controller/testsuite/test_flagspec.c index d2844b2d..cb3189e7 100644 --- a/src/controller/testsuite/test_flagspec.c +++ b/src/controller/testsuite/test_flagspec.c @@ -375,6 +375,116 @@ static void test_reject_no_argument_flag_given_a_value_reports_distinctly(void) cleanup_dir(); } +/* ---- flagspec_reject() typo-suggestion tests (Requirement 10) --------- + * A second fixture, g_ambiguous_spec, is purpose-built for the + * ambiguous-match case below so its edit distances are deliberate rather + * than incidental to g_testcmd_spec's two very dissimilar flag names. */ + +static const flagspec_t g_ambiguous_spec[2] = { + { 0, "targeta", "target-a", no_argument, 0, NULL }, + { 0, "targetb", "target-b", no_argument, 0, NULL }, +}; + +static void test_reject_suggests_single_close_match_flag_typo(void) +{ + /* "--hst" is "--host" with the 'o' deleted -- one edit, well inside + * the closeness threshold for a 6-character long-form spelling -- and + * is nowhere near "--command". Requirement 10.1: name the + * likely-intended flag in the rejection message. */ + fresh_dir(); + capture_t cap; + capture_begin(&cap); + int rc = flagspec_reject("testcmd", g_testcmd_spec, 2, "--hst", + g_testcmd_usage); + char out_buf[512], err_buf[512]; + capture_end(&cap, out_buf, sizeof(out_buf), err_buf, sizeof(err_buf)); + + TEST_ASSERT_TRUE(rc < 0); + TEST_ASSERT_EQUAL_STRING("", out_buf); + TEST_ASSERT_NOT_NULL(strstr(err_buf, "unrecognized flag '--hst'")); + TEST_ASSERT_NOT_NULL(strstr(err_buf, "did you mean")); + TEST_ASSERT_NOT_NULL(strstr(err_buf, "--host")); + cleanup_dir(); +} + +static void test_reject_gives_no_suggestion_for_a_distant_typo(void) +{ + /* "--wombat" does not closely resemble "--host" or "--command" by any + * reasonable edit-distance threshold. Requirement 10.2: report + * unrecognized without guessing. */ + fresh_dir(); + capture_t cap; + capture_begin(&cap); + int rc = flagspec_reject("testcmd", g_testcmd_spec, 2, "--wombat", + g_testcmd_usage); + char out_buf[512], err_buf[512]; + capture_end(&cap, out_buf, sizeof(out_buf), err_buf, sizeof(err_buf)); + + TEST_ASSERT_TRUE(rc < 0); + TEST_ASSERT_EQUAL_STRING("", out_buf); + TEST_ASSERT_NOT_NULL(strstr(err_buf, "unrecognized flag '--wombat'")); + TEST_ASSERT_NULL(strstr(err_buf, "did you mean")); + cleanup_dir(); +} + +static void test_reject_gives_no_suggestion_when_typo_is_ambiguous(void) +{ + /* "--targetc" is exactly one substitution away from BOTH "--targeta" + * and "--targetb". Requirement 10.1 only promises a guess when + * exactly one flag qualifies -- two equally-plausible candidates must + * produce no suggestion at all, the same as a typo that matches + * nothing. */ + fresh_dir(); + capture_t cap; + capture_begin(&cap); + int rc = flagspec_reject("testcmd", g_ambiguous_spec, 2, "--targetc", + "testcmd --targeta|--targetb"); + char out_buf[512], err_buf[512]; + capture_end(&cap, out_buf, sizeof(out_buf), err_buf, sizeof(err_buf)); + + TEST_ASSERT_TRUE(rc < 0); + TEST_ASSERT_EQUAL_STRING("", out_buf); + TEST_ASSERT_NOT_NULL(strstr(err_buf, "unrecognized flag '--targetc'")); + TEST_ASSERT_NULL(strstr(err_buf, "did you mean")); + cleanup_dir(); +} + +/* ---- flagspec_render_help() tests (Requirement 8.2) -------------------- */ + +static void test_render_help_formats_multi_flag_table(void) +{ + flagspec_t spec[3]; + memcpy(spec, g_testcmd_spec, sizeof(g_testcmd_spec)); + spec[2] = (flagspec_t){ 0, "json", "output-format", no_argument, 0, + "table" }; + + char* help = flagspec_render_help(spec, 3); + TEST_ASSERT_NOT_NULL(help); + + /* required flag: short + long form, marked required */ + TEST_ASSERT_NOT_NULL(strstr(help, "-h")); + TEST_ASSERT_NOT_NULL(strstr(help, "--host")); + TEST_ASSERT_NOT_NULL(strstr(help, "required")); + + /* optional flag: short + long form, marked optional, default shown */ + TEST_ASSERT_NOT_NULL(strstr(help, "-c")); + TEST_ASSERT_NOT_NULL(strstr(help, "--command")); + TEST_ASSERT_NOT_NULL(strstr(help, "optional")); + TEST_ASSERT_NOT_NULL(strstr(help, "off")); + + /* long-only flag: no short form, still shows long form + default */ + TEST_ASSERT_NOT_NULL(strstr(help, "--json")); + TEST_ASSERT_NOT_NULL(strstr(help, "table")); + + free(help); +} + +static void test_render_help_rejects_null_and_empty_inputs(void) +{ + TEST_ASSERT_NULL(flagspec_render_help(NULL, 2)); + TEST_ASSERT_NULL(flagspec_render_help(g_testcmd_spec, 0)); +} + void test_flagspec(void) { UnitySetTestFile(__FILE__); @@ -390,4 +500,9 @@ void test_flagspec(void) RUN_TEST(test_reject_rejects_missing_value_short_flag); RUN_TEST(test_reject_handles_null_bad_flag_and_null_usage_gracefully); RUN_TEST(test_reject_no_argument_flag_given_a_value_reports_distinctly); + RUN_TEST(test_reject_suggests_single_close_match_flag_typo); + RUN_TEST(test_reject_gives_no_suggestion_for_a_distant_typo); + RUN_TEST(test_reject_gives_no_suggestion_when_typo_is_ambiguous); + RUN_TEST(test_render_help_formats_multi_flag_table); + RUN_TEST(test_render_help_rejects_null_and_empty_inputs); }