rbd: mark optional positional arguments as such in help output

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 <idryomov@gmail.com>

diff --git a/src/test/cli/rbd/help.t b/src/test/cli/rbd/help.t
index 045da189c5247151d0763b234835a4b385bde151..c5090f3ba8a6b9a9fed8a5111e5e4e3d6f0ee76b 100644 (file)
--- 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 <pool>] [--namespace <namespace>] 
                                  [--image <image>] 
-                                 <image-spec> <mode> 
+                                 <image-spec> [<mode>] 
   
   Enable RBD mirroring for an image.
   
@@ -1943,7 +1943,7 @@
                                         [--pool <pool>] 
                                         [--namespace <namespace>] 
                                         [--image <image>] 
-                                        <interval> <start-time> 
+                                        <interval> [<start-time>] 
   
   Add mirror snapshot schedule.
   
@@ -1978,7 +1978,7 @@
                                         [--pool <pool>] 
                                         [--namespace <namespace>] 
                                         [--image <image>] 
-                                        <interval> <start-time> 
+                                        [<interval>] [<start-time>] 
   
   Remove mirror snapshot schedule.
   
@@ -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.
   
@@ -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.
   

diff --git a/src/tools/rbd/OptionPrinter.cc b/src/tools/rbd/OptionPrinter.cc
index 5a5bf9504989cc77a33150aa4778e2c94c32a218..0fea6b691f1bd07aebd68b52197d4ec293691932 100644 (file)
--- 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<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());

diff --git a/src/tools/rbd/Schedule.cc b/src/tools/rbd/Schedule.cc
index d7fcf1253deb6b95a05428777eb8e220892fc48f..15dda3aee7ef940941d2d89b9ee07d4d55824865 100644 (file)
--- a/src/tools/rbd/Schedule.cc
+++ b/src/tools/rbd/Schedule.cc
@@ -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,

diff --git a/src/tools/rbd/Schedule.h b/src/tools/rbd/Schedule.h
index 90e78eb7f682c734cfd6d4ca0f17b2ebf058c931..bf0964bb1e51b6fd948cf4c730b769cabc176fe1 100644 (file)
--- 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<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);
 

diff --git a/src/tools/rbd/action/MirrorImage.cc b/src/tools/rbd/action/MirrorImage.cc
index ab037b296e3060ae89c8b22509a190b8008243ad..505d377f4807f694ba6bdc286f6b4a99d4652b5f 100644 (file)
--- 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<std::string>()->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 af684944017d118d13e235874c154471443351ba..3f269c2ad8717b6d1df7a3fb1f5ead4dc00cb774 100644 (file)
--- 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 0f92e762bc2fde054b36016278440de29a53dabb..c07ff8a0da02a5239cdcb5d51d4f03508421ce7f 100644 (file)
--- 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,