Merge pull request #437 from rabbitmq/mk-help-command-exit-status

Make --help, help usage help messages consistent

(cherry picked from commit 49d4d39)

lib/rabbitmq/cli/auto_complete.ex

@@ -13,9 +13,12 @@
 ## The Initial Developer of the Original Code is GoPivotal, Inc.
 ## Copyright (c) 2007-2020 VMware, Inc. or its affiliates.  All rights reserved.
 
-defmodule Rabbitmq.CLI.AutoComplete do
+defmodule RabbitMQ.CLI.AutoComplete do
   alias RabbitMQ.CLI.Core.{CommandModules, Parser}
 
+  # Use the same jaro distance limit as in Elixir's "did you mean?"
+  @jaro_distance_limit 0.77
+
   @spec complete(String.t(), [String.t()]) :: [String.t()]
   def complete(_, []) do
     []
@@ -31,6 +34,26 @@ defmodule Rabbitmq.CLI.AutoComplete do
     end
   end
 
+  def suggest_command(_cmd_name, empty) when empty == %{} do
+    nil
+  end
+  def suggest_command(typed, module_map) do
+    suggestion =
+      module_map
+      |> Map.keys()
+      |> Enum.map(fn existing ->
+        {existing, String.jaro_distance(existing, typed)}
+      end)
+      |> Enum.max_by(fn {_, distance} -> distance end)
+
+    case suggestion do
+      {cmd, distance} when distance >= @jaro_distance_limit ->
+        {:suggest, cmd}
+      _ ->
+        nil
+    end
+  end
+
   defp complete(tokens) do
     {command, command_name, _, _, _} = Parser.parse(tokens)
     last_token = List.last(tokens)

lib/rabbitmq/cli/core/parser.ex

@@ -17,9 +17,6 @@ defmodule RabbitMQ.CLI.Core.Parser do
   alias RabbitMQ.CLI.{CommandBehaviour, FormatterBehaviour}
   alias RabbitMQ.CLI.Core.{CommandModules, Config}
 
-  # Use the same jaro distance limit as in Elixir `did_you_mean`
-  @jaro_distance_limit 0.77
-
   def default_switches() do
     [
       node: :atom,
@@ -90,6 +87,13 @@ defmodule RabbitMQ.CLI.Core.Parser do
     end
   end
 
+  def command_suggestion(_cmd_name, empty) when empty == %{} do
+    nil
+  end
+  def command_suggestion(typed, module_map) do
+    RabbitMQ.CLI.AutoComplete.suggest_command(typed, module_map)
+  end
+
   defp look_up_command(parsed_args, options) do
     case parsed_args do
       [cmd_name | arguments] ->
@@ -164,27 +168,6 @@ defmodule RabbitMQ.CLI.Core.Parser do
     end
   end
 
-  defp command_suggestion(_cmd_name, empty) when empty == %{} do
-    nil
-  end
-
-  defp command_suggestion(typed, module_map) do
-    suggestion =
-      module_map
-      |> Map.keys()
-      |> Enum.map(fn existing ->
-        {existing, String.jaro_distance(existing, typed)}
-      end)
-      |> Enum.max_by(fn {_, distance} -> distance end)
-
-    case suggestion do
-      {cmd, distance} when distance >= @jaro_distance_limit ->
-        {:suggest, cmd}
-      _ ->
-        nil
-    end
-  end
-
   defp parse_alias(input, command_name, module, alias_content, options) do
     {pre_command_options, tail, invalid} = parse_global_head(input)
     [^command_name | other] = tail

lib/rabbitmq/cli/ctl/commands/help_command.ex

@@ -29,32 +29,52 @@ defmodule RabbitMQ.CLI.Ctl.Commands.HelpCommand do
   def distribution(_), do: :none
   use RabbitMQ.CLI.Core.MergesNoDefaults
 
-  def validate(_, _), do: :ok
+  def validate([], _), do: :ok
+  def validate([_command], _), do: :ok
+  def validate(args, _) when length(args) > 1 do
+    {:validation_failure, :too_many_args}
+  end
 
   def run([command_name | _], opts) do
     CommandModules.load(opts)
 
-    case CommandModules.module_map()[command_name] do
+    module_map = CommandModules.module_map()
+    case module_map[command_name] do
       nil ->
-        all_usage(opts)
+        # command not found
+        # {:error, all_usage(opts)}
+        case RabbitMQ.CLI.AutoComplete.suggest_command(command_name, module_map) do
+          {:suggest, suggested} ->
+            suggest_message = "\nCommand '#{command_name}' not found. \n" <>
+              "Did you mean '#{suggested}'? \n"
+            {:error, ExitCodes.exit_usage(), suggest_message}
+          nil ->
+            {:error, ExitCodes.exit_usage(), "\nCommand '#{command_name}' not found."}
+        end
 
       command ->
-        command_usage(command, opts)
+        {:ok, command_usage(command, opts)}
     end
   end
 
-  def run(_, opts) do
+  def run([], opts) do
     CommandModules.load(opts)
 
     case opts[:list_commands] do
-      true -> commands_description()
-      _ -> all_usage(opts)
+      true ->
+        {:ok, commands_description()}
+      _ ->
+        {:ok, all_usage(opts)}
     end
   end
 
-  def output(result, _) do
-    {:error, ExitCodes.exit_ok(), result}
+  def output({:ok, result}, _) do
+    {:ok, result}
   end
+  def output({:error, result}, _) do
+    {:error, ExitCodes.exit_usage(), result}
+  end
+  use RabbitMQ.CLI.DefaultOutput
 
   def banner(_, _), do: nil
 
@@ -77,12 +97,9 @@ defmodule RabbitMQ.CLI.Ctl.Commands.HelpCommand do
 
   def all_usage(opts) do
     tool_name = program_name(opts)
-    Enum.join(
-      tool_usage(tool_name) ++
-        [Enum.join(["Available commands:"] ++ commands_description(), "\n")] ++
-        help_footer(tool_name),
-      "\n\n"
-    ) <> "\n"
+    tool_usage(tool_name) ++
+        ["\n\nAvailable commands:\n"] ++ commands_description() ++
+        help_footer(tool_name)
   end
 
   def command_usage(command, opts) do
@@ -212,7 +229,7 @@ short            | long          | description
         end
       end)
 
-    module_map
+    lines = module_map
     |> Enum.map(
       fn({name, cmd}) ->
         description = CommandBehaviour.description(cmd)
@@ -223,6 +240,7 @@ short            | long          | description
     |> Enum.sort_by(
       fn({help_section, _}) ->
         case help_section do
+          :deprecated -> 999
           :other -> 100
           {:plugin, _} -> 99
           :help -> 1
@@ -241,16 +259,18 @@ short            | long          | description
     |> Enum.map(
       fn({help_section, section_helps}) ->
         [
-          "\n" <> bright(section_head(help_section)) <> ":\n" |
+          "\n" <> bright(section_head(help_section)) <> ":\n\n" |
           Enum.sort(section_helps)
           |> Enum.map(
             fn({name, {description, _}}) ->
-              "   #{String.pad_trailing(name, pad_commands_to)}  #{description}"
+              "   #{String.pad_trailing(name, pad_commands_to)}  #{description}\n"
             end)
         ]
 
       end)
     |> Enum.concat()
+
+    lines ++ ["\n"]
   end
 
   defp section_head(help_section) do

lib/rabbitmqctl.ex

@@ -59,7 +59,7 @@ defmodule RabbitMQCtl do
 
     # the user asked for --help and we are displaying it to her,
     # reporting a success
-    {:ok, ExitCodes.exit_ok(), HelpCommand.all_usage(parsed_options)};
+    {:ok, ExitCodes.exit_ok(), Enum.join(HelpCommand.all_usage(parsed_options), "")};
   end
 
   def exec_command(["--version"] = _unparsed_command, opts) do
@@ -80,7 +80,7 @@ defmodule RabbitMQCtl do
 
         usage_string =
           command_not_found_string <>
-            HelpCommand.all_usage(parsed_options)
+            Enum.join(HelpCommand.all_usage(parsed_options), "")
 
         {:error, ExitCodes.exit_usage(), usage_string}
 
@@ -249,7 +249,7 @@ defmodule RabbitMQCtl do
   end
 
   def auto_complete(script_name, args) do
-    Rabbitmq.CLI.AutoComplete.complete(script_name, args)
+    RabbitMQ.CLI.AutoComplete.complete(script_name, args)
     |> Stream.map(&IO.puts/1)
     |> Stream.run()
 

test/core/auto_complete_test.exs

@@ -17,7 +17,7 @@
 defmodule AutoCompleteTest do
   use ExUnit.Case, async: false
 
-  @subject Rabbitmq.CLI.AutoComplete
+  @subject RabbitMQ.CLI.AutoComplete
 
 
   test "Auto-completes a command" do

test/ctl/help_command_test.exs

@@ -18,7 +18,7 @@ defmodule HelpCommandTest do
   use ExUnit.Case, async: false
   import TestHelper
 
-  alias RabbitMQ.CLI.Core.{CommandModules, ExitCodes}
+  alias RabbitMQ.CLI.Core.{CommandModules}
 
   @command RabbitMQ.CLI.Ctl.Commands.HelpCommand
 
@@ -27,8 +27,22 @@ defmodule HelpCommandTest do
     :ok
   end
 
+  test "validate: providing no position arguments passes validation" do
+    assert @command.validate([], %{}) == :ok
+  end
+
+  test "validate: providing one position argument passes validation" do
+    assert @command.validate(["status"], %{}) == :ok
+  end
+
+  test "validate: providing two or more position arguments fails validation" do
+    assert @command.validate(["extra1", "extra2"], %{}) ==
+      {:validation_failure, :too_many_args}
+  end
+
   test "run: prints basic usage info" do
-    output = @command.run([], %{})
+    {:ok, lines} = @command.run([], %{})
+    output = Enum.join(lines, "\n")
     assert output =~ ~r/[-n <node>] [-t <timeout>]/
     assert output =~ ~r/commands/i
   end
@@ -49,9 +63,6 @@ defmodule HelpCommandTest do
   end
 
   test "run prints command info" do
-    assert @command.run([], %{}) =~ ~r/commands/i
-
-    # Checks to verify that each module's command appears in the list.
     ctl_commands = CommandModules.module_map
     |> Enum.filter(fn({_name, command_mod}) ->
                      to_string(command_mod) =~ ~r/^RabbitMQ\.CLI\.Ctl\.Commands/
@@ -61,20 +72,14 @@ defmodule HelpCommandTest do
     Enum.each(
       ctl_commands,
       fn(command) ->
-        assert @command.run([], %{}) =~ ~r/\n\s+#{command}.*\n/
+        {:ok, lines} = @command.run([], %{})
+        output = Enum.join(lines, "\n")
+        assert output =~ ~r/\n\s+#{command}.*\n/
       end)
   end
 
-  test "run: exits with code of OK" do
-    assert @command.output("Help string", %{}) ==
-      {:error, ExitCodes.exit_ok, "Help string"}
-  end
-
-  test "run: no arguments print general help" do
-    assert @command.run([], %{}) =~ ~r/usage/i
-  end
-
-  test "run: unrecognised arguments print general help" do
-    assert @command.run(["extra1", "extra2"], %{}) =~ ~r/usage/i
+  test "run: exits with the code of OK" do
+    assert @command.output({:ok, "Help string"}, %{}) ==
+      {:ok, "Help string"}
   end
 end