rivescript-js icon indicating copy to clipboard operation
rivescript-js copied to clipboard

Cannot use one macro's return value as an argument to another macro

Open kjleitz opened this issue 5 years ago • 6 comments

Steps to reproduce:

(in the JS file)

import RiveScript from 'rivescript';

function sayFoo(): string {
  return 'Foo';
}

function startsWithFoo(someString): boolean {
  return !!someString.match(/^foo/i);
}

function alertArgs(...args): void {
  alert(args);
}

const engine = new RiveScript();

engine.loadFile('public/path/to/some/brainfile.rive').then(() => {
  engine.sortReplies();

  engine.setSubroutine('say-foo', (_rs, _args) => {
    return sayFoo(); // should return 'Foo' as a string
  });

  engine.setSubroutine('starts-with-foo', (_rs, args) => {
    return '' + startsWithFoo(args[0]); // should return 'true' as a string
  });

  engine.setSubroutine('alert-args', (_rs, args) => {
    return '' + alertArgs(args); // should return 'undefined' as a string, but also trigger an "alert" dialog
  });
});

(in the RS file)

// public/path/to/some/brainfile.rive

! version = 2.0

> begin
  + request
  - {ok}
< begin

+ using macro return val as argument to macro should respond with true
- <call>starts-with-foo <call>say-foo</call></call>

+ setting macro return val in variable and using var as arg to macro should respond with true
- <set foovariable=<call>say-foo</call>>
^ <call>starts-with-foo <get foovariable></call>

+ using macro return val as star in a redirect and then using star as arg to macro should respond with true
- {@ redirecttrigger <call>say-foo</call>}

+ redirecttrigger *
- <call>starts-with-foo <star></call>

+ trigger to show what is happening
- <call>alert-args <call>say-foo</call></call>

(description of the interaction)

user > using macro return val as argument to macro should respond with true
bot  > false</call>

user > setting macro return val in variable and using var as arg to macro should respond with true
bot  > false</call>

user > using macro return val as star in a redirect and then using star as arg to macro should respond with true
bot  > false</call>

user > trigger to show what is happening
(alert dialog pops up and says "<call>say-foo")
bot  > undefined</call>

Seems to be a parsing the child's </call> closing tag as the closing tag to the parent!

kjleitz avatar Oct 04 '18 00:10 kjleitz

You're right on the diagnosis that the regexp /<call>(.+?)<\/call>/ catches the wrong closing tag.

So this isn't a supported feature. :( If you know a clever way to make it possible send a pull request! A good algorithm to parse tags in a sane inside-out order would be useful because most of RiveScript's tags are regexp based and order dependant.

kirsle avatar Oct 04 '18 00:10 kirsle

I don't have the bandwidth to work on it at the moment, but I'd love to make an attempt when I have some time 😃

In the meantime, though, I wonder if this would be a cheap alternative that would satisfy the feature's "story", but take the burden off of the parser and instead place it on the user:

What about multiple levels of <call> tags, similar to the multiple levels of <star>, <reply>, and <input>? So, if you know you have a stack of macros, you could do...

<call>foo <call2>bar <call3>baz</call3></call2></call>

kjleitz avatar Oct 04 '18 13:10 kjleitz

For context, are nested <call> tags supported in the other RS interpreters? I don't see a spec for it in the RiveScript "working draft", so if there's an existing solution I'll check out the other implementations before working on a PR.

Side note: thank you so much for all your work on this language... it's seriously awesome. RiveScript and its peripherals are wicked stable and feature-rich, and just seeing how active you are as a maintainer is super cool. Anyway, thanks, hahah.

kjleitz avatar Oct 05 '18 15:10 kjleitz

Nested <call> tags aren't supported in the other implementations, either. They all more or less share the same logic throughout with the same regexps in most places.

There is one area where RiveScript does support nested tags, due to some regular expression cleverness. This support was added just recently (last couple years) -- all of the <bot>, <get>, <set>, <add>, <sub>, <div> and <mult> tags that deal with user variables can be nested in all sorts of ways. What these tags had in common though is they are all self-contained, without an opening and a closing part.

So when RiveScript is dealing with these, it has a regexp like /<([^<]+)>/ which matches a <tag> that contains no other tag inside of its own brackets. So a <set name=<bot name>> would match the <bot name> tag first, because it contains no nested tag, process it, and then come back for the <set name=whatever> on a second pass, until it runs out of self-enclosed tags to handle.

Unfortunately that approach isn't good for tags that open and close, and can't be mixed-matched with RiveScript's {curly bracket} tags.

My thoughts on the "best" way to completely overhaul the tag processing system (let tags be mixed and matched in all sorts of new combinations) would essentially boil down to: parsing the characters one at a time, keeping track of opening and closing tag characters and nested state. Essentially write a custom lexer/tokenizer and parser as a proper scripting language would do -- but that doesn't sound fun to write, and it would be painful to port equivalent logic across five programming languages.

So I've just been dealing with these limitations until I get a sudden spark of brilliance to find a clever and simpler algorithm for this -- ideally one that doesn't bring in external dependencies because that's when portability across the programming languages can start to get dicey.

If you wanna take a stab at it, feel free! :smile:

kirsle avatar Oct 09 '18 17:10 kirsle

<call></call> is the only "open/close" set of angle bracket tags in RiveScript, right?

Hmmm.

What about this?

# switch this...
/<([^<]+)>/

# ...to this:
/<(?!call)((?!\/call)[^<]+?)>/

# which, given the following...
<call>foo <call>bar <get <star>></call></call>

# ...will match:
<star> # full match
star   # first group

# and given the following...
<call>foo <call>bar <get foo></call></call>

# ...will match:
<get foo> # full match
get foo   # first group

# (run that through first)

# then, go for the `call` tags
/<call>(?!.*<call>)(.*?)<\/call>/

# which, given the following...
<call>foo <call>bar <call>baz</call> <call>xyz</call></call></call>

# ...will match:
<call>xyz</call> # full match
xyz              # first group

# and given the following...
<call>foo <call>bar <call>baz</call> 123</call></call>

# ...will match:
<call>baz</call> # full match
baz              # first group


# Could also do this for open/close tags, more generally:
#   /<([^>]+?)>(?!.*<\1>).*?<\/\1>/
# But then I realized there's an example in the tutorial
# where you can put arbitrary HTML in a response,
# so it would try to pick that up. Might not be good.

So basically, go through the line with /<(?!call)((?!\/call)[^<]+?)>/ to match all non-call tags first (returning the most deeply nested one until they're all gone). Then go through the line with /<call>(?!.*<call>)(.*?)<\/call>/ to match all call tags (again returning the most deeply nested one until they're all gone).

What do you think?

(Alternative: what about using an XML parser, just for parsing the angle-bracketed tags? It would add a dependency, but at least it would be a pretty mature and widespread one, so portability issues might not be so prevalent cross-language)

kjleitz avatar Oct 09 '18 19:10 kjleitz

@kirsle This came up again in my project, so I thought I'd poke you—have any thoughts about the solution above?

kjleitz avatar Aug 02 '19 20:08 kjleitz