From cb0df397aae552adc80713ca0d59ed1ebfd3b1be Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Tue, 8 Feb 2022 10:11:49 +0100 Subject: [PATCH] rbd: mark optional positional arguments as such in help output Currently at least five commands have optional positional arguments. Overloading po::value()->default_value("") for this is a bit sneaky but nothing better fits into the existing Shell.cc framework. Note that strictly speaking "[] []" should be "[ []]" but we aren't doing that here because "ceph" command doesn't do it either. Fixes: https://tracker.ceph.com/issues/54191 Signed-off-by: Ilya Dryomov --- src/test/cli/rbd/help.t | 10 +++++----- src/tools/rbd/OptionPrinter.cc | 13 +++++++++++++ src/tools/rbd/Schedule.cc | 16 ++++++++++++---- src/tools/rbd/Schedule.h | 2 +- src/tools/rbd/action/MirrorImage.cc | 3 ++- src/tools/rbd/action/MirrorSnapshotSchedule.cc | 4 ++-- src/tools/rbd/action/TrashPurgeSchedule.cc | 4 ++-- 7 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/test/cli/rbd/help.t b/src/test/cli/rbd/help.t index 045da189c5247..c5090f3ba8a6b 100644 --- a/src/test/cli/rbd/help.t +++ b/src/test/cli/rbd/help.t @@ -1665,7 +1665,7 @@ rbd help mirror image enable usage: rbd mirror image enable [--pool ] [--namespace ] [--image ] - + [] Enable RBD mirroring for an image. @@ -1943,7 +1943,7 @@ [--pool ] [--namespace ] [--image ] - + [] Add mirror snapshot schedule. @@ -1978,7 +1978,7 @@ [--pool ] [--namespace ] [--image ] - + [] [] Remove mirror snapshot schedule. @@ -2514,7 +2514,7 @@ rbd help trash purge schedule add usage: rbd trash purge schedule add [--pool ] [--namespace ] - + [] Add trash purge schedule. @@ -2543,7 +2543,7 @@ rbd help trash purge schedule remove usage: rbd trash purge schedule remove [--pool ] [--namespace ] - + [] [] Remove trash purge schedule. diff --git a/src/tools/rbd/OptionPrinter.cc b/src/tools/rbd/OptionPrinter.cc index 5a5bf9504989c..0fea6b691f1bd 100644 --- a/src/tools/rbd/OptionPrinter.cc +++ b/src/tools/rbd/OptionPrinter.cc @@ -3,6 +3,7 @@ #include "tools/rbd/OptionPrinter.h" #include "tools/rbd/IndentStream.h" +#include "include/ceph_assert.h" namespace rbd { @@ -43,10 +44,22 @@ void OptionPrinter::print_short(std::ostream &os, size_t initial_offset) { for (size_t i = 0; i < m_positional.options().size(); ++i) { std::stringstream option; + // we overload po::value()->default_value("") to signify + // an optional positional argument (purely for help printing purposes) + boost::any v; + bool required = !m_positional.options()[i]->semantic()->apply_default(v); + if (!required) { + auto ptr = boost::any_cast(&v); + ceph_assert(ptr && ptr->empty()); + option << "["; + } option << "<" << m_positional.options()[i]->long_name() << ">"; if (m_positional.options()[i]->semantic()->max_tokens() > 1) { option << " [<" << m_positional.options()[i]->long_name() << "> ...]"; } + if (!required) { + option << "]"; + } max_option_width = std::max(max_option_width, option.str().size()); positionals.emplace_back(option.str()); diff --git a/src/tools/rbd/Schedule.cc b/src/tools/rbd/Schedule.cc index d7fcf1253deb6..15dda3aee7ef9 100644 --- a/src/tools/rbd/Schedule.cc +++ b/src/tools/rbd/Schedule.cc @@ -152,11 +152,19 @@ void normalize_level_spec_args(std::map *args) { } } -void add_schedule_options(po::options_description *positional) { - positional->add_options() - ("interval", "schedule interval"); +void add_schedule_options(po::options_description *positional, + bool mandatory) { + if (mandatory) { + positional->add_options() + ("interval", "schedule interval"); + } else { + positional->add_options() + ("interval", po::value()->default_value(""), + "schedule interval"); + } positional->add_options() - ("start-time", "schedule start time"); + ("start-time", po::value()->default_value(""), + "schedule start time"); } int get_schedule_args(const po::variables_map &vm, bool mandatory, diff --git a/src/tools/rbd/Schedule.h b/src/tools/rbd/Schedule.h index 90e78eb7f682c..bf0964bb1e51b 100644 --- a/src/tools/rbd/Schedule.h +++ b/src/tools/rbd/Schedule.h @@ -23,7 +23,7 @@ int get_level_spec_args(const boost::program_options::variables_map &vm, void normalize_level_spec_args(std::map *args); void add_schedule_options( - boost::program_options::options_description *positional); + boost::program_options::options_description *positional, bool mandatory); int get_schedule_args(const boost::program_options::variables_map &vm, bool mandatory, std::map *args); diff --git a/src/tools/rbd/action/MirrorImage.cc b/src/tools/rbd/action/MirrorImage.cc index ab037b296e306..505d377f4807f 100644 --- a/src/tools/rbd/action/MirrorImage.cc +++ b/src/tools/rbd/action/MirrorImage.cc @@ -77,7 +77,8 @@ void get_arguments_enable(po::options_description *positional, po::options_description *options) { at::add_image_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE); positional->add_options() - ("mode", "mirror image mode (journal or snapshot) [default: journal]"); + ("mode", po::value()->default_value(""), + "mirror image mode (journal or snapshot) [default: journal]"); } void get_arguments_disable(po::options_description *positional, diff --git a/src/tools/rbd/action/MirrorSnapshotSchedule.cc b/src/tools/rbd/action/MirrorSnapshotSchedule.cc index af684944017d1..3f269c2ad8717 100644 --- a/src/tools/rbd/action/MirrorSnapshotSchedule.cc +++ b/src/tools/rbd/action/MirrorSnapshotSchedule.cc @@ -121,7 +121,7 @@ std::ostream& operator<<(std::ostream& os, ScheduleStatus &s) { void get_arguments_add(po::options_description *positional, po::options_description *options) { add_level_spec_options(options); - add_schedule_options(positional); + add_schedule_options(positional, true); } int execute_add(const po::variables_map &vm, @@ -156,7 +156,7 @@ int execute_add(const po::variables_map &vm, void get_arguments_remove(po::options_description *positional, po::options_description *options) { add_level_spec_options(options); - add_schedule_options(positional); + add_schedule_options(positional, false); } int execute_remove(const po::variables_map &vm, diff --git a/src/tools/rbd/action/TrashPurgeSchedule.cc b/src/tools/rbd/action/TrashPurgeSchedule.cc index 0f92e762bc2fd..c07ff8a0da02a 100644 --- a/src/tools/rbd/action/TrashPurgeSchedule.cc +++ b/src/tools/rbd/action/TrashPurgeSchedule.cc @@ -151,7 +151,7 @@ std::ostream& operator<<(std::ostream& os, ScheduleStatus &s) { void get_arguments_add(po::options_description *positional, po::options_description *options) { add_level_spec_options(options, false); - add_schedule_options(positional); + add_schedule_options(positional, true); } int execute_add(const po::variables_map &vm, @@ -186,7 +186,7 @@ int execute_add(const po::variables_map &vm, void get_arguments_remove(po::options_description *positional, po::options_description *options) { add_level_spec_options(options, false); - add_schedule_options(positional); + add_schedule_options(positional, false); } int execute_remove(const po::variables_map &vm, -- 2.39.5