SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

shared/options: fix --help indentation for long options

Browse Source 
In 4339197f5d 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 4339197f5d.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2026-04-28 15:44:28 +02:00
committed by Zbigniew Jędrzejewski-Szmek
parent 37c272228d
commit d0f726ed8f
4 changed files with 52 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/shared/options.c
+20 -20
View File
@@ -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:
* <prefix>-<short><joiner>--<long>=[<metavar>]
* -<short><joiner>--<long>=[<metavar>]
* "=" 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;
/* 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);
_cleanup_free_ char *s = option_get_synopsis(opt, " ", /* show_metavar= */ true);
if (!s)
return log_oom();
r = table_add_many(table, TABLE_STRING, s);
if (r < 0)
return table_log_add_error(r);
_cleanup_strv_free_ char **t = strv_split(opt->help, /* separators= */ NULL);
/* 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. */
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_STRV_WRAPPED, t);
r = table_add_many(table, TABLE_STRING, t);
if (r < 0)
return table_log_add_error(r);
_cleanup_strv_free_ char **split = strv_split(opt->help, /* separators= */ NULL);
if (!split)
return log_oom();
r = table_add_many(table, TABLE_STRV_WRAPPED, split);
if (r < 0)
return table_log_add_error(r);
}
src/shared/options.h
+1 -1
View File
@@ -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[],
src/test/test-options.c
+30 -30
View File
@@ -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", "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, 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, 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", "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", "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);
src/udev/scsi_id/scsi_id.c
+1 -1
View File
@@ -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;
}