Skip to content

Commit

Permalink
tool: print usage information when arguments fail to parse
Browse files Browse the repository at this point in the history
This changes the output when the user forgets to pass a required
argument such that the usage string is now returned.

Example:

  todd@va1022:~/kudu$ build/latest/bin/kudu fs dump block
  Invalid argument: must provide positional argument block_id

  Usage: build/latest/bin/kudu fs dump block <block_id>
  [-fs_wal_dir=<dir>] [-fs_data_dirs=<dirs>]

Change-Id: I4695aca5cb3f848f525784476403a9f577fdba13
Reviewed-on: http://gerrit.cloudera.org:8080/7301
Reviewed-by: Adar Dembo <[email protected]>
Tested-by: Kudu Jenkins
  • Loading branch information
toddlipcon committed Aug 1, 2017
1 parent 9f7fed6 commit 7e0a56b
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 15 deletions.
24 changes: 16 additions & 8 deletions src/kudu/tools/kudu-tool-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,16 @@ class ToolTest : public KuduTest {
StripTrailingNewline(stderr);
}
if (stdout_lines) {
*stdout_lines = strings::Split(out, "\n", strings::SkipEmpty());
*stdout_lines = strings::Split(out, "\n");
while (!stdout_lines->empty() && stdout_lines->back() == "") {
stdout_lines->pop_back();
}
}
if (stderr_lines) {
*stderr_lines = strings::Split(err, "\n", strings::SkipEmpty());
*stderr_lines = strings::Split(err, "\n");
while (!stderr_lines->empty() && stderr_lines->back() == "") {
stderr_lines->pop_back();
}
}
return s;
}
Expand Down Expand Up @@ -230,11 +236,13 @@ class ToolTest : public KuduTest {
const string kPositionalArgumentMessage = "must provide positional argument";
const string kVariadicArgumentMessage = "must provide variadic positional argument";
const string& message = variadic ? kVariadicArgumentMessage : kPositionalArgumentMessage;
string err;
RunTool(arg_str, nullptr, &err, nullptr, nullptr);

Status expected_status = Status::InvalidArgument(Substitute("$0 $1", message, required_arg));
ASSERT_EQ(expected_status.ToString(), err);

vector<string> err_lines;
RunTool(arg_str, nullptr, nullptr, nullptr, /* stderr_lines = */ &err_lines);
ASSERT_GE(err_lines.size(), 3) << err_lines;
ASSERT_EQ(expected_status.ToString(), err_lines[0]);
ASSERT_STR_MATCHES(err_lines[2], "Usage: kudu.*");
}

void RunFsCheck(const string& arg_str,
Expand Down Expand Up @@ -800,7 +808,7 @@ TEST_F(ToolTest, TestFsDumpCFile) {
SCOPED_TRACE(stdout);
ASSERT_GE(stdout.size(), 4);
ASSERT_EQ(stdout[0], "Header:");
ASSERT_EQ(stdout[1], "Footer:");
ASSERT_EQ(stdout[2], "Footer:");
}
{
NO_FATALS(RunActionStdoutLines(Substitute(
Expand All @@ -816,7 +824,7 @@ TEST_F(ToolTest, TestFsDumpCFile) {
SCOPED_TRACE(stdout);
ASSERT_GT(stdout.size(), kNumEntries);
ASSERT_EQ(stdout[0], "Header:");
ASSERT_EQ(stdout[1], "Footer:");
ASSERT_EQ(stdout[2], "Footer:");
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/kudu/tools/tool_action.cc
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ Status Action::Run(const vector<Mode*>& chain,
return runner_({ chain, this, required_args, variadic_args });
}

string Action::BuildHelp(const vector<Mode*>& chain) const {
string Action::BuildHelp(const vector<Mode*>& chain,
Action::HelpMode mode) const {
SetOptionalParameterDefaultValues();
string usage_msg = Substitute("Usage: $0 $1", BuildUsageString(chain), name());
string desc_msg;
Expand Down Expand Up @@ -295,6 +296,9 @@ string Action::BuildHelp(const vector<Mode*>& chain) const {
desc_msg += google::DescribeOneFlag(gflag_info);
desc_msg += "\n";
}
if (mode == USAGE_ONLY) {
return usage_msg;
}
string msg;
AppendHardWrapped(usage_msg, 8, &msg);
msg += "\n\n";
Expand Down
10 changes: 9 additions & 1 deletion src/kudu/tools/tool_action.h
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,17 @@ class ActionBuilder {
// A leaf node in the tree, representing a logical operation taken by the tool.
class Action {
public:
enum HelpMode {
// Return the full help text, including descriptions for each
// of the arguments.
FULL_HELP,
// Return only a single-line usage statement.
USAGE_ONLY
};

// Returns the help for this action given its parent mode chain.
std::string BuildHelp(const std::vector<Mode*>& chain) const;
std::string BuildHelp(const std::vector<Mode*>& chain,
HelpMode mode = FULL_HELP) const;

// Returns the help xml for this action
std::string BuildHelpXML(const std::vector<Mode*>& chain) const;
Expand Down
13 changes: 8 additions & 5 deletions src/kudu/tools/tool_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,18 @@ int DispatchCommand(const vector<Mode*>& chain,
vector<string> variadic_args;
Status s = MarshalArgs(chain, action, remaining_args,
&required_args, &variadic_args);
if (s.ok()) {
s = action->Run(chain, required_args, variadic_args);
if (!s.ok()) {
cerr << s.ToString() << endl;
cerr << endl;
cerr << action->BuildHelp(chain, Action::USAGE_ONLY) << endl;
return 1;
}
s = action->Run(chain, required_args, variadic_args);
if (s.ok()) {
return 0;
} else {
cerr << s.ToString() << endl;
return 1;
}
cerr << s.ToString() << endl;
return 1;
}

// Replace hyphens with underscores in a string and return a copy.
Expand Down

0 comments on commit 7e0a56b

Please sign in to comment.