From: Zbigniew Jędrzejewski-Szmek Date: Tue, 28 Apr 2026 09:12:43 +0000 (+0200) Subject: shared/options: fix --help indentation for long options X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d0f726ed8f9f62c0fef14723aab241ff7b23e9ef;p=thirdparty%2Fsystemd.git shared/options: fix --help indentation for long options In 4339197f5d4f712bc900d8e09c892015d48b19bb the helper to format -o/--opt= was split out, but the indentation was for --long-options was messed up. We'd print: Options: -h --help Show this help --version Show package version --no-ask-password Do not prompt for password ... But we want -h --help Show this help --version Show package version --no-ask-password Do not prompt for password ... The prefix argument was arguably ugly, even if it allowed one alloc to be avoided. Let's get rid of it and let the handler prefix the string as appropriate. This makes other callers nicer too. Fixup for 4339197f5d4f712bc900d8e09c892015d48b19bb. --- diff --git a/src/shared/options.c b/src/shared/options.c index 644c7ae27c3..74f697edbe9 100644 --- a/src/shared/options.c +++ b/src/shared/options.c @@ -306,18 +306,15 @@ size_t option_parser_get_n_args(const OptionParser *state) { return state->argc - state->positional_offset; } -char* option_get_synopsis(const char *prefix, const Option *opt, const char *joiner, bool show_metavar) { +char* option_get_synopsis(const Option *opt, const char *joiner, bool show_metavar) { assert(opt); assert(!FLAGS_SET(opt->flags, OPTION_GROUP_MARKER)); /* A group marker should not be displayed */ - if (!prefix) - prefix = ""; - if (opt->flags & (OPTION_HELP_ENTRY_VERBATIM | OPTION_POSITIONAL_ENTRY)) - return strjoin(prefix, ASSERT_PTR(opt->long_code)); + return strdup(ASSERT_PTR(opt->long_code)); /* The option formatted appropriately for --help strings, error messages, and similar: - * ---=[] + * ---=[] * "=" is shown only when a long form is defined: -l --long=ARG, --long=ARG, -s ARG. * The joiner arg is used between the short and long forms. * As a special case, if the option has no long form and show_metavar is true, @@ -338,16 +335,14 @@ char* option_get_synopsis(const char *prefix, const Option *opt, const char *joi bool need_eq = option_takes_arg(opt) && opt->long_code; if (!show_metavar) - return strjoin(prefix, - sc, + return strjoin(sc, joiner, opt->long_code ? "--" : "", strempty(opt->long_code), need_eq ? "=" : ""); bool need_quote = opt->metavar && strchr(opt->metavar, ' '); - return strjoin(prefix, - sc, + return strjoin(sc, joiner, opt->long_code ? "--" : "", strempty(opt->long_code), @@ -389,22 +384,27 @@ int _option_parser_get_help_table( /* No help string — we do not show the option */ continue; + _cleanup_free_ char *s = option_get_synopsis(opt, " ", /* show_metavar= */ true); + if (!s) + return log_oom(); + /* We indent the option string by two spaces. We could set the minimum cell width and * right-align for a similar result, but that'd be more work. This is only used for * display. */ - _cleanup_free_ char *s = option_get_synopsis(" ", opt, " ", /* show_metavar= */ true); - if (!s) + const char *prefix = opt->short_code != 0 ? " " : " "; + _cleanup_free_ char *t = strjoin(prefix, s); + if (!t) return log_oom(); - r = table_add_many(table, TABLE_STRING, s); + r = table_add_many(table, TABLE_STRING, t); if (r < 0) return table_log_add_error(r); - _cleanup_strv_free_ char **t = strv_split(opt->help, /* separators= */ NULL); - if (!t) + _cleanup_strv_free_ char **split = strv_split(opt->help, /* separators= */ NULL); + if (!split) return log_oom(); - r = table_add_many(table, TABLE_STRV_WRAPPED, t); + r = table_add_many(table, TABLE_STRV_WRAPPED, split); if (r < 0) return table_log_add_error(r); } diff --git a/src/shared/options.h b/src/shared/options.h index 3d08390b33e..f9913951f42 100644 --- a/src/shared/options.h +++ b/src/shared/options.h @@ -191,7 +191,7 @@ char* option_parser_consume_next_arg(OptionParser *state); char** option_parser_get_args(const OptionParser *state); size_t option_parser_get_n_args(const OptionParser *state); -char* option_get_synopsis(const char *prefix, const Option *opt, const char *joiner, bool show_metavar); +char* option_get_synopsis(const Option *opt, const char *joiner, bool show_metavar); int _option_parser_get_help_table( const Option options[], diff --git a/src/test/test-options.c b/src/test/test-options.c index 6a0744e6736..4bdec73d401 100644 --- a/src/test/test-options.c +++ b/src/test/test-options.c @@ -1306,47 +1306,47 @@ static void test_option_get_synopsis_one( bool show_metavar, const char *expected) { log_debug("%s", expected); - _cleanup_free_ char *s = option_get_synopsis(". ", opt, joiner, show_metavar); + _cleanup_free_ char *s = option_get_synopsis(opt, joiner, show_metavar); ASSERT_STREQ(s, expected); } TEST(option_get_synopsis) { - test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "X" }, "/", true, ". -x/--xxx=X"); - test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "X" }, NULL, true, ". -x --xxx=X"); - test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "X" }, "/", false, ". -x/--xxx=" ); - test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "X" }, " ", true, ". -x --xxx=X"); - test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "X" }, " ", false, ". -x --xxx=" ); - test_option_get_synopsis_one(&(const Option) { 0, 0, 0, "xxx", "X" }, "+", true, ". --xxx=X" ); - test_option_get_synopsis_one(&(const Option) { 0, 0, 0, "xxx", "X" }, "+", false, ". --xxx=" ); - test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', NULL, "X" }, " ", true, ". -x X" ); - test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', NULL, "X" }, "/", false, ". -x" ); - - test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "A B" }, "/", true, ". -x/--xxx='A B'"); - test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "A B" }, " ", true, ". -x --xxx='A B'"); - test_option_get_synopsis_one(&(const Option) { 0, 0, 0, "xxx", "A B" }, "+", true, ". --xxx='A B'" ); - test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', NULL, "A B" }, " ", true, ". -x 'A B'" ); - - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "X" }, "/", true, ". -x/--xxx[=X]"); - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "X" }, NULL, true, ". -x --xxx[=X]"); + test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "X" }, "/", true, "-x/--xxx=X"); + test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "X" }, NULL, true, "-x --xxx=X"); + test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "X" }, "/", false, "-x/--xxx=" ); + test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "X" }, " ", true, "-x --xxx=X"); + test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "X" }, " ", false, "-x --xxx=" ); + test_option_get_synopsis_one(&(const Option) { 0, 0, 0, "xxx", "X" }, "+", true, "--xxx=X" ); + test_option_get_synopsis_one(&(const Option) { 0, 0, 0, "xxx", "X" }, "+", false, "--xxx=" ); + test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', NULL, "X" }, " ", true, "-x X" ); + test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', NULL, "X" }, "/", false, "-x" ); + + test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "A B" }, "/", true, "-x/--xxx='A B'"); + test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', "xxx", "A B" }, " ", true, "-x --xxx='A B'"); + test_option_get_synopsis_one(&(const Option) { 0, 0, 0, "xxx", "A B" }, "+", true, "--xxx='A B'" ); + test_option_get_synopsis_one(&(const Option) { 0, 0, 'x', NULL, "A B" }, " ", true, "-x 'A B'" ); + + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "X" }, "/", true, "-x/--xxx[=X]"); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "X" }, NULL, true, "-x --xxx[=X]"); /* Note: --xxx[=] would be silly, so we show --xxx=. It's a corner case. Maybe this should change. */ - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "X" }, "/", false, ". -x/--xxx=" ); - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "X" }, " ", true, ". -x --xxx[=X]"); - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "X" }, " ", false, ". -x --xxx=" ); - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 0, "xxx", "X" }, "+", true, ". --xxx[=X]" ); - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 0, "xxx", "X" }, "+", false, ". --xxx=" ); - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', NULL, "X" }, " ", true, ". -x [X]" ); - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', NULL, "X" }, "/", false, ". -x" ); - - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "A B" }, "/", true, ". -x/--xxx[='A B']"); - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "A B" }, " ", true, ". -x --xxx[='A B']"); - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 0, "xxx", "A B" }, "+", true, ". --xxx[='A B']" ); - test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', NULL, "A B" }, " ", true, ". -x ['A B']" ); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "X" }, "/", false, "-x/--xxx=" ); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "X" }, " ", true, "-x --xxx[=X]"); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "X" }, " ", false, "-x --xxx=" ); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 0, "xxx", "X" }, "+", true, "--xxx[=X]" ); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 0, "xxx", "X" }, "+", false, "--xxx=" ); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', NULL, "X" }, " ", true, "-x [X]" ); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', NULL, "X" }, "/", false, "-x" ); + + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "A B" }, "/", true, "-x/--xxx[='A B']"); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', "xxx", "A B" }, " ", true, "-x --xxx[='A B']"); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 0, "xxx", "A B" }, "+", true, "--xxx[='A B']" ); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG, 'x', NULL, "A B" }, " ", true, "-x ['A B']" ); test_option_get_synopsis_one(&(const Option) { 0, OPTION_OPTIONAL_ARG | OPTION_HELP_ENTRY | OPTION_STOPS_PARSING, - 'x', "xxx", "A B" }, "/", true, ". -x/--xxx[='A B']"); + 'x', "xxx", "A B" }, "/", true, "-x/--xxx[='A B']"); - test_option_get_synopsis_one(&(const Option) { 0, OPTION_HELP_ENTRY_VERBATIM, 'u', "special special", "unused" }, "/", true, ". special special"); - test_option_get_synopsis_one(&(const Option) { 0, OPTION_POSITIONAL_ENTRY, 'u', "(fixed)", "unused" }, "/", true, ". (fixed)"); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_HELP_ENTRY_VERBATIM, 'u', "special special", "unused" }, "/", true, "special special"); + test_option_get_synopsis_one(&(const Option) { 0, OPTION_POSITIONAL_ENTRY, 'u', "(fixed)", "unused" }, "/", true, "(fixed)"); } DEFINE_TEST_MAIN(LOG_DEBUG); diff --git a/src/udev/scsi_id/scsi_id.c b/src/udev/scsi_id/scsi_id.c index 41140db8162..f272648c420 100644 --- a/src/udev/scsi_id/scsi_id.c +++ b/src/udev/scsi_id/scsi_id.c @@ -331,7 +331,7 @@ static int per_dev_options(struct scsi_id_device *dev_scsi, int *good_bad, enum } else return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Option %s not supported in the config file.", - strnull(option_get_synopsis("", opts.opt, "/", /* show_metavar=*/ false))); + strnull(option_get_synopsis(opts.opt, "/", /* show_metavar=*/ false))); return 0; }