From 00b45539488ccb2e5ee59d054f4ea03f68ab808a Mon Sep 17 00:00:00 2001 From: Noah Petherbridge Date: Fri, 6 May 2016 10:33:56 -0700 Subject: [PATCH 1/3] Execute object macros within conditionals --- Changes.md | 5 +++++ src/brain.coffee | 16 +++++++++++----- test/test-objects.coffee | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/Changes.md b/Changes.md index 227d952..93c5b74 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,10 @@ # Changes +* 1.12.2 TBD + - Fix the `` tags not being executed on the left side of conditionals, + so that `test == true => Success` types of conditions should + work (bug #107). + * 1.12.1 2016-05-05 - Fix the `...` regular expression to match line break characters and preserve them in the argument list sent to an object macro (bug #108). diff --git a/src/brain.coffee b/src/brain.coffee index cebba4d..cb87a67 100644 --- a/src/brain.coffee +++ b/src/brain.coffee @@ -49,17 +49,17 @@ class Brain # If the BEGIN block exists, consult it first. if @master._topics.__begin__ - begin = @_getReply(user, "request", "begin", 0, scope) + begin = @_getReply(user, "request", "begin", 0, scope, async) # OK to continue? if begin.indexOf("{ok}") > -1 - reply = @_getReply(user, msg, "normal", 0, scope) + reply = @_getReply(user, msg, "normal", 0, scope, async) begin = begin.replace(/\{ok\}/g, reply) reply = begin reply = @processTags(user, msg, reply, [], [], 0, scope) else - reply = @_getReply(user, msg, "normal", 0, scope) + reply = @_getReply(user, msg, "normal", 0, scope, async) reply = @processCallTags(reply, scope, async) @@ -245,15 +245,17 @@ class Brain reply ## - # string _getReply (string user, string msg, string context, int step, scope) + # string _getReply (string user, string msg, string context, int step, scope, async) # # The internal reply method. DO NOT CALL THIS DIRECTLY. # # * user, msg and scope are the same as reply() # * context = "normal" or "begin" # * step = the recursion depth + # * scope = the call scope for object macros + # * async = boolean, whether object macros should run asynchronously ## - _getReply: (user, msg, context, step, scope) -> + _getReply: (user, msg, context, step, scope, async) -> # Needed to sort replies? if not @master._sorted.topics @warn "You forgot to call sortReplies()!" @@ -444,6 +446,10 @@ class Brain left = @processTags(user, msg, left, stars, thatstars, step, scope) right = @processTags(user, msg, right, stars, thatstars, step, scope) + # Execute any tags in the conditions. + left = @processCallTags(left, scope, async) + right = @processCallTags(right, scope, async) + # Defaults? if left.length is 0 left = "undefined" diff --git a/test/test-objects.coffee b/test/test-objects.coffee index 232595c..8148a16 100644 --- a/test/test-objects.coffee +++ b/test/test-objects.coffee @@ -84,6 +84,44 @@ exports.test_get_variable = (test) -> bot.reply("show me var", "test") test.done() +exports.test_objects_in_conditions = (test) -> + bot = new TestCase(test, """ + > object test_condition javascript + return args[0] === "1" ? "true" : "false"; + < object + + + test * + * test_condition == true => True. + * test_condition == false => False. + - Call failed. + """) + bot.reply("test 1", "True.") + bot.reply("test 2", "False.") + bot.reply("test 0", "False.") + bot.reply("test x", "False.") + test.done() + +exports.test_line_breaks_in_call = (test) -> + bot = new TestCase(test, """ + > object macro javascript + var a = args.join("; "); + return a; + < object + + // Variables with newlines aren't expected to interpolate, because + // tag processing only happens in one phase. + ! var name = name with\\nnew line + + + test literal newline + - macro "argumentwith\\nnewline" + + + test botvar newline + - macro "" + """) + bot.reply("test literal newline", "argumentwith\nnewline") + bot.reply("test botvar newline", "name with\\nnew line") + test.done() + exports.test_js_string_in_setSubroutine = (test) -> bot = new TestCase(test, """ + hello From 0ae0e3b7fe512c39630438b7254b549754b2365d Mon Sep 17 00:00:00 2001 From: Noah Petherbridge Date: Fri, 6 May 2016 10:57:45 -0700 Subject: [PATCH 2/3] Fix weight tag regexp to also gobble up surrounding spaces --- Changes.md | 4 ++++ src/brain.coffee | 2 +- test/test-triggers.coffee | 33 ++++++++++++++++++++++++++++++++- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/Changes.md b/Changes.md index 93c5b74..7eb5e49 100644 --- a/Changes.md +++ b/Changes.md @@ -4,6 +4,10 @@ - Fix the `` tags not being executed on the left side of conditionals, so that `test == true => Success` types of conditions should work (bug #107). + - Fix trigger regexp processing so that if a `{weight}` tag contains a space + before or after it (or: a space between `{weight}` and the rest of the + trigger text), the spaces are also stripped so that matching isn't broken + for that trigger (bug #102). * 1.12.1 2016-05-05 - Fix the `...` regular expression to match line break characters diff --git a/src/brain.coffee b/src/brain.coffee index cb87a67..a0263f0 100644 --- a/src/brain.coffee +++ b/src/brain.coffee @@ -600,7 +600,7 @@ class Brain regexp = regexp.replace(/\*/g, "(.+?)") # Convert * into (.+?) regexp = regexp.replace(/#/g, "(\\d+?)") # Convert # into (\d+?) regexp = regexp.replace(/_/g, "(\\w+?)") # Convert _ into (\w+?) - regexp = regexp.replace(/\{weight=\d+\}/g, "") # Remove {weight} tags + regexp = regexp.replace(/\s*\{weight=\d+\}\s*/g, "") # Remove {weight} tags regexp = regexp.replace(//g, "(.*?)") regexp = regexp.replace(/\|{2,}/, '|') # Remove empty entities regexp = regexp.replace(/(\(|\[)\|/g, '$1') # Remove empty entities from start of alt/opts diff --git a/test/test-triggers.coffee b/test/test-triggers.coffee index 709f7a8..987b25a 100644 --- a/test/test-triggers.coffee +++ b/test/test-triggers.coffee @@ -117,11 +117,42 @@ exports.test_weighted_triggers = (test) -> + hello *{weight=20} - Hi there! + + // Test that spaces before or after the {weight} tag are gobbled up along + // with the {weight} tag itself. + + + something{weight=100} + - Weighted something + + + something + - Unweighted something + + + nothing {weight=100} + - Weighted nothing + + + nothing + - Unweighted nothing + + + {weight=100}everything + - Weighted everything + + + everything + - Unweighted everything + + + {weight=100} blank + - Weighted blank + + + blank + - Unweighted blank """) bot.reply("Hello robot.", "Hi there!") bot.reply("Hello or something.", "Hi there!") bot.reply("Can you run a Google search for Node", "Sure!") bot.reply("Can you run a Google search for Node or something", "Or something. Sure!") + bot.reply("something", "Weighted something") + bot.reply("nothing", "Weighted nothing") + bot.reply("everything", "Weighted everything") + bot.reply("blank", "Weighted blank") test.done() exports.test_empty_piped_arrays = (test) -> @@ -232,4 +263,4 @@ exports.test_empty_piped_optionals = (test) -> bot.reply("Cat should not feel warm to me", "Purrfect.") bot.reply("Bye!", "Anything else?") bot.reply("Love you", "Anything else?") - test.done() \ No newline at end of file + test.done() From 29905f77c929811a5ecb0d018bf5e9e822ded9e0 Mon Sep 17 00:00:00 2001 From: Noah Petherbridge Date: Tue, 14 Jun 2016 11:28:50 -0700 Subject: [PATCH 3/3] Make it clear that async objects do not work in conditions --- eg/README.md | 6 +-- eg/reply-async/README.md | 25 ++++++++- eg/{async-object => second-reply}/README.md | 29 +++++----- eg/{async-object => second-reply}/bot.js | 8 +-- .../second-reply.rive} | 10 ++-- src/brain.coffee | 20 +++---- src/rivescript.coffee | 2 +- test/test-objects.coffee | 54 ++++++++++++++++--- 8 files changed, 109 insertions(+), 45 deletions(-) rename eg/{async-object => second-reply}/README.md (64%) rename eg/{async-object => second-reply}/bot.js (93%) rename eg/{async-object/async.rive => second-reply/second-reply.rive} (51%) diff --git a/eg/README.md b/eg/README.md index 8f42526..980a124 100644 --- a/eg/README.md +++ b/eg/README.md @@ -28,9 +28,9 @@ RiveScript-js. with its users. * [reply-async](reply-async/) - Demonstrates using the `replyAsync()` method and a JavaScript object macro that returns a promise. -* [async-object](async-object/) - Demonstrates a JavaScript object macro in - RiveScript that asynchronously sends the user a second message at some point - in the future, asynchronously from the immediately requested message. +* [second-reply](second-reply/) - Demonstrates a JavaScript object macro in + RiveScript that sends a second reply to the user at some point in the future, + separately from the initially requested reply. * [scope](scope/) - Demonstrates the usage of the `scope` parameter to the `reply()` function for passing the parent scope down into JavaScript object macros. diff --git a/eg/reply-async/README.md b/eg/reply-async/README.md index 70c30f4..8dda404 100644 --- a/eg/reply-async/README.md +++ b/eg/reply-async/README.md @@ -1,9 +1,32 @@ -# Reply-async +# replyAsync Example This example demonstrates using replyAsync function. You should use **replyAsync** instead of **reply** if you have subroutines that return promises. +## Important Note About Conditionals + +Asynchronous object macros are *not* supported within the conditional side of +`*Conditions` in RiveScript. That is, RiveScript code like the following will +not work (where the `weather` macro calls out to an HTTP API and returns a +promise with the result): + +```rivescript ++ is it sunny outside +* weather == sunny => It appears it is! +- It doesn't look sunny outside. +``` + +Conditionals in RiveScript require their results to be immediately available +so it can check if the comparison is truthy. So, even if you use `replyAsync()`, +the `` tags in conditionals only support running synchronous object +macros (ones that return a string result and not a promise). + +However, asynchronous object macros can be used in the *reply* portion of the +conditional (the part on the right side of the `=>` separator). This is the +text eventually returned to the user and it can return as a promise when you use +`replyAsync()` just like if it were in a `-Reply` command. + ## Running the example ```bash diff --git a/eg/async-object/README.md b/eg/second-reply/README.md similarity index 64% rename from eg/async-object/README.md rename to eg/second-reply/README.md index a7bfcc6..23dc10b 100644 --- a/eg/async-object/README.md +++ b/eg/second-reply/README.md @@ -1,23 +1,24 @@ -# Asynchronous Objects +# Asynchronous Second Reply This example demonstrates how a JavaScript object macro in a RiveScript bot can -asynchronously send a user a message after a timeout. +send a second reply to the user asynchronously from the original reply, after a +timeout. -In this example we have our chatbot in a prototypical object named `AsyncBot`, +In this example we have our chatbot in a prototypical object named `MyBot`, and the bot implements functions such as `sendMessage(user, message)` to deliver messages to users. If you imagine this bot were to connect to an IRC server, the implementation of `sendMessage()` might deliver a private message to a particular user by nickname over IRC. In this example, it just writes a message to the console. -View the source code of `bot.js` and `async.rive` for details. The JavaScript -source is well-documented. The key pieces are: +View the source code of `bot.js` and `second-reply.rive` for details. The +JavaScript source is well-documented. The key pieces are: -* In the call to `reply()`, we pass a reference to `this` (the `AsyncBot` +* In the call to `reply()`, we pass a reference to `this` (the `MyBot` object) in as the `scope` parameter. This scope is passed all the way down to JavaScript object macros, so that `this` inside the object macro refers to the - very same `AsyncBot` instance in the application code. -* The `asyncTest` object macro in `async.rive` is able to call the + very same `MyBot` instance in the application code. +* The `replyTest` object macro in `second-reply.rive` is able to call the `sendMessage()` function after a two-second delay by using `setTimeout()`. Any asynchronous JavaScript call could've been used in its place. For example, imagine the bot needed to call a web API to get local weather information and @@ -27,13 +28,11 @@ source is well-documented. The key pieces are: ``` % node bot.js -> async test -[Soandso] async test +> reply test +[Soandso] reply test [Bot] @Soandso: Wait for it... -> [Bot] @Soandso: Async reply! -lol -[Soandso] lol -[Bot] @Soandso: No reply for that. Type "async test" to test the asynchronous macro. +> [Bot] @Soandso: Second reply! ``` -The "Async reply!" line was delivered 2 seconds after the "Wait for it..." line. +The "Second reply!" line was delivered 2 seconds after the +"Wait for it..." line. diff --git a/eg/async-object/bot.js b/eg/second-reply/bot.js similarity index 93% rename from eg/async-object/bot.js rename to eg/second-reply/bot.js index 7eefa20..c50e4b5 100644 --- a/eg/async-object/bot.js +++ b/eg/second-reply/bot.js @@ -1,4 +1,4 @@ -// Asynchronous Objects Example +// Asynchronous Second Reply Example // See the accompanying README.md for details. // Run this demo: `node bot.js` @@ -10,12 +10,12 @@ var readline = require("readline"); var RiveScript = require("../../lib/rivescript"); // Create a prototypical class for our own chatbot. -var AsyncBot = function(onReady) { +var MyBot = function(onReady) { var self = this; self.rs = new RiveScript(); // Load the replies and process them. - self.rs.loadFile("async.rive", function() { + self.rs.loadFile("second-reply.rive", function() { self.rs.sortReplies(); onReady(); }); @@ -40,7 +40,7 @@ var AsyncBot = function(onReady) { }; // Create and run the example bot. -var bot = new AsyncBot(function() { +var bot = new MyBot(function() { // Drop into an interactive shell to get replies from the user. // If this were something like an IRC bot, it would have a message // handler from the server for when a user sends a private message diff --git a/eg/async-object/async.rive b/eg/second-reply/second-reply.rive similarity index 51% rename from eg/async-object/async.rive rename to eg/second-reply/second-reply.rive index 5f7c9c2..d94262d 100644 --- a/eg/async-object/async.rive +++ b/eg/second-reply/second-reply.rive @@ -1,18 +1,18 @@ // Example of an asynchronous object macro. + * -- No reply for that. Type "async test" to test the asynchronous macro. +- No reply for that. Type "reply test" to test the asynchronous macro. -+ async test -- Wait for it... asyncTest ++ reply test +- Wait for it... replyTest -> object asyncTest javascript +> object replyTest javascript var self = this; var nick = rs.currentUser(); // Send a second reply after 2 seconds. setTimeout(function() { - self.sendMessage(nick, "Async reply!"); + self.sendMessage(nick, "Second reply!"); }, 2000); return; diff --git a/src/brain.coffee b/src/brain.coffee index 1048eb3..23b6cfc 100644 --- a/src/brain.coffee +++ b/src/brain.coffee @@ -53,17 +53,17 @@ class Brain # If the BEGIN block exists, consult it first. if @master._topics.__begin__ - begin = @_getReply(user, "request", "begin", 0, scope, async) + begin = @_getReply(user, "request", "begin", 0, scope) # OK to continue? if begin.indexOf("{ok}") > -1 - reply = @_getReply(user, msg, "normal", 0, scope, async) + reply = @_getReply(user, msg, "normal", 0, scope) begin = begin.replace(/\{ok\}/g, reply) reply = begin reply = @processTags(user, msg, reply, [], [], 0, scope) else - reply = @_getReply(user, msg, "normal", 0, scope, async) + reply = @_getReply(user, msg, "normal", 0, scope) reply = @processCallTags(reply, scope, async) @@ -249,7 +249,7 @@ class Brain reply ## - # string _getReply (string user, string msg, string context, int step, scope, async) + # string _getReply (string user, string msg, string context, int step, scope) # # The internal reply method. DO NOT CALL THIS DIRECTLY. # @@ -257,9 +257,8 @@ class Brain # * context = "normal" or "begin" # * step = the recursion depth # * scope = the call scope for object macros - # * async = boolean, whether object macros should run asynchronously ## - _getReply: (user, msg, context, step, scope, async) -> + _getReply: (user, msg, context, step, scope) -> # Needed to sort replies? if not @master._sorted.topics @warn "You forgot to call sortReplies()!" @@ -454,9 +453,12 @@ class Brain left = @processTags(user, msg, left, stars, thatstars, step, scope) right = @processTags(user, msg, right, stars, thatstars, step, scope) - # Execute any tags in the conditions. - left = @processCallTags(left, scope, async) - right = @processCallTags(right, scope, async) + # Execute any tags in the conditions. We explicitly send + # `false` as the async parameter, because we can't run async + # object macros in conditionals; we need the result NOW + # for comparison. + left = @processCallTags(left, scope, false) + right = @processCallTags(right, scope, false) # Defaults? if left.length is 0 diff --git a/src/rivescript.coffee b/src/rivescript.coffee index 5c78646..fedf98e 100644 --- a/src/rivescript.coffee +++ b/src/rivescript.coffee @@ -7,7 +7,7 @@ "use strict" # Constants -VERSION = "1.12.2" +VERSION = "1.13.0" # Helper modules Parser = require "./parser" diff --git a/test/test-objects.coffee b/test/test-objects.coffee index 8148a16..51cf69a 100644 --- a/test/test-objects.coffee +++ b/test/test-objects.coffee @@ -86,20 +86,60 @@ exports.test_get_variable = (test) -> exports.test_objects_in_conditions = (test) -> bot = new TestCase(test, """ + // Normal synchronous object that returns an immediate response. > object test_condition javascript return args[0] === "1" ? "true" : "false"; < object - + test * + // Asynchronous object that returns a promise. This isn't supported + // in a conditional due to the immediate/urgent nature of the result. + > object test_async_condition javascript + return new rs.Promise(function(resolve, reject) { + setTimeout(function() { + resolve(args[0] === "1" ? "true" : "false"); + }, 10); + }); + < object + + + test sync * * test_condition == true => True. * test_condition == false => False. - Call failed. + + + test async * + * test_async_condition == true => True. + * test_async_condition == false => False. + - Call failed. + + + call sync * + - Result: test_condition + + + call async * + - Result: test_async_condition """) - bot.reply("test 1", "True.") - bot.reply("test 2", "False.") - bot.reply("test 0", "False.") - bot.reply("test x", "False.") - test.done() + # First, make sure the sync object works. + bot.reply("call sync 1", "Result: true") + bot.reply("call sync 0", "Result: false") + bot.reply("call async 1", "Result: [ERR: Using async routine with reply: use replyAsync instead]") + + # Test the synchronous object in a conditional. + bot.reply("test sync 1", "True.") + bot.reply("test sync 2", "False.") + bot.reply("test sync 0", "False.") + bot.reply("test sync x", "False.") + + # Test the async object on its own and then in a conditional. This code looks + # ugly, but `test.done()` must be called only when all tests have resolved + # so we have to nest a couple of the promise-based tests this way. + bot.rs.replyAsync(bot.username, "call async 1").then((reply) -> + test.equal(reply, "Result: true") + + # Now test that it still won't work in a conditional even with replyAsync. + bot.rs.replyAsync(bot.username, "test async 1").then((reply) -> + test.equal(reply, "Call failed.") + test.done() + ) + ) exports.test_line_breaks_in_call = (test) -> bot = new TestCase(test, """ @@ -232,7 +272,7 @@ exports.test_promises_in_objects = (test) -> return new rs.Promise((resolve, reject) -> setTimeout () -> resolve("delay") - , 1000 + , 10 ) )