From ffdf4978604672ee7b327a5f65d821daeac26149 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 20 Sep 2024 09:09:28 +0900 Subject: [PATCH 1/2] strv: introduce strv_find_closest() Follow-up for 1e1ac5d53b0f126b6c4419506c7c42b67c07537f. --- src/basic/strv.c | 37 ++++++++++++++++++++++++++++++++++++- src/basic/strv.h | 2 +- src/firstboot/firstboot.c | 10 ++-------- src/test/test-strv.c | 27 +++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 10 deletions(-) diff --git a/src/basic/strv.c b/src/basic/strv.c index 5b71c36dbf..414123e558 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -66,7 +66,28 @@ char* strv_find_startswith(char * const *l, const char *name) { return NULL; } -char* strv_find_closest_by_levenshtein(char * const *l, const char *name) { +static char* strv_find_closest_prefix(char * const *l, const char *name) { + size_t best_distance = SIZE_MAX; + char *best = NULL; + + assert(name); + + STRV_FOREACH(s, l) { + char *e = startswith(*s, name); + if (!e) + continue; + + size_t n = strlen(e); + if (n < best_distance) { + best_distance = n; + best = *s; + } + } + + return best; +} + +static char* strv_find_closest_by_levenshtein(char * const *l, const char *name) { ssize_t best_distance = SSIZE_MAX; char *best = NULL; @@ -93,6 +114,20 @@ char* strv_find_closest_by_levenshtein(char * const *l, const char *name) { return best; } +char* strv_find_closest(char * const *l, const char *name) { + assert(name); + + /* Be more helpful to the user, and give a hint what the user might have wanted to type. We search + * with two mechanisms: a simple prefix match and – if that didn't yield results –, a Levenshtein + * word distance based match. */ + + char *found = strv_find_closest_prefix(l, name); + if (found) + return found; + + return strv_find_closest_by_levenshtein(l, name); +} + char* strv_find_first_field(char * const *needles, char * const *haystack) { STRV_FOREACH(k, needles) { char *value = strv_env_pairs_get((char **)haystack, *k); diff --git a/src/basic/strv.h b/src/basic/strv.h index b7845eeecc..1afb1bf659 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -17,7 +17,7 @@ char* strv_find(char * const *l, const char *name) _pure_; char* strv_find_case(char * const *l, const char *name) _pure_; char* strv_find_prefix(char * const *l, const char *name) _pure_; char* strv_find_startswith(char * const *l, const char *name) _pure_; -char* strv_find_closest_by_levenshtein(char * const *l, const char *name) _pure_; +char* strv_find_closest(char * const *l, const char *name) _pure_; /* Given two vectors, the first a list of keys and the second a list of key-value pairs, returns the value * of the first key from the first vector that is found in the second vector. */ char* strv_find_first_field(char * const *needles, char * const *haystack) _pure_; diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index 3899835d4c..236c64634c 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -187,7 +187,6 @@ static int prompt_loop(const char *text, char **l, unsigned percentage, bool (*i for (;;) { _cleanup_free_ char *p = NULL; - char *best_match = NULL; unsigned u; r = ask_string(&p, "%s %s (empty to skip, \"list\" to list options): ", @@ -223,13 +222,8 @@ static int prompt_loop(const char *text, char **l, unsigned percentage, bool (*i if (is_valid(p)) return free_and_replace(*ret, p); - /* Be helperful to the user, and give a hint what the user might have wanted to - * type. We search with two mechanisms: a simple prefix match and – if that didn't - * yield results –, a Levenshtein word distance based match. */ - best_match = strv_find_prefix(l, p); - if (!best_match) - best_match = strv_find_closest_by_levenshtein(l, p); - + /* Be more helpful to the user, and give a hint what the user might have wanted to type. */ + const char *best_match = strv_find_closest(l, p); if (best_match) log_error("Invalid data '%s', did you mean '%s'?", p, best_match); else diff --git a/src/test/test-strv.c b/src/test/test-strv.c index f7b6ee2bb2..7c25d59bf4 100644 --- a/src/test/test-strv.c +++ b/src/test/test-strv.c @@ -1188,4 +1188,31 @@ TEST(strv_rebreak_lines) { } } +TEST(strv_find_closest) { + char **l = STRV_MAKE("aaa", "aaaa", "bbb", "ccc"); + + /* prefix match */ + ASSERT_STREQ(strv_find_closest(l, "a"), "aaa"); + ASSERT_STREQ(strv_find_closest(l, "aa"), "aaa"); + ASSERT_STREQ(strv_find_closest(l, "aaa"), "aaa"); + ASSERT_STREQ(strv_find_closest(l, "aaaa"), "aaaa"); + ASSERT_STREQ(strv_find_closest(l, "b"), "bbb"); + ASSERT_STREQ(strv_find_closest(l, "bb"), "bbb"); + ASSERT_STREQ(strv_find_closest(l, "bbb"), "bbb"); + ASSERT_STREQ(strv_find_closest(l, "c"), "ccc"); + ASSERT_STREQ(strv_find_closest(l, "cc"), "ccc"); + ASSERT_STREQ(strv_find_closest(l, "ccc"), "ccc"); + + /* levenshtein match */ + ASSERT_STREQ(strv_find_closest(l, "aab"), "aaa"); + ASSERT_STREQ(strv_find_closest(l, "abb"), "bbb"); + ASSERT_STREQ(strv_find_closest(l, "cbc"), "ccc"); + ASSERT_STREQ(strv_find_closest(l, "aax"), "aaa"); + ASSERT_STREQ(strv_find_closest(l, "bbbb"), "bbb"); + ASSERT_STREQ(strv_find_closest(l, "cbbb"), "bbb"); + ASSERT_STREQ(strv_find_closest(l, "bbbx"), "bbb"); + + ASSERT_NULL(strv_find_closest(l, "sfajosajfosdjaofjdsaf")); +} + DEFINE_TEST_MAIN(LOG_INFO); From 07919a18571e90ff6b59bc33cdedd5cdde5f5a3d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 20 Sep 2024 09:31:46 +0900 Subject: [PATCH 2/2] verbs: use strv_find_closest() This also makes the list of verbs is always shown on failure. --- src/shared/verbs.c | 95 ++++++++++--------------------------------- src/test/test-verbs.c | 22 ++++++++++ 2 files changed, 43 insertions(+), 74 deletions(-) diff --git a/src/shared/verbs.c b/src/shared/verbs.c index 7e7ac2a255..8442963b37 100644 --- a/src/shared/verbs.c +++ b/src/shared/verbs.c @@ -10,6 +10,7 @@ #include "macro.h" #include "process-util.h" #include "string-util.h" +#include "strv.h" #include "verbs.h" #include "virt.h" @@ -54,70 +55,14 @@ const Verb* verbs_find_verb(const char *name, const Verb verbs[]) { return NULL; } -static const Verb* verbs_find_prefix_verb(const char *name, const Verb verbs[]) { - size_t best_distance = SIZE_MAX; - const Verb *best = NULL; - - assert(verbs); - - if (!name) - return NULL; - - for (size_t i = 0; verbs[i].dispatch; i++) { - const char *e; - size_t l; - - e = startswith(verbs[i].verb, name); - if (!e) - continue; - - l = strlen(e); - if (l < best_distance) { - best_distance = l; - best = verbs + i; - } - } - - return best; -} - -static const Verb* verbs_find_closest_verb(const char *name, const Verb verbs[]) { - ssize_t best_distance = SSIZE_MAX; - const Verb *best = NULL; - - assert(verbs); - - if (!name) - return NULL; - - for (size_t i = 0; verbs[i].dispatch; i++) { - ssize_t distance; - - distance = strlevenshtein(verbs[i].verb, name); - if (distance < 0) { - log_debug_errno(distance, "Failed to determine Levenshtein distance between %s and %s: %m", verbs[i].verb, name); - return NULL; - } - - if (distance > 5) /* If the distance is just too far off, don't make a bad suggestion */ - continue; - - if (distance < best_distance) { - best_distance = distance; - best = verbs + i; - } - } - - return best; -} - int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { const Verb *verb; const char *name; - int left; + int r, left; assert(verbs); assert(verbs[0].dispatch); + assert(verbs[0].verb); assert(argc >= 0); assert(argv); assert(argc >= optind); @@ -129,32 +74,34 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { verb = verbs_find_verb(name, verbs); if (!verb) { + _cleanup_strv_free_ char **verb_strv = NULL; + + for (size_t i = 0; verbs[i].dispatch; i++) { + r = strv_extend(&verb_strv, verbs[i].verb); + if (r < 0) + return log_oom(); + } + if (name) { - /* Be helperful to the user, and give a hint what the user might have wanted to - * type. We search with two mechanisms: a simple prefix match and – if that didn't - * yield results –, a Levenshtein word distance based match. */ - verb = verbs_find_prefix_verb(name, verbs); - if (!verb) - verb = verbs_find_closest_verb(name, verbs); - if (verb) + /* Be more helpful to the user, and give a hint what the user might have wanted to type. */ + const char *found = strv_find_closest(verb_strv, name); + if (found) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Unknown command verb '%s', did you mean '%s'?", name, verb->verb); + "Unknown command verb '%s', did you mean '%s'?", name, found); return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown command verb '%s'.", name); } - _cleanup_free_ char *verb_list = NULL; - size_t i; - - for (i = 0; verbs[i].dispatch; i++) - if (!strextend_with_separator(&verb_list, ", ", verbs[i].verb)) + if (strv_length(verb_strv) >= 2) { + _cleanup_free_ char *joined = strv_join(verb_strv, ", "); + if (!joined) return log_oom(); - if (i > 2) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Command verb required (one of %s).", verb_list); + "Command verb required (one of %s).", joined); + } - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Command verb required."); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Command verb '%s' required.", verbs[0].verb); } if (!name) diff --git a/src/test/test-verbs.c b/src/test/test-verbs.c index 6e30794fa3..a033d07ee7 100644 --- a/src/test/test-verbs.c +++ b/src/test/test-verbs.c @@ -54,6 +54,28 @@ TEST(verbs_no_default) { }; test_dispatch_one(STRV_MAKE(NULL), verbs, -EINVAL); + test_dispatch_one(STRV_MAKE("hel"), verbs, -EINVAL); + test_dispatch_one(STRV_MAKE("helpp"), verbs, -EINVAL); + test_dispatch_one(STRV_MAKE("hgrejgoraoiosafso"), verbs, -EINVAL); +} + +TEST(verbs_no_default_many) { + static const Verb verbs[] = { + { "help", VERB_ANY, VERB_ANY, 0, noop_dispatcher }, + { "list-images", VERB_ANY, 1, 0, noop_dispatcher }, + { "list", VERB_ANY, 2, 0, noop_dispatcher }, + { "status", 2, VERB_ANY, 0, noop_dispatcher }, + { "show", VERB_ANY, VERB_ANY, 0, noop_dispatcher }, + { "terminate", 2, VERB_ANY, 0, noop_dispatcher }, + { "login", 2, 2, 0, noop_dispatcher }, + { "copy-to", 3, 4, 0, noop_dispatcher }, + {} + }; + + test_dispatch_one(STRV_MAKE(NULL), verbs, -EINVAL); + test_dispatch_one(STRV_MAKE("hel"), verbs, -EINVAL); + test_dispatch_one(STRV_MAKE("helpp"), verbs, -EINVAL); + test_dispatch_one(STRV_MAKE("hgrejgoraoiosafso"), verbs, -EINVAL); } DEFINE_TEST_MAIN(LOG_INFO);