From 0607142729e07d870d42842cf8f8d57699757a39 Mon Sep 17 00:00:00 2001
From: Julien Elbaz <elbywan@hotmail.com>
Date: Sat, 6 Jan 2024 09:50:02 +0100
Subject: [PATCH] chore: prettier cli help output

---
 src/cli/cli.cr           | 83 ++++++++++++++++++++--------------------
 src/cli/dlx.cr           |  6 +--
 src/cli/exec.cr          |  2 +-
 src/cli/init.cr          |  2 +-
 src/cli/install.cr       | 40 +++++++++----------
 src/cli/run.cr           |  4 +-
 src/cli/store.cr         | 14 +++----
 src/cli/why.cr           |  2 +-
 src/ext/option_parser.cr | 22 +++++++++++
 9 files changed, 98 insertions(+), 77 deletions(-)

diff --git a/src/cli/cli.cr b/src/cli/cli.cr
index 260bdca..bad57b6 100644
--- a/src/cli/cli.cr
+++ b/src/cli/cli.cr
@@ -96,26 +96,6 @@ module Zap
 
         separator("Commands")
 
-        subSeparator("Install", early_line_break: false)
-
-        command(["install", "i", "add"], "This command installs one or more packages and any packages that they depends on.", "[options] <package(s)>") do
-          on_install(parser)
-        end
-
-        command(["remove", "rm", "uninstall", "un"], "This command removes one or more packages from the node_modules folder, the package.json file and the lockfile.", "[options] <package(s)>") do
-          on_install(parser, remove_packages: true)
-        end
-
-        command(["update", "up", "upgrade"], "This command updates the lockfile to use the newest package versions.", "[options] <package(s)>") do
-          on_install(parser, update_packages: true)
-        end
-
-        command(["why", "y"], "Show information about why a package is installed.", "<package(s)>") do
-          on_why(parser)
-        end
-
-        subSeparator("Execute")
-
         command(["dlx", "x"], "Install one or more packages and run a command in a temporary environment.", "[options] <command>") do
           on_dlx(parser)
         end
@@ -124,24 +104,34 @@ module Zap
           on_exec(parser)
         end
 
-        command(["run", "r"], "Run a package's \"script\" command.", "[options] <script>") do
-          on_run(parser)
-        end
-
-        subSeparator("Miscellaneous")
-
         command(["init", "innit", "create"], "Create a new package.json file.", "[options] <initializer>") do
           on_init(parser)
         end
 
+        command(["install", "i", "add"], "Install one or more packages and any packages that they depends on.", "[options] <package(s)>") do
+          on_install(parser)
+        end
+
         command(["rebuild", "rb"], "Rebuild native dependencies.", "<package(s)> [options are passed through]") do
           on_rebuild(parser)
         end
 
+        command(["remove", "rm", "uninstall", "un"], "Remove one or more packages from the node_modules folder, the package.json file and the lockfile.", "[options] <package(s)>") do
+          on_install(parser, remove_packages: true)
+        end
+
         command(["store", "s"], "Manage the global store used to save packages and cache registry responses.") do
           on_store(parser)
         end
 
+        command(["update", "up", "upgrade"], "Update the lockfile to use the newest package versions.", "[options] <package(s)>") do
+          on_install(parser, update_packages: true)
+        end
+
+        command(["why", "y"], "Show information about why a package is installed.", "<package(s)>") do
+          on_why(parser)
+        end
+
         parser.before_each do |arg|
           if @command_config.nil? && !parser.@handlers.keys.includes?(arg)
             @command_config = RunConfig.new(fallback_to_exec: true)
@@ -171,19 +161,19 @@ module Zap
     macro common_options(sub = false)
       {% if sub %}subSeparator{% else %}separator{% end %}("Common", early_line_break: false)
 
-      parser.on("-C <path>", "--dir <path>", "Use PATH as the root directory of the project. #{"[env: ZAP_PREFIX]".colorize.dim}") do |path|
+      flag("-C <path>", "--dir <path>", "Use PATH as the root directory of the project. #{"[env: ZAP_PREFIX]".colorize.dim}") do |path|
         @config = @config.copy_with(prefix: Path.new(path).expand.to_s, global: false)
       end
 
-      parser.on("--concurrency <number>", "Set the maximum number of tasks that will be run in parallel. (default: 5) #{"[env: ZAP_CONCURRENCY]".colorize.dim}") do |concurrency|
+      flag("--concurrency <number>", "Set the maximum number of tasks that will be run in parallel. (default: 5) #{"[env: ZAP_CONCURRENCY]".colorize.dim}") do |concurrency|
         @config = @config.copy_with(concurrency: concurrency.to_i32)
       end
 
-      parser.on("--deferred-output", "Do not print the output in real time when running multiple scripts in parallel but instead defer it to have a nicer packed output. (default: false unless CI) #{"[env: ZAP_DEFERRED_OUTPUT]".colorize.dim}") do
+      flag("--deferred-output", "Do not print the output in real time when running multiple scripts in parallel but instead defer it to have a nicer packed output. (default: false unless CI) #{"[env: ZAP_DEFERRED_OUTPUT]".colorize.dim}") do
         @config = @config.copy_with(deferred_output: true)
       end
 
-      parser.on(
+      flag(
         "--file-backend <clonefile|hardlink|copyfile|copy>",
         <<-DESCRIPTION
         The backend to use when linking packages on disk. #{"[env: ZAP_FILE_BACKEND]".colorize.dim}
@@ -197,7 +187,7 @@ module Zap
         @config = @config.copy_with(file_backend: Backend::Backends.parse(backend))
       end
 
-      parser.on(
+      flag(
         "--flock-scope <global|package|none>",
         <<-DESCRIPTION
         Set the scope of the file lock mechanism used to prevent store corruption. #{"[env: ZAP_FLOCK_SCOPE]".colorize.dim}
@@ -210,25 +200,25 @@ module Zap
         @config = @config.copy_with(flock_scope: Config::FLockScope.parse(scope))
       end
 
-      parser.on("-g", "--global", "Operates in \"global\" mode, so that packages are installed into the global folder instead of the current working directory.") do |path|
+      flag("-g", "--global", "Operates in \"global\" mode, so that packages are installed into the global folder instead of the current working directory.") do |path|
         @config = @config.copy_with(prefix: @config.deduce_global_prefix, global: true)
       end
 
-      parser.on("-h", "--help", "Show this help.") do
+      flag("-h", "--help", "Show this help.") do
         puts parser
         exit
       end
 
-      parser.on("--silent", "Minimize the output. #{"[env: ZAP_SILENT]".colorize.dim}") do
+      flag("--silent", "Minimize the output. #{"[env: ZAP_SILENT]".colorize.dim}") do
         @config = @config.copy_with(silent: true)
       end
 
-      parser.on("-v", "--version", "Show version.") do
+      flag("-v", "--version", "Show version.") do
         puts "v#{VERSION}"
         exit
       end
 
-      parser.on("--lockfile-format <yaml|message_pack>", "The serialization to use when saving the lockfile to the disk. (Default: the current lockfile format, or YAML) #{"[env: ZAP_LOCKFILE_FORMAT]".colorize.dim}") do |format|
+      flag("--lockfile-format <yaml|message_pack>", "The serialization to use when saving the lockfile to the disk. (Default: the current lockfile format, or YAML) #{"[env: ZAP_LOCKFILE_FORMAT]".colorize.dim}") do |format|
         @config = @config.copy_with(lockfile_format: Lockfile::Format.parse(format))
       end
     end
@@ -236,21 +226,21 @@ module Zap
     macro workspace_options(sub = false)
       {% if sub %}subSeparator{% else %}separator{% end %}("Workspace")
 
-      parser.on("-F FILTER", "--filter <pattern>", "Filtering allows you to restrict commands to specific subsets of packages.") do |filter|
+      flag("-F <pattern>", "--filter <pattern>", "Filtering allows you to restrict commands to specific subsets of packages.") do |filter|
         filters = @config.filters || Array(Utils::Filter).new
         filters << Utils::Filter.new(filter)
         @config = @config.copy_with(filters: filters)
       end
 
-      parser.on("--ignore-workspaces", "Will completely ignore workspaces when applying the command. #{"[env: ZAP_NO_WORKSPACES]".colorize.dim}") do
+      flag("--ignore-workspaces", "Will completely ignore workspaces when applying the command. #{"[env: ZAP_NO_WORKSPACES]".colorize.dim}") do
         @config = @config.copy_with(no_workspaces: true)
       end
 
-      parser.on("-r", "--recursive", "Will apply the command to all packages in the workspace. #{"[env: ZAP_RECURSIVE]".colorize.dim}") do
+      flag("-r", "--recursive", "Will apply the command to all packages in the workspace. #{"[env: ZAP_RECURSIVE]".colorize.dim}") do
         @config = @config.copy_with(recursive: true)
       end
 
-      parser.on("-w", "--workspace-root", "Will apply the command to the root workspace package. #{"[env: ZAP_ROOT_WORKSPACE]".colorize.dim}") do
+      flag("-w", "--workspace-root", "Will apply the command to the root workspace package. #{"[env: ZAP_ROOT_WORKSPACE]".colorize.dim}") do
         @config = @config.copy_with(root_workspace: true)
       end
     end
@@ -267,7 +257,7 @@ module Zap
     end
 
     private macro separator(text, *, prepend = false)
-      %text = "\n• #{ {{text}}.colorize.underline }\n".colorize.blue.bold.to_s
+      %text = "\n#{ {{text + ":"}}.colorize.underline }\n".colorize.green.bold.to_s
       {% if prepend %}
       parser.@flags.unshift(%text)
       {% else %}
@@ -277,10 +267,11 @@ module Zap
 
     private macro subSeparator(text, *, early_line_break = true)
       prefix = "#{ {% if early_line_break %}NEW_LINE{% else %}nil{% end %} }"
-      parser.separator("#{prefix}    · #{ {{text}} }\n".colorize.light_blue)
+      parser.separator("#{prefix}    #{ {{text}} }\n".colorize.blue.bold)
     end
 
     # @command_color_index = 0
+
     private def command_formatter(flag)
       # flag.colorize(COLORS[@command_color_index % COLORS.size]).bold.tap {
       #   @command_color_index += 1
@@ -288,6 +279,14 @@ module Zap
       flag.colorize.bold.to_s
     end
 
+    private def flag_formatter(flag)
+      flag.split(' ').each.map_with_index { |elt, index| index == 0 ? elt.colorize.cyan : elt }.join(' ')
+    end
+
+    private macro flag(*args, &block)
+      parser.on(*{{ args }}, ->flag_formatter(String)){{ block }}
+    end
+
     private macro command(input, description, args = nil)
       {% if input.is_a?(StringLiteral) %}
         parser.on({{input}},{{description}}, ->command_formatter(String)) do
diff --git a/src/cli/dlx.cr b/src/cli/dlx.cr
index e2575ba..6a42922 100644
--- a/src/cli/dlx.cr
+++ b/src/cli/dlx.cr
@@ -8,13 +8,13 @@ class Zap::CLI
 
     separator("Options")
 
-    parser.on("-c <shell_command>", "--call <shell_command>", "Runs the command inside of a shell.") do |command|
+    flag("-c <shell_command>", "--call <shell_command>", "Runs the command inside of a shell.") do |command|
       @command_config = dlx_config.copy_with(call: command)
     end
-    parser.on("-p <package>", "--package <package>", "The package or packages to install.") do |package|
+    flag("-p <package>", "--package <package>", "The package or packages to install.") do |package|
       dlx_config.packages << package
     end
-    parser.on("-q", "--quiet", "Mute most of the output coming from zap. #{"[env: ZAP_DLX_QUIET]".colorize.dim}") do |package|
+    flag("-q", "--quiet", "Mute most of the output coming from zap. #{"[env: ZAP_DLX_QUIET]".colorize.dim}") do |package|
       @command_config = dlx_config.copy_with(quiet: true)
     end
 
diff --git a/src/cli/exec.cr b/src/cli/exec.cr
index 9c3f6bd..771730b 100644
--- a/src/cli/exec.cr
+++ b/src/cli/exec.cr
@@ -8,7 +8,7 @@ class Zap::CLI
 
     separator("Options")
 
-    parser.on("--parallel", "Run all commands in parallel without any kind of topological ordering. #{"[env: ZAP_EXEC_PARALLEL]".colorize.dim}") do
+    flag("--parallel", "Run all commands in parallel without any kind of topological ordering. #{"[env: ZAP_EXEC_PARALLEL]".colorize.dim}") do
       @command_config = exec_config.copy_with(parallel: true)
     end
 
diff --git a/src/cli/init.cr b/src/cli/init.cr
index 102a223..b3d6083 100644
--- a/src/cli/init.cr
+++ b/src/cli/init.cr
@@ -9,7 +9,7 @@ class Zap::CLI
 
     separator("Options")
 
-    parser.on("-y", "--yes", %(Automatically answer "yes" to any prompts that zap might print on the command line. #{"[env: ZAP_INIT_YES]".colorize.dim})) do |package|
+    flag("-y", "--yes", %(Automatically answer "yes" to any prompts that zap might print on the command line. #{"[env: ZAP_INIT_YES]".colorize.dim})) do |package|
       @command_config = init_config.copy_with(yes: true)
     end
 
diff --git a/src/cli/install.cr b/src/cli/install.cr
index ee12582..45d06be 100644
--- a/src/cli/install.cr
+++ b/src/cli/install.cr
@@ -9,28 +9,28 @@ class Zap::CLI
 
     separator("Options")
 
-    parser.on("--ignore-scripts", "If true, does not run scripts specified in package.json files. #{"[env: ZAP_INSTALL_IGNORE_SCRIPTS]".colorize.dim}") do
+    flag("--frozen-lockfile <true|false>", "If true, will fail if the lockfile is outdated. #{"[env: ZAP_INSTALL_FROZEN_LOCKFILE]".colorize.dim}") do |frozen_lockfile|
+      @command_config = install_config.copy_with(frozen_lockfile: Utils::Various.str_to_bool(frozen_lockfile))
+    end
+    flag("--ignore-scripts", "If true, does not run scripts specified in package.json files. #{"[env: ZAP_INSTALL_IGNORE_SCRIPTS]".colorize.dim}") do
       @command_config = install_config.copy_with(ignore_scripts: true)
     end
-    parser.on("--no-logs", "If true, will not print logs like deprecation warnings. #{"[env: ZAP_INSTALL_PRINT_LOGS=false]".colorize.dim}") do
+    flag("--no-logs", "If true, will not print logs like deprecation warnings. #{"[env: ZAP_INSTALL_PRINT_LOGS=false]".colorize.dim}") do
       @command_config = install_config.copy_with(print_logs: false)
     end
-    parser.on("--production", "If true, will not install devDependencies.") do
-      @command_config = install_config.copy_with(omit: [Commands::Install::Config::Omit::Dev])
-    end
-    parser.on("--peers", "Pass this flag to enable checking for missing peer dependencies. #{"[env: ZAP_INSTALL_CHECK_PEER_DEPENDENCIES]".colorize.dim}") do
+    flag("--peers", "Pass this flag to enable checking for missing peer dependencies. #{"[env: ZAP_INSTALL_CHECK_PEER_DEPENDENCIES]".colorize.dim}") do
       @command_config = install_config.copy_with(check_peer_dependencies: true)
     end
-    parser.on("--prefer-offline", "Bypass staleness checks for package metadata cached from the registry. #{"[env: ZAP_INSTALL_PREFER_OFFLINE]".colorize.dim}") do
+    flag("--prefer-offline", "Bypass staleness checks for package metadata cached from the registry. #{"[env: ZAP_INSTALL_PREFER_OFFLINE]".colorize.dim}") do
       @command_config = install_config.copy_with(prefer_offline: true)
     end
-    parser.on("--frozen-lockfile <true|false>", "If true, will fail if the lockfile is outdated. #{"[env: ZAP_INSTALL_FROZEN_LOCKFILE]".colorize.dim}") do |frozen_lockfile|
-      @command_config = install_config.copy_with(frozen_lockfile: Utils::Various.str_to_bool(frozen_lockfile))
+    flag("--production", "If true, will not install devDependencies.") do
+      @command_config = install_config.copy_with(omit: [Commands::Install::Config::Omit::Dev])
     end
 
     subSeparator("Strategies")
 
-    parser.on(
+    flag(
       "--install-strategy <classic|classic_shallow|isolated>",
       <<-DESCRIPTION
       The strategy used to install packages. #{"[env: ZAP_INSTALL_STRATEGY]".colorize.dim}
@@ -44,36 +44,36 @@ class Zap::CLI
       @command_config = install_config.copy_with(strategy: InstallConfig::InstallStrategy.parse(strategy))
     end
 
-    parser.on("--classic", "Shorthand for: --install-strategy classic") do
+    flag("--classic", "Shorthand for: --install-strategy classic") do
       @command_config = install_config.copy_with(strategy: InstallConfig::InstallStrategy::Classic)
     end
 
-    parser.on("--isolated", "Shorthand for: --install-strategy isolated") do
+    flag("--isolated", "Shorthand for: --install-strategy isolated") do
       @command_config = install_config.copy_with(strategy: InstallConfig::InstallStrategy::Isolated)
     end
 
-    parser.on("--pnp", "Shorthand for: --install-strategy pnp") do
+    flag("--pnp", "Shorthand for: --install-strategy pnp") do
       @command_config = install_config.copy_with(strategy: InstallConfig::InstallStrategy::Pnp)
     end
 
     subSeparator("Save")
 
     unless update_packages
-      parser.on("-D", "--save-dev", "Added packages will appear in your devDependencies. #{"[env: ZAP_INSTALL_SAVE_DEV]".colorize.dim}") do
+      flag("--no-save", "Prevents saving to dependencies. #{"[env: ZAP_INSTALL_SAVE=false]".colorize.dim}") do
+        @command_config = install_config.copy_with(save: false)
+      end
+      flag("-D", "--save-dev", "Added packages will appear in your devDependencies. #{"[env: ZAP_INSTALL_SAVE_DEV]".colorize.dim}") do
         @command_config = install_config.copy_with(save_dev: true)
       end
-      parser.on("-E", "--save-exact", "Saved dependencies will be configured with an exact version rather than using npm's default semver range operator. #{"[env: ZAP_INSTALL_SAVE_EXACT]".colorize.dim}") do |path|
+      flag("-E", "--save-exact", "Saved dependencies will be configured with an exact version rather than using npm's default semver range operator. #{"[env: ZAP_INSTALL_SAVE_EXACT]".colorize.dim}") do |path|
         @command_config = install_config.copy_with(save_exact: true)
       end
-      parser.on("-O", "--save-optional", "Added packages will appear in your optionalDependencies. #{"[env: ZAP_INSTALL_SAVE_OPTIONAL]".colorize.dim}") do
+      flag("-O", "--save-optional", "Added packages will appear in your optionalDependencies. #{"[env: ZAP_INSTALL_SAVE_OPTIONAL]".colorize.dim}") do
         @command_config = install_config.copy_with(save_optional: true)
       end
-      parser.on("-P", "--save-prod", "Added packages will appear in your dependencies. #{"[env: ZAP_INSTALL_SAVE_PROD]".colorize.dim}") do
+      flag("-P", "--save-prod", "Added packages will appear in your dependencies. #{"[env: ZAP_INSTALL_SAVE_PROD]".colorize.dim}") do
         @command_config = install_config.copy_with(save_prod: true)
       end
-      parser.on("--no-save", "Prevents saving to dependencies. #{"[env: ZAP_INSTALL_SAVE=false]".colorize.dim}") do
-        @command_config = install_config.copy_with(save: false)
-      end
     end
 
     parser.missing_option do |option|
diff --git a/src/cli/run.cr b/src/cli/run.cr
index c1aeae1..5064ae9 100644
--- a/src/cli/run.cr
+++ b/src/cli/run.cr
@@ -8,11 +8,11 @@ class Zap::CLI
 
     separator("Options")
 
-    parser.on("--if-present", "Will prevent exiting with status code != 0 when the script is not found. #{"[env: ZAP_RUN_IF_PRESENT]".colorize.dim}") do
+    flag("--if-present", "Will prevent exiting with status code != 0 when the script is not found. #{"[env: ZAP_RUN_IF_PRESENT]".colorize.dim}") do
       @command_config = run_config.copy_with(if_present: true)
     end
 
-    parser.on("--parallel", "Run all scripts in parallel without any kind of topological ordering #{"[env: ZAP_RUN_PARALLEL]".colorize.dim}.") do
+    flag("--parallel", "Run all scripts in parallel without any kind of topological ordering #{"[env: ZAP_RUN_PARALLEL]".colorize.dim}.") do
       @command_config = run_config.copy_with(parallel: true)
     end
 
diff --git a/src/cli/store.cr b/src/cli/store.cr
index 1a89ef8..30c8cde 100644
--- a/src/cli/store.cr
+++ b/src/cli/store.cr
@@ -5,22 +5,22 @@ class Zap::CLI
     @command_config = nil
     separator("Options")
 
-    command("path", "Prints the path to the zap store.") do
-      @command_config = StoreConfig.new(action: StoreConfig::StoreAction::PrintPath)
-    end
-
-    command("clear", "Clears the zap store.") do
+    command("clear", "Clear the zap store.") do
       @command_config = StoreConfig.new(action: StoreConfig::StoreAction::Clear)
     end
 
-    command("clear-http-cache", "Clears the cached registry responses.") do
+    command("clear-http-cache", "Clear the cached registry responses.") do
       @command_config = StoreConfig.new(action: StoreConfig::StoreAction::ClearHttpCache)
     end
 
-    command("clear-packages", "Clears the stored packages.") do
+    command("clear-packages", "Clear the stored packages.") do
       @command_config = StoreConfig.new(action: StoreConfig::StoreAction::ClearPackages)
     end
 
+    command("path", "Print the path to the zap store.") do
+      @command_config = StoreConfig.new(action: StoreConfig::StoreAction::PrintPath)
+    end
+
     parser.before_each do |arg|
       if @command_config.nil? && !parser.@handlers.keys.includes?(arg)
         puts parser
diff --git a/src/cli/why.cr b/src/cli/why.cr
index 5098798..e5295d8 100644
--- a/src/cli/why.cr
+++ b/src/cli/why.cr
@@ -6,7 +6,7 @@ class Zap::CLI
 
     separator("Options")
 
-    parser.on("--short", "Do not display the dependencies paths. #{"[env: ZAP_WHY_SHORT]".colorize.dim}") do
+    flag("--short", "Do not display the dependencies paths. #{"[env: ZAP_WHY_SHORT]".colorize.dim}") do
       @command_config = why_config.copy_with(short: true)
     end
 
diff --git a/src/ext/option_parser.cr b/src/ext/option_parser.cr
index 187587b..9ae30fc 100644
--- a/src/ext/option_parser.cr
+++ b/src/ext/option_parser.cr
@@ -22,4 +22,26 @@ class OptionParser
     flag, value_type = parse_flag_definition(flag)
     @handlers[flag] = Handler.new(value_type, block)
   end
+
+  def on(short_flag : String, long_flag : String, description : String, flag_formatter : String -> String, &block : String ->)
+    check_starts_with_dash short_flag, "short_flag", allow_empty: true
+    check_starts_with_dash long_flag, "long_flag"
+
+    append_flag "#{short_flag}, #{long_flag}", description, flag_formatter
+
+    short_flag, short_value_type = parse_flag_definition(short_flag)
+    long_flag, long_value_type = parse_flag_definition(long_flag)
+
+    # Pick the "most required" argument type between both flags
+    if short_value_type.required? || long_value_type.required?
+      value_type = FlagValue::Required
+    elsif short_value_type.optional? || long_value_type.optional?
+      value_type = FlagValue::Optional
+    else
+      value_type = FlagValue::None
+    end
+
+    handler = Handler.new(value_type, block)
+    @handlers[short_flag] = @handlers[long_flag] = handler
+  end
 end