Merge pull request #111 from aichaos/bug/107-object-in-condition

Execute object macros within conditionals

Changes.md

@@ -1,5 +1,14 @@
 # Changes
 
+* 1.13.0 TBD
+  - Fix the `<call>` tags not being executed on the left side of conditionals,
+    so that `<call>test</call> == 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.2 2016-05-16
   - Call the error handler on `loadDirectory()` when the directory doesn't exist
     or isn't a directory (bug #117).

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.

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
+* <call>weather <get zipcode></call> == 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 `<call>` 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

eg/async-object/README.md → 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.

eg/async-object/bot.js → 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

eg/async-object/async.rive → 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... <call>asyncTest</call>
++ reply test
+- Wait for it... <call>replyTest</call>
 
-> 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;

src/brain.coffee

@@ -256,6 +256,7 @@ class Brain
   # * user, msg and scope are the same as reply()
   # * context = "normal" or "begin"
   # * step = the recursion depth
+  # * scope = the call scope for object macros
   ##
   _getReply: (user, msg, context, step, scope) ->
     # Needed to sort replies?
@@ -452,6 +453,13 @@ class Brain
               left  = @processTags(user, msg, left, stars, thatstars, step, scope)
               right = @processTags(user, msg, right, stars, thatstars, step, scope)
 
+              # Execute any <call> 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
                 left = "undefined"
@@ -602,7 +610,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(/<zerowidthstar>/g, "(.*?)")
     regexp = regexp.replace(/\|{2,}/, '|') # Remove empty entities
     regexp = regexp.replace(/(\(|\[)\|/g, '$1') # Remove empty entities from start of alt/opts

src/rivescript.coffee

@@ -7,7 +7,7 @@
 "use strict"
 
 # Constants
-VERSION  = "1.12.2"
+VERSION  = "1.13.0"
 
 # Helper modules
 Parser  = require "./parser"

test/test-objects.coffee

@@ -84,6 +84,84 @@ exports.test_get_variable = (test) ->
   bot.reply("show me var", "test")
   test.done()
 
+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
+
+    // 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 *
+    * <call>test_condition <star></call> == true  => True.
+    * <call>test_condition <star></call> == false => False.
+    - Call failed.
+
+    + test async *
+    * <call>test_async_condition <star></call> == true  => True.
+    * <call>test_async_condition <star></call> == false => False.
+    - Call failed.
+
+    + call sync *
+    - Result: <call>test_condition <star></call>
+
+    + call async *
+    - Result: <call>test_async_condition <star></call>
+  """)
+  # 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, """
+    > 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
+    - <call>macro "argumentwith\\nnewline"</call>
+
+    + test botvar newline
+    - <call>macro "<bot name>"</call>
+  """)
+  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
@@ -194,7 +272,7 @@ exports.test_promises_in_objects = (test) ->
     return new rs.Promise((resolve, reject) ->
       setTimeout () ->
         resolve("delay")
-      , 1000
+      , 10
     )
   )
 

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()
+  test.done()