Skip to content

Commit

Permalink
rbd: mark optional positional arguments as such in help output
Browse files Browse the repository at this point in the history
Currently at least five commands have optional positional arguments.

Overloading po::value<std::string>()->default_value("") for this
is a bit sneaky but nothing better fits into the existing Shell.cc
framework.

Note that strictly speaking "[<interval>] [<start-time>]" should be
"[<interval> [<start-time>]]" 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 <[email protected]>
  • Loading branch information
idryomov committed Feb 8, 2022
1 parent 56e348e commit cb0df39
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 15 deletions.
10 changes: 5 additions & 5 deletions src/test/cli/rbd/help.t
Original file line number Diff line number Diff line change
Expand Up @@ -1665,7 +1665,7 @@
rbd help mirror image enable
usage: rbd mirror image enable [--pool <pool>] [--namespace <namespace>]
[--image <image>]
<image-spec> <mode>
<image-spec> [<mode>]
Enable RBD mirroring for an image.
Expand Down Expand Up @@ -1943,7 +1943,7 @@
[--pool <pool>]
[--namespace <namespace>]
[--image <image>]
<interval> <start-time>
<interval> [<start-time>]
Add mirror snapshot schedule.
Expand Down Expand Up @@ -1978,7 +1978,7 @@
[--pool <pool>]
[--namespace <namespace>]
[--image <image>]
<interval> <start-time>
[<interval>] [<start-time>]
Remove mirror snapshot schedule.
Expand Down Expand Up @@ -2514,7 +2514,7 @@
rbd help trash purge schedule add
usage: rbd trash purge schedule add [--pool <pool>] [--namespace <namespace>]
<interval> <start-time>
<interval> [<start-time>]
Add trash purge schedule.
Expand Down Expand Up @@ -2543,7 +2543,7 @@
rbd help trash purge schedule remove
usage: rbd trash purge schedule remove
[--pool <pool>] [--namespace <namespace>]
<interval> <start-time>
[<interval>] [<start-time>]
Remove trash purge schedule.
Expand Down
13 changes: 13 additions & 0 deletions src/tools/rbd/OptionPrinter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "tools/rbd/OptionPrinter.h"
#include "tools/rbd/IndentStream.h"
#include "include/ceph_assert.h"

namespace rbd {

Expand Down Expand Up @@ -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<std::string>()->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<std::string>(&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());
Expand Down
16 changes: 12 additions & 4 deletions src/tools/rbd/Schedule.cc
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,19 @@ void normalize_level_spec_args(std::map<std::string, std::string> *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<std::string>()->default_value(""),
"schedule interval");
}
positional->add_options()
("start-time", "schedule start time");
("start-time", po::value<std::string>()->default_value(""),
"schedule start time");
}

int get_schedule_args(const po::variables_map &vm, bool mandatory,
Expand Down
2 changes: 1 addition & 1 deletion src/tools/rbd/Schedule.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ int get_level_spec_args(const boost::program_options::variables_map &vm,
void normalize_level_spec_args(std::map<std::string, std::string> *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<std::string, std::string> *args);

Expand Down
3 changes: 2 additions & 1 deletion src/tools/rbd/action/MirrorImage.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string>()->default_value(""),
"mirror image mode (journal or snapshot) [default: journal]");
}

void get_arguments_disable(po::options_description *positional,
Expand Down
4 changes: 2 additions & 2 deletions src/tools/rbd/action/MirrorSnapshotSchedule.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/tools/rbd/action/TrashPurgeSchedule.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit cb0df39

Please sign in to comment.