From afb38800d7fa72aee3fabd2fbc0cfed7ec083caa Mon Sep 17 00:00:00 2001 From: John Engelman Date: Mon, 9 Sep 2019 08:19:26 -0500 Subject: [PATCH 1/2] Resolves #107: Add supporting for capturing remaining arguments using `arg+` syntax --- core/matcher.go | 28 ++++++++++++++++++++++++++++ core/matcher_test.go | 40 ++++++++++++++++++++++++++++++---------- 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/core/matcher.go b/core/matcher.go index 7b786ef2..72958a5b 100644 --- a/core/matcher.go +++ b/core/matcher.go @@ -174,11 +174,34 @@ func isValidHitChatRule(message *models.Message, rule models.Rule, processedInpu args := utils.RuleArgTokenizer(processedInput) var optionalArgs int var requiredArgs int + var varArgs int // take note of all optional args that end with a '?' for _, arg := range rule.Args { if strings.HasSuffix(arg, "?") { optionalArgs++ } + if strings.HasSuffix(arg, "+") { + varArgs++ + } + } + if varArgs > 1 { + // error, can ony have 1 + msg := fmt.Sprintf("You cannot specify more than 1 variable argument") + message.Output = msg + return false + } + if strings.HasSuffix(rule.Args[len(rule.Args)-1], "+") { + if optionalArgs > 0 { + // error, cannot combine optional and varargs + msg := fmt.Sprintf("You cannot combine optional arguments with variable arguments") + message.Output = msg + return false + } + } else if varArgs == 1 { + // error, vararg but not in last position + msg := fmt.Sprintf("You must specify the variable argument in the last argument position") + message.Output = msg + return false } // ensure we only require args that don't end with '?' requiredArgs = len(rule.Args) - optionalArgs @@ -190,6 +213,11 @@ func isValidHitChatRule(message *models.Message, rule models.Rule, processedInpu } // Go through the supplied args and make them available as variables for index, arg := range rule.Args { + // If this is a varag method, then join all the remaining args and end + if strings.HasSuffix(arg, "+") { + message.Vars[strings.TrimSuffix(arg, "+")] = strings.Join(args[index:], " ") + break + } // strip '?' from end of arg arg = strings.TrimSuffix(arg, "?") // index starts at 0 so we need to account for that diff --git a/core/matcher_test.go b/core/matcher_test.go index 6c9a7e38..2996b4d6 100644 --- a/core/matcher_test.go +++ b/core/matcher_test.go @@ -680,6 +680,13 @@ func Test_handleChatServiceRule(t *testing.T) { HelpText: "foo ", } + ruleVarg := models.Rule{ + Name: "Test Rules with varargs", + Respond: "foo", + Args: []string{"arg1", "argv+"}, + HelpText: "foo ", + } + ruleHearWithArgs := models.Rule{ Name: "Hear rule with Args set", Hear: "/hi/", @@ -720,6 +727,12 @@ func Test_handleChatServiceRule(t *testing.T) { BotMentioned: true, } + testMessageVargs := models.Message{ + Input: "foo arg1 arg2 arg3 arg4", + Vars: map[string]string{}, + BotMentioned: true, + } + testMessageIgnoreThread := models.Message{ Input: "we have a thread", Vars: map[string]string{}, @@ -733,17 +746,19 @@ func Test_handleChatServiceRule(t *testing.T) { want bool want1 bool expectMsg string + expectedVars map[string]string }{ - {"basic", args{}, false, false, ""}, - {"respond + hear", args{rule: models.Rule{Respond: "hi", Hear: "/hi/"}, hit: false, bot: testBot, message: testMessage}, false, false, ""}, - {"hear + rule args", args{rule: ruleHearWithArgs, hit: false, bot: testBot, message: testMessage}, false, false, ""}, - {"respond rule - hit false", args{rule: rule, hit: false}, false, false, ""}, - {"respond rule - hit true - valid", args{rule: rule, hit: true, bot: testBot, message: testMessage, processedInput: "arg1 arg2"}, true, true, "hmm, the 'format_output' field in your configuration is empty"}, - {"respond rule - hit true - bot not mentioned", args{rule: rule, hit: true, bot: testBot, message: testMessageBotNotMentioned, processedInput: "arg1 arg2"}, false, false, ""}, - {"respond rule - hit true - valid - not enough args", args{rule: rule, hit: true, bot: testBot, message: testMessageNotEnoughArgs, processedInput: "arg1"}, true, true, "You might be missing an argument or two. This is what I'm looking for\n```foo ```"}, - {"respond rule - hit true - valid optional arg", args{rule: ruleOpt, hit: true, bot: testBot, message: testMessageOptionalArgs, processedInput: "arg1"}, true, true, ""}, - {"respond rule - hit true - invalid", args{rule: rule, hit: true, bot: testBot, message: testMessage}, true, true, "You might be missing an argument or two. This is what I'm looking for\n```foo ```"}, - {"hear rule - ignore thread", args{rule: ruleIgnoreThread, hit: true, bot: testBot, message: testMessageIgnoreThread}, true, true, ""}, + {"basic", args{}, false, false, "", map[string]string{}}, + {"respond + hear", args{rule: models.Rule{Respond: "hi", Hear: "/hi/"}, hit: false, bot: testBot, message: testMessage}, false, false, "", map[string]string{}}, + {"hear + rule args", args{rule: ruleHearWithArgs, hit: false, bot: testBot, message: testMessage}, false, false, "", map[string]string{}}, + {"respond rule - hit false", args{rule: rule, hit: false}, false, false, "", map[string]string{}}, + {"respond rule - hit true - valid", args{rule: rule, hit: true, bot: testBot, message: testMessage, processedInput: "arg1 arg2"}, true, true, "hmm, the 'format_output' field in your configuration is empty", map[string]string{"arg1": "arg1", "arg2": "arg2"}}, + {"respond rule - hit true - bot not mentioned", args{rule: rule, hit: true, bot: testBot, message: testMessageBotNotMentioned, processedInput: "arg1 arg2"}, false, false, "", map[string]string{}}, + {"respond rule - hit true - valid - not enough args", args{rule: rule, hit: true, bot: testBot, message: testMessageNotEnoughArgs, processedInput: "arg1"}, true, true, "You might be missing an argument or two. This is what I'm looking for\n```foo ```", map[string]string{}}, + {"respond rule - hit true - valid optional arg", args{rule: ruleOpt, hit: true, bot: testBot, message: testMessageOptionalArgs, processedInput: "arg1"}, true, true, "", map[string]string{"arg1": "arg1"}}, + {"respond rule - hit true - valid vargs", args{rule: ruleVarg, hit: true, bot: testBot, message: testMessageVargs, processedInput: "arg1 arg2 arg3 arg4"}, true, true, "", map[string]string{"arg1": "arg1", "argv": "arg2 arg3 arg4"}}, + {"respond rule - hit true - invalid", args{rule: rule, hit: true, bot: testBot, message: testMessage}, true, true, "You might be missing an argument or two. This is what I'm looking for\n```foo ```", map[string]string{}}, + {"hear rule - ignore thread", args{rule: ruleIgnoreThread, hit: true, bot: testBot, message: testMessageIgnoreThread}, true, true, "", map[string]string{}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -772,6 +787,11 @@ func Test_handleChatServiceRule(t *testing.T) { if got1 != tt.want1 { t.Errorf("handleChatServiceRule() got1 = %v, want %v", got1, tt.want1) } + for argk, argv := range tt.expectedVars { + if tt.args.message.Vars[argk] != argv { + t.Errorf("handleChatServiceRules() did not extract argument %v. got = %v, want %v", argk, tt.args.message.Vars[argk], argv) + } + } } }) } From f881f8f52063645afe84936d7cccd539115e3606 Mon Sep 17 00:00:00 2001 From: wass3r Date: Wed, 23 Oct 2019 23:46:59 -0500 Subject: [PATCH 2/2] Fix no arguments case --- core/matcher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/matcher.go b/core/matcher.go index 72958a5b..f14fc845 100644 --- a/core/matcher.go +++ b/core/matcher.go @@ -190,7 +190,7 @@ func isValidHitChatRule(message *models.Message, rule models.Rule, processedInpu message.Output = msg return false } - if strings.HasSuffix(rule.Args[len(rule.Args)-1], "+") { + if len(rule.Args) > 0 && strings.HasSuffix(rule.Args[len(rule.Args)-1], "+") { if optionalArgs > 0 { // error, cannot combine optional and varargs msg := fmt.Sprintf("You cannot combine optional arguments with variable arguments")