feat(xce-test-cli-ux): add flag typo-suggestion and help-block renderer
flagspec_reject() now names a likely-intended flag when an unrecognized flag closely matches exactly one entry in the descriptor table (genuine Levenshtein DP, silent on zero or ambiguous matches). flagspec_render_help() generates the short/long/required-optional/default flag-listing block from a descriptor table. 1278/1278 controller suite passing (+5). Task 1.2 of .kiro/specs/xce-test-cli-ux/tasks.md. Requirements: 6.1, 6.2, 6.3, 6.4, 8.2, 10.1, 10.2, 10.3 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
This commit is contained in:
parent
0748f10cc6
commit
89af46af3e
4 changed files with 335 additions and 17 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
|
||||
#include <errno.h>
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
/* 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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 <getopt.h>
|
||||
|
|
@ -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 <jobid> [-s <signo>]"), 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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue