Fix behavior around commands with optional arguments4273-slash-commands

author: Douwe Maan <douwe@selenight.nl> 2016-08-18 14:21:52 -0500
committer: Douwe Maan <douwe@selenight.nl> 2016-08-18 14:29:49 -0500
commit: 2703330a19e813351e9c33241a59d6b7f54741df (patch)
tree: 0caa4c5a119b165fd4fe9e3712f0de5caffa403e
parent: 6f2f2a6baae128cd4f8111d91bdcfb7b76e39b97 (diff)
download: gitlab-ce-2703330a19e813351e9c33241a59d6b7f54741df.tar.gz
5 files changed, 63 insertions, 42 deletions
diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb
index 0d7838055fe..9ac1124abc1 100644
--- a/app/services/slash_commands/interpret_service.rb
+++ b/app/services/slash_commands/interpret_service.rb
@@ -18,11 +18,11 @@ module SlashCommands
 
       content, commands = extractor.extract_commands(content, opts)
 
-      commands.each do |name, args|
+      commands.each do |name, arg|
         definition = self.class.command_definitions_by_name[name.to_sym]
         next unless definition
 
-        definition.execute(self, opts, args)
+        definition.execute(self, opts, arg)
       end
 
       [content, @updates]
diff --git a/lib/gitlab/slash_commands/command_definition.rb b/lib/gitlab/slash_commands/command_definition.rb
index 2ff8f4eddf0..60d35be2599 100644
--- a/lib/gitlab/slash_commands/command_definition.rb
+++ b/lib/gitlab/slash_commands/command_definition.rb
@@ -28,13 +28,14 @@ module Gitlab
         context.instance_exec(&condition_block)
       end
 
-      def execute(context, opts, args)
+      def execute(context, opts, arg)
         return if noop? || !available?(opts)
 
-        block_arity = action_block.arity
-        return unless (args.present? && block_arity == 1) || (args.blank? && block_arity <= 0)
-
-        context.instance_exec(args, &action_block)
+        if arg.present?
+          context.instance_exec(arg, &action_block)
+        elsif action_block.arity == 0
+          context.instance_exec(&action_block)
+        end
       end
 
       def to_h(opts)
diff --git a/lib/gitlab/slash_commands/extractor.rb b/lib/gitlab/slash_commands/extractor.rb
index c790b825347..a672e5e4855 100644
--- a/lib/gitlab/slash_commands/extractor.rb
+++ b/lib/gitlab/slash_commands/extractor.rb
@@ -39,7 +39,7 @@ module Gitlab
         content.delete!("\r")
         content.gsub!(commands_regex(opts)) do
           if $~[:cmd]
-            commands << [$~[:cmd], $~[:args]].reject(&:blank?)
+            commands << [$~[:cmd], $~[:arg]].reject(&:blank?)
             ''
           else
             $~[0]
@@ -50,13 +50,14 @@ module Gitlab
       end
 
       private
+
       # Builds a regular expression to match known commands.
       # First match group captures the command name and
       # second match group captures its arguments.
       #
       # It looks something like:
       #
-      #   /^\/(?<cmd>close|reopen|...)(?:( |$))(?<args>[^\/\n]*)(?:\n|$)/
+      #   /^\/(?<cmd>close|reopen|...)(?:( |$))(?<arg>[^\/\n]*)(?:\n|$)/
       def commands_regex(opts)
         names = command_names(opts).map(&:to_s)
 
@@ -64,7 +65,7 @@ module Gitlab
             (?<code>
               # Code blocks:
               # ```
-              # Anything, including `/cmd args` which are ignored by this filter
+              # Anything, including `/cmd arg` which are ignored by this filter
               # ```
 
               ^```
@@ -75,7 +76,7 @@ module Gitlab
             (?<html>
               # HTML block:
               # <tag>
-              # Anything, including `/cmd args` which are ignored by this filter
+              # Anything, including `/cmd arg` which are ignored by this filter
               # </tag>
 
               ^<[^>]+?>\n
@@ -86,7 +87,7 @@ module Gitlab
             (?<html>
               # Quote block:
               # >>>
-              # Anything, including `/cmd args` which are ignored by this filter
+              # Anything, including `/cmd arg` which are ignored by this filter
               # >>>
 
               ^>>>
@@ -102,7 +103,7 @@ module Gitlab
               (?<cmd>#{Regexp.union(names)})
               (?:
                 [ ]
-                (?<args>[^\/\n]*)
+                (?<arg>[^\/\n]*)
               )?
               (?:\n|$)
             )
diff --git a/spec/lib/gitlab/slash_commands/command_definition_spec.rb b/spec/lib/gitlab/slash_commands/command_definition_spec.rb
index 2a75fab24b0..c9c2f314e57 100644
--- a/spec/lib/gitlab/slash_commands/command_definition_spec.rb
+++ b/spec/lib/gitlab/slash_commands/command_definition_spec.rb
@@ -24,7 +24,7 @@ describe Gitlab::SlashCommands::CommandDefinition do
   describe "#noop?" do
     context "when the command has an action block" do
       before do
-        subject.action_block = -> { }
+        subject.action_block = proc { }
       end
 
       it "returns false" do
@@ -44,7 +44,7 @@ describe Gitlab::SlashCommands::CommandDefinition do
 
     context "when the command has a condition block" do
       before do
-        subject.condition_block = -> { go }
+        subject.condition_block = proc { go }
       end
 
       context "when the condition block returns true" do
@@ -78,7 +78,7 @@ describe Gitlab::SlashCommands::CommandDefinition do
       it "doesn't execute the command" do
         expect(context).not_to receive(:instance_exec)
 
-        subject.execute(context, {})
+        subject.execute(context, {}, nil)
 
         expect(context.run).to be false
       end
@@ -86,52 +86,82 @@ describe Gitlab::SlashCommands::CommandDefinition do
 
     context "when the command is not a noop" do
       before do
-        subject.action_block = -> { self.run = true }
+        subject.action_block = proc { self.run = true }
       end
 
       context "when the command is not available" do
         before do
-          subject.condition_block = -> { false }
+          subject.condition_block = proc { false }
         end
 
         it "doesn't execute the command" do
-          subject.execute(context, {})
+          subject.execute(context, {}, nil)
 
           expect(context.run).to be false
         end
       end
 
       context "when the command is available" do
-        context "when the command has an exact number of arguments" do
+        context "when the commnd has no arguments" do
           before do
-            subject.action_block = ->(arg) { self.run = arg }
+            subject.action_block = proc { self.run = true }
           end
 
-          context "when the command is provided a wrong number of arguments" do
-            it "doesn't execute the command" do
-              subject.execute(context, {}, true, true)
+          context "when the command is provided an argument" do
+            it "executes the command" do
+              subject.execute(context, {}, true)
 
-              expect(context.run).to be false
+              expect(context.run).to be true
             end
           end
 
-          context "when the command is provided the right number of arguments" do
+          context "when the command is not provided an argument" do
+            it "executes the command" do
+              subject.execute(context, {}, nil)
+
+              expect(context.run).to be true
+            end
+          end
+        end
+
+        context "when the command has 1 required argument" do
+          before do
+            subject.action_block = ->(arg) { self.run = arg }
+          end
+
+          context "when the command is provided an argument" do
             it "executes the command" do
               subject.execute(context, {}, true)
 
               expect(context.run).to be true
             end
           end
+
+          context "when the command is not provided an argument" do
+            it "doesn't execute the command" do
+              subject.execute(context, {}, nil)
+
+              expect(context.run).to be false
+            end
+          end
         end
 
-        context "when the command has a variable number of arguments" do
+        context "when the command has 1 optional argument" do
           before do
-            subject.action_block = ->(*args) { self.run = args.first }
+            subject.action_block = proc { |arg = nil| self.run = arg || true }
+          end
+
+          context "when the command is provided an argument" do
+            it "executes the command" do
+              subject.execute(context, {}, true)
+
+              expect(context.run).to be true
+            end
           end
 
-          context "when the command is provided any number of arguments" do
+          context "when the command is not provided an argument" do
             it "executes the command" do
-              subject.execute(context, {}, true, true)
+              subject.execute(context, {}, nil)
 
               expect(context.run).to be true
             end
diff --git a/spec/lib/gitlab/slash_commands/dsl_spec.rb b/spec/lib/gitlab/slash_commands/dsl_spec.rb
index 9ec00c25787..26217a0e3b2 100644
--- a/spec/lib/gitlab/slash_commands/dsl_spec.rb
+++ b/spec/lib/gitlab/slash_commands/dsl_spec.rb
@@ -31,16 +31,12 @@ describe Gitlab::SlashCommands::Dsl do
       command :cond_action do |arg|
         arg
       end
-
-      command :wildcard do |*args|
-        args
-      end
     end
   end
 
   describe '.command_definitions' do
     it 'returns an array with commands definitions' do
-      no_args_def, one_arg_def, two_args_def, cc_def, cond_action_def, wildcard_def = DummyClass.command_definitions
+      no_args_def, one_arg_def, two_args_def, cc_def, cond_action_def = DummyClass.command_definitions
 
       expect(no_args_def.name).to eq(:no_args)
       expect(no_args_def.aliases).to eq([:none])
@@ -76,13 +72,6 @@ describe Gitlab::SlashCommands::Dsl do
       expect(cond_action_def.params).to eq([])
       expect(cond_action_def.condition_block).to be_a_kind_of(Proc)
       expect(cond_action_def.action_block).to be_a_kind_of(Proc)
-
-      expect(wildcard_def.name).to eq(:wildcard)
-      expect(wildcard_def.aliases).to eq([])
-      expect(wildcard_def.description).to eq('')
-      expect(wildcard_def.params).to eq([])
-      expect(wildcard_def.condition_block).to be_nil
-      expect(wildcard_def.action_block).to be_a_kind_of(Proc)
     end
   end
 end
author	Douwe Maan <douwe@selenight.nl>	2016-08-18 14:21:52 -0500
committer	Douwe Maan <douwe@selenight.nl>	2016-08-18 14:29:49 -0500
commit	2703330a19e813351e9c33241a59d6b7f54741df (patch)
tree	0caa4c5a119b165fd4fe9e3712f0de5caffa403e
parent	6f2f2a6baae128cd4f8111d91bdcfb7b76e39b97 (diff)
download	gitlab-ce-2703330a19e813351e9c33241a59d6b7f54741df.tar.gz