tools: extend nfdc help to cover more cases

Change-Id: I0cd182c8635b15c8ffb750c913e14fa99e595d45
Refs: #4503
diff --git a/tests/tools/nfdc/command-parser.t.cpp b/tests/tools/nfdc/command-parser.t.cpp
index d55ccec..47aa815 100644
--- a/tests/tools/nfdc/command-parser.t.cpp
+++ b/tests/tools/nfdc/command-parser.t.cpp
@@ -64,7 +64,7 @@
   defHelp
     .addArg("noun", ArgValueType::STRING, Required::NO, Positional::YES)
     .addArg("verb", ArgValueType::STRING, Required::NO, Positional::YES);
-  parser.addCommand(defHelp, dummyExecute, AVAILABLE_IN_ONE_SHOT);
+  parser.addCommand(defHelp, dummyExecute, AVAILABLE_IN_ONE_SHOT | AVAILABLE_IN_HELP);
 
   CommandDefinition defStatusShow("status", "show");
   parser.addCommand(defStatusShow, dummyExecute);
@@ -83,7 +83,10 @@
   parser.addCommand(defRouteAdd, dummyExecute);
   parser.addAlias("route", "add", "add2");
 
-  BOOST_CHECK_EQUAL(parser.listCommands("", ParseMode::ONE_SHOT).size(), 3);
+  CommandDefinition defHidden("hidden", "");
+  parser.addCommand(defHidden, dummyExecute, AVAILABLE_IN_BATCH);
+
+  BOOST_CHECK_EQUAL(parser.listCommands("", ParseMode::ONE_SHOT).size(), 4);
   BOOST_CHECK_EQUAL(parser.listCommands("", ParseMode::BATCH).size(), 3);
   BOOST_CHECK_EQUAL(parser.listCommands("route", ParseMode::ONE_SHOT).size(), 2);
   BOOST_CHECK_EQUAL(parser.listCommands("unknown", ParseMode::ONE_SHOT).size(), 0);
@@ -100,14 +103,6 @@
   BOOST_CHECK_EQUAL(noun, "help");
   BOOST_CHECK_EQUAL(verb, "");
 
-  std::tie(noun, verb, ca, execute) = parser.parse({"foo", "help"}, ParseMode::ONE_SHOT);
-  BOOST_CHECK_EQUAL(noun, "help");
-  BOOST_CHECK_EQUAL(verb, "");
-
-  std::tie(noun, verb, ca, execute) = parser.parse({"foo", "bar", "-h"}, ParseMode::ONE_SHOT);
-  BOOST_CHECK_EQUAL(noun, "help");
-  BOOST_CHECK_EQUAL(verb, "");
-
   std::tie(noun, verb, ca, execute) = parser.parse({"status"}, ParseMode::ONE_SHOT);
   BOOST_CHECK_EQUAL(noun, "status");
   BOOST_CHECK_EQUAL(verb, "show");
@@ -137,6 +132,8 @@
                     CommandParser::NoSuchCommandError);
   BOOST_CHECK_THROW(parser.parse({"route", "add"}, ParseMode::ONE_SHOT),
                     CommandDefinition::Error);
+  BOOST_CHECK_THROW(parser.parse({"hidden"}, ParseMode::ONE_SHOT),
+                    CommandParser::NoSuchCommandError);
 }
 
 BOOST_AUTO_TEST_SUITE_END() // TestCommandParser
diff --git a/tests/tools/nfdc/help.t.cpp b/tests/tools/nfdc/help.t.cpp
new file mode 100644
index 0000000..ab53004
--- /dev/null
+++ b/tests/tools/nfdc/help.t.cpp
@@ -0,0 +1,106 @@
+/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
+/*
+ * Copyright (c) 2014-2018,  Regents of the University of California,
+ *                           Arizona Board of Regents,
+ *                           Colorado State University,
+ *                           University Pierre & Marie Curie, Sorbonne University,
+ *                           Washington University in St. Louis,
+ *                           Beijing Institute of Technology,
+ *                           The University of Memphis.
+ *
+ * This file is part of NFD (Named Data Networking Forwarding Daemon).
+ * See AUTHORS.md for complete list of NFD authors and contributors.
+ *
+ * NFD is free software: you can redistribute it and/or modify it under the terms
+ * of the GNU General Public License as published by the Free Software Foundation,
+ * either version 3 of the License, or (at your option) any later version.
+ *
+ * NFD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ * without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * NFD, e.g., in COPYING.md file.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "nfdc/help.hpp"
+
+#include "tests/test-common.hpp"
+
+namespace nfd {
+namespace tools {
+namespace nfdc {
+namespace tests {
+
+using namespace nfd::tests;
+
+BOOST_AUTO_TEST_SUITE(Nfdc)
+BOOST_FIXTURE_TEST_SUITE(TestHelp, BaseFixture)
+
+BOOST_AUTO_TEST_CASE(Basic)
+{
+  CommandParser parser;
+  ExecuteCommand dummyExecute = [] (ExecuteContext&) { BOOST_ERROR("should not be called"); };
+
+  boost::test_tools::output_test_stream out;
+  const std::string header("nfdc [-h|--help] [-V|--version] <command> [<args>]\n\n");
+  const std::string trailer("\nSee 'nfdc help <command>' to read about a specific subcommand.\n");
+
+  helpList(out, parser);
+  BOOST_CHECK(out.is_equal(header + "All subcommands:\n"
+                                    "  (none)\n"));
+
+  parser.addCommand(CommandDefinition("status", "show"), dummyExecute);
+  parser.addCommand(CommandDefinition("face", "list"), dummyExecute);
+  parser.addCommand(CommandDefinition("route", "list"), dummyExecute);
+  parser.addCommand(CommandDefinition("route", "add"), dummyExecute);
+  parser.addCommand(CommandDefinition("batch", "command"), dummyExecute,
+                    AVAILABLE_IN_BATCH | AVAILABLE_IN_HELP);
+
+  helpList(out, parser);
+  BOOST_CHECK(out.is_equal(header +
+                           "All subcommands:\n"
+                           "  status show     \n"
+                           "  face list       \n"
+                           "  route list      \n"
+                           "  route add       \n" + trailer));
+
+  helpList(out, parser, ParseMode::ONE_SHOT, "route");
+  BOOST_CHECK(out.is_equal(header +
+                           "Subcommands starting with route:\n"
+                           "  route list      \n"
+                           "  route add       \n" + trailer));
+
+  helpList(out, parser, ParseMode::ONE_SHOT, "hello");
+  BOOST_CHECK(out.is_equal(header +
+                           "Subcommands starting with hello:\n"
+                           "  (none)\n"));
+
+  helpList(out, parser, ParseMode::BATCH);
+  BOOST_CHECK(out.is_equal(header +
+                           "All subcommands:\n"
+                           "  status show     \n"
+                           "  face list       \n"
+                           "  route list      \n"
+                           "  route add       \n"
+                           "  batch command   \n" + trailer));
+
+  BOOST_CHECK_EQUAL(help(out, parser, {}), 2);
+  BOOST_CHECK(out.is_empty());
+
+  BOOST_CHECK_EQUAL(help(out, parser, {"help"}), 0);
+  BOOST_CHECK(out.is_equal(header +
+                           "All subcommands:\n"
+                           "  status show     \n"
+                           "  face list       \n"
+                           "  route list      \n"
+                           "  route add       \n" + trailer));
+}
+
+BOOST_AUTO_TEST_SUITE_END() // TestCommandParser
+BOOST_AUTO_TEST_SUITE_END() // Nfdc
+
+} // namespace tests
+} // namespace nfdc
+} // namespace tools
+} // namespace nfd
diff --git a/tools/nfdc/available-commands.cpp b/tools/nfdc/available-commands.cpp
index 272c13d..fca0745 100644
--- a/tools/nfdc/available-commands.cpp
+++ b/tools/nfdc/available-commands.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2017,  Regents of the University of California,
+ * Copyright (c) 2014-2018,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -25,7 +25,6 @@
 
 #include "available-commands.hpp"
 #include "face-module.hpp"
-#include "help.hpp"
 #include "rib-module.hpp"
 #include "status.hpp"
 #include "strategy-choice-module.hpp"
@@ -37,7 +36,6 @@
 void
 registerCommands(CommandParser& parser)
 {
-  registerHelpCommand(parser);
   registerStatusCommands(parser);
   FaceModule::registerCommands(parser);
   RibModule::registerCommands(parser);
diff --git a/tools/nfdc/command-parser.cpp b/tools/nfdc/command-parser.cpp
index 74ecf36..59c4912 100644
--- a/tools/nfdc/command-parser.cpp
+++ b/tools/nfdc/command-parser.cpp
@@ -108,7 +108,7 @@
 }
 
 std::tuple<std::string, std::string, CommandArguments, ExecuteCommand>
-CommandParser::parse(std::vector<std::string> tokens, ParseMode mode) const
+CommandParser::parse(const std::vector<std::string>& tokens, ParseMode mode) const
 {
   BOOST_ASSERT(mode == ParseMode::ONE_SHOT);
 
@@ -130,20 +130,6 @@
       i = m_commands.find({noun, ""});
     }
     nameLen = std::min<size_t>(1, tokens.size());
-
-    if (i == m_commands.end()) {
-      const auto helpStrings = {"help", "--help", "-h"};
-      auto helpIt = std::find_first_of(tokens.begin(), tokens.end(),
-                                       helpStrings.begin(), helpStrings.end());
-      if (helpIt != tokens.end()) {
-        NDN_LOG_TRACE("fallback to noun=help verb=");
-        i = m_commands.find({"help", ""});
-        if (i != m_commands.end()) {
-          tokens.erase(helpIt);
-          nameLen = 0;
-        }
-      }
-    }
   }
 
   if (i == m_commands.end() || (i->second->modes & static_cast<AvailableIn>(mode)) == 0) {
diff --git a/tools/nfdc/command-parser.hpp b/tools/nfdc/command-parser.hpp
index 5ff5217..a872fcd 100644
--- a/tools/nfdc/command-parser.hpp
+++ b/tools/nfdc/command-parser.hpp
@@ -103,7 +103,7 @@
    *  \return noun, verb, arguments, execute function
    */
   std::tuple<std::string, std::string, CommandArguments, ExecuteCommand>
-  parse(std::vector<std::string> tokens, ParseMode mode) const;
+  parse(const std::vector<std::string>& tokens, ParseMode mode) const;
 
 private:
   typedef std::pair<std::string, std::string> CommandName;
diff --git a/tools/nfdc/help.cpp b/tools/nfdc/help.cpp
index e67992c..88ebff2 100644
--- a/tools/nfdc/help.cpp
+++ b/tools/nfdc/help.cpp
@@ -28,6 +28,8 @@
 
 #include <ndn-cxx/util/logger.hpp>
 
+#include <cerrno>
+#include <cstring>
 #include <unistd.h>
 
 namespace nfd {
@@ -71,35 +73,31 @@
   std::string manpage = "nfdc-" + noun;
 
   ::execlp("man", "man", manpage.data(), nullptr);
-  NDN_LOG_FATAL("Error opening man page for " << manpage);
+  NDN_LOG_FATAL("Error opening man page for " << manpage << ": " << std::strerror(errno));
 }
 
-void
-help(ExecuteContext& ctx, const CommandParser& parser)
+int
+help(std::ostream& os, const CommandParser& parser, std::vector<std::string> args)
 {
-  auto noun = ctx.args.get<std::string>("noun", "");
-  auto verb = ctx.args.get<std::string>("verb", "");
+  const auto helpOpts = {"help", "--help", "-h"};
+  auto it = std::find_first_of(args.begin(), args.end(), helpOpts.begin(), helpOpts.end());
+  if (it == args.end())
+    return 2;
+
+  args.erase(it);
+  auto noun = args.size() > 0 ? args[0] : "";
+  auto verb = args.size() > 1 ? args[1] : "";
 
   if (noun.empty()) {
-    helpList(ctx.out, parser);
+    helpList(os, parser);
+    return 0;
   }
   else {
     helpCommand(noun, verb); // should not return
-    ctx.exitCode = 1;
+    return 1;
   }
 }
 
-void
-registerHelpCommand(CommandParser& parser)
-{
-  CommandDefinition defHelp("help", "");
-  defHelp
-    .setTitle("display help information")
-    .addArg("noun", ArgValueType::STRING, Required::NO, Positional::YES)
-    .addArg("verb", ArgValueType::STRING, Required::NO, Positional::YES);
-  parser.addCommand(defHelp, bind(&help, _1, cref(parser)));
-}
-
 } // namespace nfdc
 } // namespace tools
 } // namespace nfd
diff --git a/tools/nfdc/help.hpp b/tools/nfdc/help.hpp
index 07fce90..ccad718 100644
--- a/tools/nfdc/help.hpp
+++ b/tools/nfdc/help.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2014-2017,  Regents of the University of California,
+/*
+ * Copyright (c) 2014-2018,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -32,19 +32,29 @@
 namespace tools {
 namespace nfdc {
 
+/** \brief writes the list of available commands to a stream
+ *  \param os the output stream to write the list to
+ *  \param parser instance of CommandParser containing the commands to list
+ *  \param mode only the commands available in this mode are listed
+ *  \param noun if not empty, only the commands starting with this noun are listed
+ */
 void
 helpList(std::ostream& os, const CommandParser& parser,
          ParseMode mode = ParseMode::ONE_SHOT, const std::string& noun = "");
 
-/** \brief the 'help' command
+/** \brief tries to help the user, if requested on the command line
+ *
+ *  Depending on the provided command line arguments \p args, this function can either
+ *  open the man page for a specific command, or list all commands available in \p parser.
+ *  In the former case, this function never returns if successful.
+ *
+ *  \retval 0 a list of available commands was successfully written to \p os
+ *  \retval 1 help was requested, but an error was encountered while exec'ing the `man` binary
+ *  \retval 2 help was not provided because \p args did not contain any help-related options
  */
-void
-help(ExecuteContext& ctx, const CommandParser& parser);
-
-/** \brief registers 'help' command
- */
-void
-registerHelpCommand(CommandParser& parser);
+int
+help(std::ostream& os, const CommandParser& parser,
+     std::vector<std::string> args);
 
 } // namespace nfdc
 } // namespace tools
diff --git a/tools/nfdc/main.cpp b/tools/nfdc/main.cpp
index a164a8e..5a7ce13 100644
--- a/tools/nfdc/main.cpp
+++ b/tools/nfdc/main.cpp
@@ -41,7 +41,7 @@
   CommandParser parser;
   registerCommands(parser);
 
-  if (args.empty() || args[0] == "-h" || args[0] == "--help") {
+  if (args.empty()) {
     helpList(std::cout, parser);
     return 0;
   }
@@ -55,11 +55,13 @@
   CommandArguments ca;
   ExecuteCommand execute;
   try {
-    std::tie(noun, verb, ca, execute) = parser.parse(std::move(args), ParseMode::ONE_SHOT);
+    std::tie(noun, verb, ca, execute) = parser.parse(args, ParseMode::ONE_SHOT);
   }
   catch (const std::invalid_argument& e) {
-    std::cerr << e.what() << std::endl;
-    return 2;
+    int ret = help(std::cout, parser, std::move(args));
+    if (ret == 2)
+      std::cerr << e.what() << std::endl;
+    return ret;
   }
 
   try {
diff --git a/tools/nfdc/status.cpp b/tools/nfdc/status.cpp
index 495a21e..5365fbb 100644
--- a/tools/nfdc/status.cpp
+++ b/tools/nfdc/status.cpp
@@ -121,7 +121,7 @@
 {
   CommandDefinition defStatusReport("status", "report");
   defStatusReport
-    .setTitle("print NFD status report")
+    .setTitle("print full status report")
     .addArg("format", ArgValueType::REPORT_FORMAT, Required::NO, Positional::YES);
   parser.addCommand(defStatusReport, &reportStatusComprehensive);