Fix help message printing

programs/disks/CommandRemove.cpp

@@ -10,8 +10,7 @@ public:
     CommandRemove()
     {
         command_name = "remove";
-        description = "Remove file or directory with all children. Throws exception if file doesn't exists.\nPath should be in format './' "
-                      "or './path' or 'path'";
+        description = "Remove file or directory with all children. Throws exception if file doesn't exists";
         options_description.add_options()("path", po::value<String>(), "path from which we copy (mandatory, positional)");
         positional_options_description.add("path", 1);
     }

programs/disks/CommandSwitchDisk.cpp

@@ -7,18 +7,13 @@
 namespace DB
 {
 
-namespace ErrorCodes
-{
-extern const int BAD_ARGUMENTS;
-};
-
 class CommandSwitchDisk final : public ICommand
 {
 public:
     explicit CommandSwitchDisk() : ICommand()
     {
         command_name = "switch-disk";
-        description = "Change disk";
+        description = "Switch disk";
         options_description.add_options()("disk", po::value<String>(), "the disk to switch to (mandatory, positional)")(
             "path", po::value<String>(), "the path to switch on the disk");
         positional_options_description.add("disk", 1);

programs/disks/DisksApp.cpp

@@ -44,7 +44,7 @@ CommandPtr DisksApp::getCommandByName(String command) const
     }
     catch (...)
     {
-        throw Exception(ErrorCodes::BAD_ARGUMENTS, "The command {} is unknown", command);
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, "The command `{}` is unknown", command);
     }
 }
 
@@ -199,6 +199,10 @@ bool DisksApp::processQueryText(String text)
                 std::cerr << "COMMAND: " << command->command_name << "\n";
                 std::cerr << command->options_description << "\n";
             }
+            else
+            {
+                printAvailableCommandsHelpMessage();
+            }
         }
         else
         {
@@ -282,34 +286,70 @@ void DisksApp::processOptions()
         config().setString("log-level", options["log-level"].as<String>());
 }
 
-void DisksApp::printHelpMessage(const ProgramOptionsDescription &)
+
+void DisksApp::printEntryHelpMessage()
 {
-    std::optional<ProgramOptionsDescription> help_description
-        = createOptionsDescription("Help Message for clickhouse-disks", getTerminalWidth());
-
-    help_description->add(options_description);
-
     std::cout << "ClickHouse disk management tool\n";
-    std::cout << "Usage: ./clickhouse-disks [OPTION]\n";
-    std::cout << "clickhouse-disks\n\n";
+    std::cout << options_description << '\n';
+}
 
+size_t DisksApp::getMagicConstant()
+{
+    size_t magic_constant = 0;
     for (const auto & [current_command, _] : command_descriptions)
     {
-        std::cout << command_descriptions[current_command]->command_name;
+        std::string command_string{};
+        command_string += command_descriptions[current_command]->command_name;
         bool was = false;
         for (const auto & [alias_name, alias_command_name] : aliases)
         {
             if (alias_command_name == current_command)
             {
                 if (was)
-                    std::cout << ",";
+                    command_string += ",";
                 else
-                    std::cout << "(";
-                std::cout << alias_name;
+                    command_string += "(";
+                command_string += alias_name;
                 was = true;
             }
         }
-        std::cout << (was ? ")" : "") << " \t" << command_descriptions[current_command]->description << "\n\n";
+        command_string += (was ? ")" : "");
+
+        magic_constant = std::max(magic_constant, command_string.size());
+    }
+    return magic_constant + 2;
+}
+
+void DisksApp::printAvailableCommandsHelpMessage()
+{
+    size_t magic_constant = getMagicConstant();
+
+    std::cout << "\x1b[1;33mAvailable commands:\x1b[0m\n";
+    for (const auto & [current_command, _] : command_descriptions)
+    {
+        std::string command_string{};
+        command_string += command_descriptions[current_command]->command_name;
+        bool was = false;
+        for (const auto & [alias_name, alias_command_name] : aliases)
+        {
+            if (alias_command_name == current_command)
+            {
+                if (was)
+                    command_string += ",";
+                else
+                    command_string += "(";
+                command_string += alias_name;
+                was = true;
+            }
+        }
+        command_string += (was ? ")" : "");
+        std::cout << "\x1b[1;32m" << command_string << "\x1b[0m";
+        for (size_t i = command_string.size(); i < magic_constant; ++i)
+        {
+            std::cout << " ";
+        }
+
+        std::cout << command_descriptions[current_command]->description << "\n";
     }
 }
 
@@ -347,7 +387,8 @@ void DisksApp::init(const std::vector<String> & common_arguments)
 
     if (options.count("help"))
     {
-        printHelpMessage(options_description);
+        printEntryHelpMessage();
+        printAvailableCommandsHelpMessage();
         exit(0); // NOLINT(concurrency-mt-unsafe)
     }
 
@@ -373,6 +414,7 @@ int DisksApp::main(const std::vector<String> & /*args*/)
     }
     else
     {
+        printEntryHelpMessage();
         throw Exception(ErrorCodes::BAD_ARGUMENTS, "No config-file specified");
     }
 

programs/disks/DisksApp.h

@@ -38,12 +38,15 @@ public:
     static void parseAndCheckOptions(
         const std::vector<String> & arguments, const ProgramOptionsDescription & options_description, CommandLineOptions & options);
 
-    void printHelpMessage(const ProgramOptionsDescription &);
+    void printEntryHelpMessage();
+    void printAvailableCommandsHelpMessage();
 
     std::vector<String> getCompletions(const String & prefix) const;
 
     std::vector<String> getEmptyCompletion(CommandPtr command_) const;
 
+    size_t getMagicConstant();
+
     ~DisksApp() override;
 
 private:

programs/disks/DisksClient.cpp

@@ -13,8 +13,15 @@
 #include <Formats/registerFormats.h>
 #include <Common/TerminalSize.h>
 
+namespace ErrorCodes
+{
+extern const int BAD_ARGUMENTS;
+extern const int LOGICAL_ERROR;
+};
+
 namespace DB
 {
+
 std::vector<String> split(const String & text, const String & delimiters)
 {
     std::vector<String> arguments;
@@ -34,5 +41,250 @@ std::vector<String> split(const String & text, const String & delimiters)
         arguments.push_back({prev, text.end()});
     }
     return arguments;
+}
+
+DiskWithPath::DiskWithPath(DiskPtr disk_, std::optional<String> path_)
+    : disk(disk_)
+    , path(
+          [&]()
+          {
+              if (path_.has_value())
+              {
+                  if (!fs::path{path_.value()}.is_absolute())
+                  {
+                      throw Exception(ErrorCodes::BAD_ARGUMENTS, "Initializing path {} is not absolute", path_.value());
+                  }
+                  return path_.value();
+              }
+              else
+              {
+                  return String{"/"};
+              }
+          }())
+{
+    if (!disk->isDirectory(normalizePathAndGetAsRelative(path)))
+    {
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, "Initializing path {} at disk {} is not a directory", path, disk->getName());
     }
 }
+
+std::vector<String> DiskWithPath::listAllFilesByPath(const String & any_path) const
+{
+    if (isDirectory(any_path))
+    {
+        std::vector<String> file_names;
+        disk->listFiles(getRelativeFromRoot(any_path), file_names);
+        return file_names;
+    }
+    else
+    {
+        return {};
+    }
+}
+
+std::vector<String> DiskWithPath::getAllFilesByPattern(std::string pattern) const
+{
+    auto [path_before, path_after] = [&]() -> std::pair<String, String>
+    {
+        auto slash_pos = pattern.find_last_of('/');
+        if (slash_pos >= pattern.size())
+        {
+            return {"", pattern};
+        }
+        else
+        {
+            return {pattern.substr(0, slash_pos + 1), pattern.substr(slash_pos + 1, pattern.size() - slash_pos - 1)};
+        }
+    }();
+
+    if (!isDirectory(path_before))
+    {
+        return {};
+    }
+    else
+    {
+        std::vector<String> file_names = listAllFilesByPath(path_before);
+
+        std::vector<String> answer;
+
+        for (const auto & file_name : file_names)
+        {
+            if (file_name.starts_with(path_after))
+            {
+                String file_pattern = path_before + file_name;
+                if (isDirectory(file_pattern))
+                {
+                    file_pattern = file_pattern + "/";
+                }
+                answer.push_back(file_pattern);
+            }
+        }
+        return answer;
+    }
+};
+
+void DiskWithPath::setPath(const String & any_path)
+{
+    if (isDirectory(any_path))
+    {
+        path = getAbsolutePath(any_path);
+    }
+    else
+    {
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, "Path {} at disk {} is not a directory", any_path, disk->getName());
+    }
+}
+
+String DiskWithPath::validatePathAndGetAsRelative(const String & path)
+{
+    String lexically_normal_path = fs::path(path).lexically_normal();
+    if (lexically_normal_path.find("..") != std::string::npos)
+        throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "Path {} is not normalized", path);
+
+    /// If path is absolute we should keep it as relative inside disk, so disk will look like
+    /// an ordinary filesystem with root.
+    if (fs::path(lexically_normal_path).is_absolute())
+        return lexically_normal_path.substr(1);
+
+    return lexically_normal_path;
+}
+
+std::string DiskWithPath::normalizePathAndGetAsRelative(const std::string & messyPath)
+{
+    std::filesystem::path path(messyPath);
+    std::filesystem::path canonical_path = std::filesystem::weakly_canonical(path);
+    std::string npath = canonical_path.make_preferred().string();
+    return validatePathAndGetAsRelative(npath);
+}
+
+std::string DiskWithPath::normalizePath(const std::string & messyPath)
+{
+    std::filesystem::path path(messyPath);
+    std::filesystem::path canonical_path = std::filesystem::weakly_canonical(path);
+    return canonical_path.make_preferred().string();
+}
+
+DisksClient::DisksClient(std::vector<std::pair<DiskPtr, std::optional<String>>> && disks_with_paths, std::optional<String> begin_disk)
+{
+    if (disks_with_paths.empty())
+    {
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, "Initializing array of disks is empty");
+    }
+    if (!begin_disk.has_value())
+    {
+        begin_disk = disks_with_paths[0].first->getName();
+    }
+    bool has_begin_disk = true;
+    for (auto & [disk, path] : disks_with_paths)
+    {
+        addDisk(disk, path);
+        if (disk->getName() == begin_disk.value())
+        {
+            has_begin_disk = true;
+        }
+    }
+    if (!has_begin_disk)
+    {
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, "There is no begin_disk '{}' in initializing array", begin_disk.value());
+    }
+    current_disk = std::move(begin_disk.value());
+}
+
+const DiskWithPath & DisksClient::getDiskWithPath(const String & disk) const
+{
+    try
+    {
+        return disks.at(disk);
+    }
+    catch (...)
+    {
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, "The disk '{}' is unknown", disk);
+    }
+}
+
+DiskWithPath & DisksClient::getDiskWithPath(const String & disk)
+{
+    try
+    {
+        return disks.at(disk);
+    }
+    catch (...)
+    {
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, "The disk '{}' is unknown", disk);
+    }
+}
+
+const DiskWithPath & DisksClient::getCurrentDiskWithPath() const
+{
+    try
+    {
+        return disks.at(current_disk);
+    }
+    catch (...)
+    {
+        throw Exception(ErrorCodes::LOGICAL_ERROR, "There is no current disk in client");
+    }
+}
+
+DiskWithPath & DisksClient::getCurrentDiskWithPath()
+{
+    try
+    {
+        return disks.at(current_disk);
+    }
+    catch (...)
+    {
+        throw Exception(ErrorCodes::LOGICAL_ERROR, "There is no current disk in client");
+    }
+}
+
+void DisksClient::switchToDisk(const String & disk_, const std::optional<String> & path_)
+{
+    if (disks.contains(disk_))
+    {
+        if (path_.has_value())
+        {
+            disks.at(disk_).setPath(path_.value());
+        }
+        current_disk = disk_;
+    }
+    else
+    {
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, "The disk '{}' is unknown", disk_);
+    }
+}
+
+std::vector<String> DisksClient::getAllDiskNames() const
+{
+    std::vector<String> answer{};
+    answer.reserve(disks.size());
+    for (const auto & [disk_name, _] : disks)
+    {
+        answer.push_back(disk_name);
+    }
+    return answer;
+}
+
+std::vector<String> DisksClient::getAllFilesByPatternFromAllDisks(std::string pattern) const
+{
+    std::vector<String> answer{};
+    for (const auto & [_, disk] : disks)
+    {
+        for (auto & word : disk.getAllFilesByPattern(pattern))
+        {
+            answer.push_back(word);
+        }
+    }
+    return answer;
+}
+
+void DisksClient::addDisk(DiskPtr disk_, const std::optional<String> & path_)
+{
+    String disk_name = disk_->getName();
+    if (disks.contains(disk_->getName()))
+    {
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, "The disk '{}' already exists", disk_name);
+    }
+    disks.emplace(disk_name, DiskWithPath{disk_, path_});
+}
+}

programs/disks/DisksClient.h

@@ -12,9 +12,7 @@
 #include <Interpreters/Context.h>
 #include <boost/program_options/options_description.hpp>
 #include <boost/program_options/variables_map.hpp>
-#include "Common/Exception.h"
 
-// #include <boost/program_options.hpp>
 namespace fs = std::filesystem;
 
 namespace DB
@@ -25,40 +23,10 @@ std::vector<String> split(const String & text, const String & delimiters);
 using ProgramOptionsDescription = boost::program_options::options_description;
 using CommandLineOptions = boost::program_options::variables_map;
 
-
-namespace ErrorCodes
-{
-extern const int BAD_ARGUMENTS;
-extern const int LOGICAL_ERROR;
-};
-
 class DiskWithPath
 {
 public:
-    explicit DiskWithPath(DiskPtr disk_, std::optional<String> path_ = std::nullopt)
-        : disk(disk_)
-        , path(
-              [&]()
-              {
-                  if (path_.has_value())
-                  {
-                      if (!fs::path{path_.value()}.is_absolute())
-                      {
-                          throw Exception(ErrorCodes::BAD_ARGUMENTS, "Initializing path {} is not absolute", path_.value());
-                      }
-                      return path_.value();
-                  }
-                  else
-                  {
-                      return String{"/"};
-                  }
-              }())
-    {
-        if (!disk->isDirectory(normalizePathAndGetAsRelative(path)))
-        {
-            throw Exception(ErrorCodes::BAD_ARGUMENTS, "Initializing path {} at disk {} is not a directory", path, disk->getName());
-        }
-    }
+    explicit DiskWithPath(DiskPtr disk_, std::optional<String> path_ = std::nullopt);
 
     String getAbsolutePath(const String & any_path) const { return normalizePath(fs::path(path) / any_path); }
 
@@ -66,106 +34,20 @@ public:
 
     bool isDirectory(const String & any_path) const { return disk->isDirectory(getRelativeFromRoot(any_path)); }
 
-    std::vector<String> listAllFilesByPath(const String & any_path) const
-    {
-        if (isDirectory(any_path))
-        {
-            std::vector<String> file_names;
-            disk->listFiles(getRelativeFromRoot(any_path), file_names);
-            return file_names;
-        }
-        else
-        {
-            return {};
-        }
-    }
+    std::vector<String> listAllFilesByPath(const String & any_path) const;
 
-    std::vector<String> getAllFilesByPattern(std::string pattern) const
-    {
-        auto [path_before, path_after] = [&]() -> std::pair<String, String>
-        {
-            auto slash_pos = pattern.find_last_of('/');
-            if (slash_pos >= pattern.size())
-            {
-                return {"", pattern};
-            }
-            else
-            {
-                return {pattern.substr(0, slash_pos + 1), pattern.substr(slash_pos + 1, pattern.size() - slash_pos - 1)};
-            }
-        }();
-
-        if (!isDirectory(path_before))
-        {
-            return {};
-        }
-        else
-        {
-            std::vector<String> file_names = listAllFilesByPath(path_before);
-
-            std::vector<String> answer;
-
-            for (const auto & file_name : file_names)
-            {
-                if (file_name.starts_with(path_after))
-                {
-                    String file_pattern = path_before + file_name;
-                    if (isDirectory(file_pattern))
-                    {
-                        file_pattern = file_pattern + "/";
-                    }
-                    answer.push_back(file_pattern);
-                }
-            }
-            return answer;
-        }
-    }
+    std::vector<String> getAllFilesByPattern(std::string pattern) const;
 
     DiskPtr getDisk() const { return disk; }
 
-    void setPath(const String & any_path)
-    {
-        if (isDirectory(any_path))
-        {
-            path = getAbsolutePath(any_path);
-        }
-        else
-        {
-            throw Exception(ErrorCodes::BAD_ARGUMENTS, "Path {} at disk {} is not a directory", any_path, disk->getName());
-        }
-    }
+    void setPath(const String & any_path);
 
     String getRelativeFromRoot(const String & any_path) const { return normalizePathAndGetAsRelative(getAbsolutePath(any_path)); }
 
 private:
-    static String validatePathAndGetAsRelative(const String & path)
-    {
-        String lexically_normal_path = fs::path(path).lexically_normal();
-        if (lexically_normal_path.find("..") != std::string::npos)
-            throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "Path {} is not normalized", path);
-
-        /// If path is absolute we should keep it as relative inside disk, so disk will look like
-        /// an ordinary filesystem with root.
-        if (fs::path(lexically_normal_path).is_absolute())
-            return lexically_normal_path.substr(1);
-
-        return lexically_normal_path;
-    }
-
-    static std::string normalizePathAndGetAsRelative(const std::string & messyPath)
-    {
-        std::filesystem::path path(messyPath);
-        std::filesystem::path canonical_path = std::filesystem::weakly_canonical(path);
-        std::string npath = canonical_path.make_preferred().string();
-        return validatePathAndGetAsRelative(npath);
-    }
-
-    static std::string normalizePath(const std::string & messyPath)
-    {
-        std::filesystem::path path(messyPath);
-        std::filesystem::path canonical_path = std::filesystem::weakly_canonical(path);
-        return canonical_path.make_preferred().string();
-    }
+    static String validatePathAndGetAsRelative(const String & path);
+    static std::string normalizePathAndGetAsRelative(const std::string & messyPath);
+    static std::string normalizePath(const std::string & messyPath);
 
     const DiskPtr disk;
     String path;
@@ -174,134 +56,29 @@ private:
 class DisksClient
 {
 public:
-    explicit DisksClient(std::vector<std::pair<DiskPtr, std::optional<String>>> && disks_with_paths, std::optional<String> begin_disk)
-    {
-        if (disks_with_paths.empty())
-        {
-            throw Exception(ErrorCodes::BAD_ARGUMENTS, "Initializing array of disks is empty");
-        }
-        if (!begin_disk.has_value())
-        {
-            begin_disk = disks_with_paths[0].first->getName();
-        }
-        bool has_begin_disk = true;
-        for (auto & [disk, path] : disks_with_paths)
-        {
-            addDisk(disk, path);
-            if (disk->getName() == begin_disk.value())
-            {
-                has_begin_disk = true;
-            }
-        }
-        if (!has_begin_disk)
-        {
-            throw Exception(ErrorCodes::BAD_ARGUMENTS, "There is no begin_disk '{}' in initializing array", begin_disk.value());
-        }
-        current_disk = std::move(begin_disk.value());
-    }
+    explicit DisksClient(std::vector<std::pair<DiskPtr, std::optional<String>>> && disks_with_paths, std::optional<String> begin_disk);
 
-    const DiskWithPath & getDiskWithPath(const String & disk) const
-    {
-        try
-        {
-            return disks.at(disk);
-        }
-        catch (...)
-        {
-            throw Exception(ErrorCodes::BAD_ARGUMENTS, "The disk '{}' is unknown", disk);
-        }
-    }
+    const DiskWithPath & getDiskWithPath(const String & disk) const;
 
-    DiskWithPath & getDiskWithPath(const String & disk)
-    {
-        try
-        {
-            return disks.at(disk);
-        }
-        catch (...)
-        {
-            throw Exception(ErrorCodes::BAD_ARGUMENTS, "The disk '{}' is unknown", disk);
-        }
-    }
+    DiskWithPath & getDiskWithPath(const String & disk);
 
-    const DiskWithPath & getCurrentDiskWithPath() const
-    {
-        try
-        {
-            return disks.at(current_disk);
-        }
-        catch (...)
-        {
-            throw Exception(ErrorCodes::LOGICAL_ERROR, "There is no current disk in client");
-        }
-    }
+    const DiskWithPath & getCurrentDiskWithPath() const;
 
-    DiskWithPath & getCurrentDiskWithPath()
-    {
-        try
-        {
-            return disks.at(current_disk);
-        }
-        catch (...)
-        {
-            throw Exception(ErrorCodes::LOGICAL_ERROR, "There is no current disk in client");
-        }
-    }
+    DiskWithPath & getCurrentDiskWithPath();
 
     DiskPtr getCurrentDisk() const { return getCurrentDiskWithPath().getDisk(); }
 
     DiskPtr getDisk(const String & disk) const { return getDiskWithPath(disk).getDisk(); }
 
-    void switchToDisk(const String & disk_, const std::optional<String> & path_)
-    {
-        if (disks.contains(disk_))
-        {
-            if (path_.has_value())
-            {
-                disks.at(disk_).setPath(path_.value());
-            }
-            current_disk = disk_;
-        }
-        else
-        {
-            throw Exception(ErrorCodes::BAD_ARGUMENTS, "The disk '{}' is unknown", disk_);
-        }
-    }
+    void switchToDisk(const String & disk_, const std::optional<String> & path_);
 
-    std::vector<String> getAllDiskNames() const
-    {
-        std::vector<String> answer{};
-        answer.reserve(disks.size());
-        for (const auto & [disk_name, _] : disks)
-        {
-            answer.push_back(disk_name);
-        }
-        return answer;
-    }
+    std::vector<String> getAllDiskNames() const;
+
+    std::vector<String> getAllFilesByPatternFromAllDisks(std::string pattern) const;
 
-    std::vector<String> getAllFilesByPatternFromAllDisks(std::string pattern) const
-    {
-        std::vector<String> answer{};
-        for (const auto & [_, disk] : disks)
-        {
-            for (auto & word : disk.getAllFilesByPattern(pattern))
-            {
-                answer.push_back(word);
-            }
-        }
-        return answer;
-    }
 
 private:
-    void addDisk(DiskPtr disk_, const std::optional<String> & path_)
-    {
-        String disk_name = disk_->getName();
-        if (disks.contains(disk_->getName()))
-        {
-            throw Exception(ErrorCodes::BAD_ARGUMENTS, "The disk '{}' already exists", disk_name);
-        }
-        disks.emplace(disk_name, DiskWithPath{disk_, path_});
-    }
+    void addDisk(DiskPtr disk_, const std::optional<String> & path_);
 
     String current_disk;
     std::unordered_map<String, DiskWithPath> disks;