From 93f9e75f354b31c16b062281a7c4f639678d3e72 Mon Sep 17 00:00:00 2001 From: Nikolay Perfilov Date: Mon, 23 Sep 2024 20:36:49 +0300 Subject: [PATCH 1/3] Print command help if command was executed with no arguments --- ydb/apps/ydb/main.cpp | 4 ++++ ydb/public/lib/ydb_cli/common/command.h | 26 +++++++++++++++++-------- ydb/public/lib/ydb_cli/common/common.h | 4 ++++ 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/ydb/apps/ydb/main.cpp b/ydb/apps/ydb/main.cpp index bdb4c507cb93..73ad86557895 100644 --- a/ydb/apps/ydb/main.cpp +++ b/ydb/apps/ydb/main.cpp @@ -13,6 +13,10 @@ int main(int argc, char **argv) { try { return NYdb::NConsoleClient::NewYCloudClient(argc, argv); } + catch (const NYdb::NConsoleClient::TMisuseWithHelpException& e) { + // command help is already printed. Just exit(1) + return EXIT_FAILURE; + } catch (const NYdb::NConsoleClient::TMisuseException& e) { Cerr << e.what() << Endl; Cerr << "Try \"--help\" option for more info." << Endl; diff --git a/ydb/public/lib/ydb_cli/common/command.h b/ydb/public/lib/ydb_cli/common/command.h index 5fc047255b85..e50e9b51a2fc 100644 --- a/ydb/public/lib/ydb_cli/common/command.h +++ b/ydb/public/lib/ydb_cli/common/command.h @@ -204,20 +204,30 @@ class TClientCommand { bool minFailed = minSet && count < minValue; bool maxFailed = maxSet && count > maxValue; if (minFailed || maxFailed) { + TStringBuilder errorMessage; if (minSet && maxSet) { if (minValue == maxValue) { - throw TMisuseException() << "Command " << ArgV[0] + errorMessage << "Command " << ArgV[0] << " requires exactly " << minValue << " free arg(s)."; + } else { + errorMessage << "Command " << ArgV[0] + << " requires from " << minValue << " to " << maxValue << " free arg(s)."; } - throw TMisuseException() << "Command " << ArgV[0] - << " requires from " << minValue << " to " << maxValue << " free arg(s)."; - } - if (minFailed) { - throw TMisuseException() << "Command " << ArgV[0] + } else if (minFailed) { + errorMessage << "Command " << ArgV[0] << " requires at least " << minValue << " free arg(s)."; + } else { + errorMessage << "Command " << ArgV[0] + << " requires at most " << maxValue << " free arg(s)."; + } + if (count == 0) { + Cerr << errorMessage << Endl; + NLastGetopt::TOptsParser parser(Opts, ArgC, ArgV); + parser.PrintUsage(Cerr); + throw TMisuseWithHelpException(); + } else { + throw TMisuseException() << errorMessage; } - throw TMisuseException() << "Command " << ArgV[0] - << " requires at most " << maxValue << " free arg(s)."; } } diff --git a/ydb/public/lib/ydb_cli/common/common.h b/ydb/public/lib/ydb_cli/common/common.h index 2160ffca2674..b6e815ea8832 100644 --- a/ydb/public/lib/ydb_cli/common/common.h +++ b/ydb/public/lib/ydb_cli/common/common.h @@ -23,8 +23,12 @@ namespace NConsoleClient { const TString HomeDir = GetHomeDir(); #endif +// Print 'Try "--help" option for more info' class TMisuseException : public yexception {}; +// Print command help +class TMisuseWithHelpException : public TMisuseException {}; + class TProfileConfig { public: TProfileConfig(const TString& profileName); From 75b0c404d1d574f19a15c459e532a8686d1d5470 Mon Sep 17 00:00:00 2001 From: Nikolay Perfilov Date: Tue, 24 Sep 2024 13:08:13 +0300 Subject: [PATCH 2/3] Check query texts for all commands with query texts --- .../lib/ydb_cli/commands/ydb_service_scripting.cpp | 5 +++++ ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp | 9 +++++---- ydb/public/lib/ydb_cli/commands/ydb_service_table.h | 2 +- ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | 5 +++++ ydb/public/lib/ydb_cli/commands/ydb_yql.cpp | 5 +++++ ydb/public/lib/ydb_cli/common/command.h | 10 +++++++--- 6 files changed, 28 insertions(+), 8 deletions(-) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_scripting.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_scripting.cpp index 747799965100..b3081294d87a 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_scripting.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_scripting.cpp @@ -84,6 +84,11 @@ void TCommandExecuteYqlScript::Parse(TConfig& config) { if (ScriptFile) { Script = ReadFromFile(ScriptFile, "script"); } + if (Script.Empty()) { + Cerr << "Neither text of script (\"--script\", \"-s\") " + << "nor path to file with script text (\"--file\", \"-f\") were provided."; + config.PrintHelpAndExit(); + } if(FlameGraphPath && FlameGraphPath->Empty()) { throw TMisuseException() << "FlameGraph path can not be empty."; diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index 379ff4e2894e..d4314d4b5f26 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -327,10 +327,11 @@ int TCommandDropTable::Run(TConfig& config) { return EXIT_SUCCESS; } -void TCommandQueryBase::CheckQueryOptions() const { +void TCommandQueryBase::CheckQueryOptions(TClientCommand::TConfig& config) const { if (!Query && !QueryFile) { - throw TMisuseException() << "Neither \"Text of query\" (\"--query\", \"-q\") " + Cerr << "Neither \"Text of query\" (\"--query\", \"-q\") " << "nor \"Path to file with query text\" (\"--file\", \"-f\") were provided."; + config.PrintHelpAndExit(); } if (Query && QueryFile) { throw TMisuseException() << "Both mutually exclusive options \"Text of query\" (\"--query\", \"-q\") " @@ -403,7 +404,7 @@ void TCommandExecuteQuery::Parse(TConfig& config) { || BatchMode != EBatchMode::Default) && QueryType == "scheme") { throw TMisuseException() << "Scheme queries does not support parameter options."; } - CheckQueryOptions(); + CheckQueryOptions(config); CheckQueryFile(); ParseParameters(config); } @@ -847,7 +848,7 @@ void TCommandExplain::SaveDiagnosticsToFile(const TString& diagnostics) { void TCommandExplain::Parse(TConfig& config) { TClientCommand::Parse(config); ParseOutputFormats(); - CheckQueryOptions(); + CheckQueryOptions(config); } int TCommandExplain::Run(TConfig& config) { diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.h b/ydb/public/lib/ydb_cli/commands/ydb_service_table.h index d8a0a8b2495d..a0716f7dc322 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.h +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.h @@ -84,7 +84,7 @@ class TCommandDropTable : public TTableCommand, public TCommandWithPath { class TCommandQueryBase { protected: - void CheckQueryOptions() const; + void CheckQueryOptions(TClientCommand::TConfig& config) const; void CheckQueryFile(); protected: diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index 9da881690929..f04bcb960461 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp @@ -98,6 +98,11 @@ void TCommandSql::Parse(TConfig& config) { Query = ReadFromFile(QueryFile, "query"); } } + if (Query.Empty()) { + Cerr << "Neither text of script (\"--script\", \"-s\") " + << "nor path to file with script text (\"--file\", \"-f\") were provided."; + config.PrintHelpAndExit(); + } // Should be called after setting ReadingSomethingFromStdin ParseParameters(config); } diff --git a/ydb/public/lib/ydb_cli/commands/ydb_yql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_yql.cpp index 3cdf76c52774..bfea4ea6c573 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_yql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_yql.cpp @@ -68,6 +68,11 @@ void TCommandYql::Parse(TConfig& config) { if (ScriptFile) { Script = ReadFromFile(ScriptFile, "script"); } + if (Script.Empty()) { + Cerr << "Neither text of script (\"--script\", \"-s\") " + << "nor path to file with script text (\"--file\", \"-f\") were provided."; + config.PrintHelpAndExit(); + } if(FlameGraphPath && FlameGraphPath->Empty()) { throw TMisuseException() << "FlameGraph path can not be empty."; diff --git a/ydb/public/lib/ydb_cli/common/command.h b/ydb/public/lib/ydb_cli/common/command.h index e50e9b51a2fc..83c03bed2f6b 100644 --- a/ydb/public/lib/ydb_cli/common/command.h +++ b/ydb/public/lib/ydb_cli/common/command.h @@ -222,15 +222,19 @@ class TClientCommand { } if (count == 0) { Cerr << errorMessage << Endl; - NLastGetopt::TOptsParser parser(Opts, ArgC, ArgV); - parser.PrintUsage(Cerr); - throw TMisuseWithHelpException(); + PrintHelpAndExit(); } else { throw TMisuseException() << errorMessage; } } } + void PrintHelpAndExit() { + NLastGetopt::TOptsParser parser(Opts, ArgC, ArgV); + parser.PrintUsage(Cerr); + throw TMisuseWithHelpException(); + } + private: size_t GetParamsCount() { size_t result = 0; From c344b015f1bc4ad79cfb5d08aa41472950f5481c Mon Sep 17 00:00:00 2001 From: Nikolay Perfilov Date: Tue, 24 Sep 2024 13:15:39 +0300 Subject: [PATCH 3/3] Fix error output formatting --- .../lib/ydb_cli/commands/ydb_service_scripting.cpp | 10 +++------- ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp | 2 +- ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | 2 +- ydb/public/lib/ydb_cli/commands/ydb_yql.cpp | 2 +- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_scripting.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_scripting.cpp index b3081294d87a..f260e51ab94c 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_scripting.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_scripting.cpp @@ -74,8 +74,9 @@ void TCommandExecuteYqlScript::Parse(TConfig& config) { ParseInputFormats(); ParseOutputFormats(); if (!Script && !ScriptFile) { - throw TMisuseException() << "Neither \"Text of script\" (\"--script\", \"-s\") " - << "nor \"Path to file with script text\" (\"--file\", \"-f\") were provided."; + Cerr << "Neither \"Text of script\" (\"--script\", \"-s\") " + << "nor \"Path to file with script text\" (\"--file\", \"-f\") were provided." << Endl; + config.PrintHelpAndExit(); } if (Script && ScriptFile) { throw TMisuseException() << "Both mutually exclusive options \"Text of script\" (\"--script\", \"-s\") " @@ -84,11 +85,6 @@ void TCommandExecuteYqlScript::Parse(TConfig& config) { if (ScriptFile) { Script = ReadFromFile(ScriptFile, "script"); } - if (Script.Empty()) { - Cerr << "Neither text of script (\"--script\", \"-s\") " - << "nor path to file with script text (\"--file\", \"-f\") were provided."; - config.PrintHelpAndExit(); - } if(FlameGraphPath && FlameGraphPath->Empty()) { throw TMisuseException() << "FlameGraph path can not be empty."; diff --git a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp index d4314d4b5f26..f0d101efc4fd 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp @@ -330,7 +330,7 @@ int TCommandDropTable::Run(TConfig& config) { void TCommandQueryBase::CheckQueryOptions(TClientCommand::TConfig& config) const { if (!Query && !QueryFile) { Cerr << "Neither \"Text of query\" (\"--query\", \"-q\") " - << "nor \"Path to file with query text\" (\"--file\", \"-f\") were provided."; + << "nor \"Path to file with query text\" (\"--file\", \"-f\") were provided." << Endl; config.PrintHelpAndExit(); } if (Query && QueryFile) { diff --git a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp index f04bcb960461..f7485aaf0e9c 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_sql.cpp @@ -100,7 +100,7 @@ void TCommandSql::Parse(TConfig& config) { } if (Query.Empty()) { Cerr << "Neither text of script (\"--script\", \"-s\") " - << "nor path to file with script text (\"--file\", \"-f\") were provided."; + << "nor path to file with script text (\"--file\", \"-f\") were provided." << Endl; config.PrintHelpAndExit(); } // Should be called after setting ReadingSomethingFromStdin diff --git a/ydb/public/lib/ydb_cli/commands/ydb_yql.cpp b/ydb/public/lib/ydb_cli/commands/ydb_yql.cpp index bfea4ea6c573..891b9a056b34 100644 --- a/ydb/public/lib/ydb_cli/commands/ydb_yql.cpp +++ b/ydb/public/lib/ydb_cli/commands/ydb_yql.cpp @@ -70,7 +70,7 @@ void TCommandYql::Parse(TConfig& config) { } if (Script.Empty()) { Cerr << "Neither text of script (\"--script\", \"-s\") " - << "nor path to file with script text (\"--file\", \"-f\") were provided."; + << "nor path to file with script text (\"--file\", \"-f\") were provided." << Endl; config.PrintHelpAndExit(); } if(FlameGraphPath && FlameGraphPath->Empty())