improve Command#usage_string and tighten specs

Author: DannyBen

lib/bashly/config_validator.rb

@@ -112,7 +112,6 @@ def assert_command(key, value)
       assert_hash key, value, Script::Command.option_keys
 
       refute value['commands'] && value['args'], "#{key} cannot have both commands and args"
-      # refute value['commands'] && value['flags'], "#{key} cannot have both commands and flags"
 
       assert_string "#{key}.name", value['name']
       assert_optional_string "#{key}.help", value['help']
@@ -151,6 +150,10 @@ def assert_command(key, value)
         refute repeatable_arg, "#{key}.catch_all makes no sense with repeatable arg (#{repeatable_arg})"
       end
 
+      if value['catch_all']
+        refute value['commands'], "#{key}.catch_all makes no sense with commands"
+      end
+
       if value['expose']
         assert value['commands'], "#{key}.expose makes no sense without commands"
       end

lib/bashly/script/command.rb

@@ -192,6 +192,19 @@ def group_string
         end
       end
 
+      # Returns a mode identifier
+      def mode
+        @mode ||= begin
+          if global_flags?               then :global_flags
+          elsif commands.any?            then :commands
+          elsif args.any? and flags.any? then :args_and_flags
+          elsif args.any?                then :args
+          elsif flags.any?               then :flags
+          else                           :empty
+          end
+        end
+      end
+
       # Returns an array of all parents. For example, the command 
       # "docker container run" will have [docker, container] as its parents
       def parents
@@ -245,14 +258,23 @@ def summary_string
       # Returns a constructed string suitable for Usage pattern
       def usage_string
         result = [full_name]
-        result << "[OPTIONS]" if global_flags?
-        result << "COMMAND" if commands.any?
-        args.each do |arg|
-          result << arg.usage_string
+
+        result.push case mode
+        when :global_flags    then ["[OPTIONS]", "COMMAND"]
+        when :commands        then ["COMMAND"]
+        when :args_and_flags  then usage_string_args + ["[OPTIONS]"]
+        when :args            then usage_string_args
+        when :flags           then ["[OPTIONS]"]
+        else                  nil
         end
-        result << "[OPTIONS]" unless flags.empty? || global_flags?
-        result << catch_all.usage_string if catch_all.enabled?
-        result.join " "
+
+        result.push catch_all.usage_string if catch_all.enabled? && commands.empty?
+        result.compact.join " "
+      end
+
+      # Returns an array of args usage_string for the command's usage_string
+      def usage_string_args
+        args.map { |arg| arg.usage_string }
       end
 
       # Returns an array of files to include as is inside the script

spec/approvals/validations/command_catch_all_commands

@@ -0,0 +1 @@
+#<Bashly::ConfigurationError: root.catch_all makes no sense with commands>

spec/bashly/script/command_spec.rb

@@ -140,7 +140,7 @@
     it "returns a Command object of the first default command" do
       expect(subject.default_command).to be_a Script::Command
       expect(subject.default_command.name).to eq 'get'
-    end    
+    end
   end
 
   describe '#default_flags' do
@@ -238,6 +238,32 @@
     end
   end
 
+  describe '#global_flags?' do
+    context "when a command has flags and commands" do
+      let(:fixture) { :mode_global_flags }
+
+      it "returns true" do
+        expect(subject.global_flags?).to be_truthy
+      end
+    end
+
+    context "when a command has flags but no commands" do
+      let(:fixture) { :mode_flags }
+
+      it "returns false" do
+        expect(subject.global_flags?).to be_falsy
+      end
+    end
+
+    context "when a command has commands but no flags" do
+      let(:fixture) { :mode_commands }
+
+      it "returns false" do
+        expect(subject.global_flags?).to be_falsy
+      end
+    end
+  end
+
   describe '#group_string' do
     it "returns a string suitable for showing the group in usage" do
       expect(subject.group_string).to eq "Commands:"
@@ -352,42 +378,118 @@
     end
   end
 
+  describe '#mode' do
+    context "when flags and commands are defined" do
+      let(:fixture) { :mode_global_flags }
+
+      it "returns :global_flags" do
+        expect(subject.mode).to eq :global_flags
+      end
+    end
+
+    context "when only commands are defined" do
+      let(:fixture) { :mode_commands }
+
+      it "returns :commands" do
+        expect(subject.mode).to eq :commands
+      end
+    end
+
+    context "when args and flags are defined" do
+      let(:fixture) { :mode_args_and_flags }
+
+      it "returns :args_and_flags" do
+        expect(subject.mode).to eq :args_and_flags
+      end
+    end
+
+    context "when only args are defined" do
+      let(:fixture) { :mode_args }
+
+      it "returns :args" do
+        expect(subject.mode).to eq :args
+      end
+    end
+
+    context "when only flags are defined" do
+      let(:fixture) { :mode_flags }
+
+      it "returns :flags" do
+        expect(subject.mode).to eq :flags
+      end
+    end
+
+    context "when nothing is defined" do
+      let(:fixture) { :mode_empty }
+
+      it "returns :empty" do
+        expect(subject.mode).to eq :empty
+      end
+    end
+  end
+
   describe '#usage_string' do
-    context "when no args and no commands are defined" do
-      let(:fixture) { :git_status }
+    context "when flags and commands are defined" do
+      let(:fixture) { :mode_global_flags }
 
-      it "returns a string suitable to be used as a usage pattern" do
-        expect(subject.usage_string).to eq "git status"
+      it "returns the correct string" do
+        expect(subject.usage_string).to eq "get [OPTIONS] COMMAND"
       end
     end
 
-    context "when flags are defined" do
-      let(:fixture) { :flags_only_command }
+    context "when only commands are defined" do
+      let(:fixture) { :mode_commands }
 
-      it "adds [OPTIONS] to the usage string" do
-        expect(subject.usage_string).to eq "git status [OPTIONS]"
-      end      
+      it "returns the correct string" do
+        expect(subject.usage_string).to eq "get COMMAND"
+      end
     end
 
-    context "when args are defined" do
-      it "includes them in the usage string" do
-        expect(subject.usage_string).to eq "get SOURCE [TARGET] [OPTIONS]"
-      end      
+    context "when args and flags are defined" do
+      let(:fixture) { :mode_args_and_flags }
+
+      it "returns the correct string" do
+        expect(subject.usage_string).to eq "get SOURCE [TARGET] [OPTIONS] [PARAMS...]"
+      end
+    end
+
+    context "when only args are defined" do
+      let(:fixture) { :mode_args }
+
+      it "returns the correct string" do
+        expect(subject.usage_string).to eq "get SOURCE [TARGET] [PARAMS...]"
+      end
     end
 
-    context "when commands are defined" do
-      let(:fixture) { :docker }
+    context "when only flags are defined" do
+      let(:fixture) { :mode_flags }
 
-      it "includes [command] in the usage string" do
-        expect(subject.usage_string).to eq "docker COMMAND"
-      end      
+      it "returns the correct string" do
+        expect(subject.usage_string).to eq "get [OPTIONS] [PARAMS...]"
+      end
+
+      context "when it has a parent" do
+        let(:fixture) { :flags_only_command }
+
+        it "returns the correct string" do
+          expect(subject.usage_string).to eq "git status [OPTIONS]"
+        end
+      end
     end
 
-    context "when catch_all is enabled" do
-      let(:fixture) { :catch_all }
+    context "when nothing is defined" do
+      let(:fixture) { :mode_empty }
+
+      it "returns the correct string" do
+        expect(subject.usage_string).to eq "get [PARAMS...]"
+      end
+
+      context "when it has a parent" do
+        let(:fixture) { :git_status }
 
-      it "includes the catch_all usage string" do
-        expect(subject.usage_string).to eq "get [...]"
+        it "returns the correct string" do
+          expect(subject.usage_string).to eq "git status"
+        end
       end
     end
   end

spec/fixtures/script/commands.yml

@@ -217,6 +217,70 @@
   name: get
   help: get something from somewhere
 
+:mode_global_flags:
+  name: get
+  help: get something from somewhere
+
+  flags:
+  - long: --force
+  - long: --verbose
+
+  commands:
+  - name: download
+    flags:
+    - long: --http
+  - name: upload
+
+:mode_commands:
+  name: get
+  help: get something from somewhere
+
+  commands:
+  - name: download
+    flags:
+    - long: --http
+  - name: upload
+
+:mode_args_and_flags:
+  name: get
+  help: get something from somewhere
+  catch_all: params
+
+  args:
+  - name: source
+    required: true
+  - name: target
+
+  flags:
+  - long: --force
+    required: true
+  - long: --verbose
+
+:mode_args:
+  name: get
+  help: get something from somewhere
+  catch_all: params
+
+  args:
+  - name: source
+    required: true
+  - name: target
+
+:mode_flags:
+  name: get
+  help: get something from somewhere
+  catch_all: params
+
+  flags:
+  - long: --force
+    required: true
+  - long: --verbose
+
+:mode_empty:
+  name: get
+  help: get something from somewhere
+  catch_all: params
+
 :nested_aliases:
   name: cli
   commands:

spec/fixtures/script/validations.yml

@@ -36,6 +36,14 @@
     help: catch_all.required should be boolean
     required: 1
 
+:command_catch_all_commands:
+  name: invalid
+  catch_all:
+    label: params
+    help: catch_all makes no sense with commands
+  commands:
+  - name: config
+
 :command_expose_invalid_type:
   name: invalid
   help: expose should be boolean or 'always'