From 48b7295ae835b762df449e0dc41984c91c6d4c83 Mon Sep 17 00:00:00 2001
From: Luis Lavena <luislavena@users.noreply.github.com>
Date: Thu, 15 Aug 2024 11:06:17 +0200
Subject: [PATCH] Forward unmodified ARGV to subcommand (#631)

* Pass all ARGV arguments verbatim to subcommand

Avoid altering original `ARGV` when combining it with possible
`SHARDS_OPTS` variable and pass them verbatim to the subcommand.

Introduce a naive integration test for subcommand to validate the change
works correctly.

* Fixes passing --help to subcommand

Avoid OptionParser to short-circuit `--help` and return immediately by
setting a flag for it and evaluating at the end of the processing of
unknown options.

This is only done for the CLI invocation and is not part of Shards
module (as the help and usage options are only available in this
context).

* Only use dummy executable on Windows

Follow the pattern used in other tests and use simple `sh` script on
non-Windows platforms.

* Avoid usage of class properties to handle help behavior

Revert the usage of class property introduced in
5a6dc6cb28973f22758532a76e4585cb13969cfc and leverage instead on a pure
instance variable in the context of `Shards.run`.
---
 spec/integration/subcommand_spec.cr | 41 +++++++++++++++++++++++++++++
 src/cli.cr                          | 12 ++++++---
 2 files changed, 50 insertions(+), 3 deletions(-)
 create mode 100644 spec/integration/subcommand_spec.cr

diff --git a/spec/integration/subcommand_spec.cr b/spec/integration/subcommand_spec.cr
new file mode 100644
index 00000000..89226a04
--- /dev/null
+++ b/spec/integration/subcommand_spec.cr
@@ -0,0 +1,41 @@
+require "./spec_helper"
+
+describe "subcommand" do
+  it "forwards all arguments to subcommand" do
+    create_shard("dummy", "0.1.0")
+    {% if flag?(:win32) %}
+      create_executable "dummy", "bin/shards-dummy", %(print ARGV.join(" "))
+    {% else %}
+      path = create_file("dummy", "bin/shards-dummy", "#!/bin/sh\necho $@\n")
+      File.chmod(path, 0o755)
+    {% end %}
+
+    with_path(git_path("dummy/bin")) do
+      output = run("shards dummy --no-color --verbose --unknown other argument")
+      output.should contain(%(--no-color --verbose --unknown other argument))
+    end
+  end
+
+  it "correctly forwards '--help' option to subcommand" do
+    create_shard("dummy", "0.1.0")
+    {% if flag?(:win32) %}
+      create_executable "dummy", "bin/shards-dummy", %(print ARGV.join(" "))
+    {% else %}
+      path = create_file("dummy", "bin/shards-dummy", "#!/bin/sh\necho $@\n")
+      File.chmod(path, 0o755)
+    {% end %}
+
+    with_path(git_path("dummy/bin")) do
+      output = run("shards dummy --help")
+      output.should contain(%(--help))
+    end
+  end
+end
+
+private def with_path(path)
+  old_path = ENV["PATH"]
+  ENV["PATH"] = "#{File.expand_path(path)}#{Process::PATH_DELIMITER}#{ENV["PATH"]}"
+  yield
+ensure
+  ENV["PATH"] = old_path
+end
diff --git a/src/cli.cr b/src/cli.cr
index 668c825e..beb8ad24 100644
--- a/src/cli.cr
+++ b/src/cli.cr
@@ -26,6 +26,8 @@ module Shards
   end
 
   def self.run
+    display_help = false
+
     OptionParser.parse(cli_options) do |opts|
       path = Dir.current
 
@@ -53,7 +55,7 @@ module Shards
       opts.on("--ignore-crystal-version", "Has no effect. Kept for compatibility, to be removed in the future.") { }
       opts.on("-v", "--verbose", "Increase the log verbosity, printing all debug statements.") { self.set_debug_log_level }
       opts.on("-q", "--quiet", "Decrease the log verbosity, printing only warnings and errors.") { self.set_warning_log_level }
-      opts.on("-h", "--help", "Print usage synopsis.") { self.display_help_and_exit(opts) }
+      opts.on("-h", "--help", "Print usage synopsis.") { display_help = true }
 
       opts.unknown_args do |args, options|
         case args[0]? || DEFAULT_COMMAND
@@ -101,12 +103,16 @@ module Shards
         else
           program_name = "shards-#{args[0]}"
           if program_path = Process.find_executable(program_name)
-            run_shards_subcommand(program_path, args)
+            run_shards_subcommand(program_path, cli_options)
           else
             display_help_and_exit(opts)
           end
         end
 
+        if display_help
+          display_help_and_exit(opts)
+        end
+
         exit
       end
     end
@@ -119,7 +125,7 @@ module Shards
     {% else %}
       shards_opts = ENV.fetch("SHARDS_OPTS", "").split
     {% end %}
-    ARGV.concat(shards_opts)
+    ARGV.dup.concat(shards_opts)
   end
 
   def self.run_shards_subcommand(process_name, args)