aichaos / rivescript-wd

The RiveScript Working Draft describes the language specification for RiveScript.
https://www.rivescript.com/
7 stars 3 forks source link

Add file-scoped parsing options #2

Closed kirsle closed 8 years ago

kirsle commented 9 years ago

Add the ability to give options to the parser during the parse phase. The options should be scoped to just the current file (or streamed data) that the option appears in. The options would be read and processed as the file is being parsed, and would apply to all parts of the file that follow the line. As such, options can be defined and re-defined in multiple places. The option ends when the file ends, so that you don't accidentally mess up other unrelated files by leaving options left on.

Syntax example:

// change the concatenation mode for ^continue commands
! local concat = space

// with concat=space, the reply is "Hello human"; by default the ^continue
// doesn't add anything so it would be "Hellohuman" with no space in between
+ hello bot
- Hello
^ human.

// change the concat mode again in the same file
! local concat = newline

// now this one will join the lines with a line break (\n)
// Human> How are you?
// Bot> I am good.
// You?
+ how are you
- I am good.
^ You?

Ideas for supported parser options:

mariohmol commented 8 years ago

I tested here and it is by topic as well.. so i have to use that in every topic.. instead of just one per file

kirsle commented 8 years ago

It shouldn't be by topic. The JS version? Here and here is where the !local concat option is applied, it should work for the whole file.

mariohmol commented 8 years ago

If I use ! local concat = newline at first line it didnt work.. but if i include inside the topic it works.

Here are my versions

node@4.2.2
rivescript@1.13.0
  ├── fs-readdir-recursive@1.0.0
  └── rsvp@3.2.1
kirsle commented 8 years ago

Do you have example code that demonstrates the bug? I took a stab at it and can't reproduce it:

! version = 2.0
! local concat = newline

+ hello
- Hello,
^ user.

+ topic test
- Entering
^ the topic.{topic=test}

> topic test
    + *
    - Exiting
    ^ the topic.{topic=random}
< topic

Output:

RiveScript Interpreter (JavaScript) -- Interactive Mode
-------------------------------------------------------
rivescript version: 1.13.0
        Reply root: tmp

You are now chatting with the RiveScript bot. Type a message and press Return
to send it. When finished, type '/quit' to exit the program.
Type '/help' for other options.

You> hello
Bot> Hello,
user.
You> topic test
Bot> Entering
the topic.
You> test
Bot> Exiting
the topic.

If I move the ! local line to the top of the inside of > topic test, it still does what I'd expect:

You> hello
Bot> Hello,user.
You> topic test
Bot> Enteringthe topic.
You> test
Bot> Exiting
the topic.
You> hello
Bot> Hello,user.
mariohmol commented 8 years ago

I have a script that i run local.. but when i try to use in this links, i receive a lot of errors..

https://www.rivescript.com/try

is it related to rivescript version in this link? or is the latest version?

the erros is because i have a line like:

> topic inicio //includes interview_common

mariohmol commented 8 years ago

i found.. the problem is because i had comment after the line:

! local concat = newline //! local concat = space

if i remove this comment.. it works

! local concat = newline

kirsle commented 8 years ago

The problem looks like the inline comment wasn't being ignored properly. The code does this:

      # Ignore in-line comments if there's a space before and after the "//"
      if line.indexOf(" // ") > -1
        line = utils.strip(line.split(" // ")[0])

Make sure there's a space on either side of the //.

mariohmol commented 8 years ago

what u think to change that for next releases? just to help new developers...

i will change my code to give a space.. thanks =)

kirsle commented 8 years ago

I could change it to only require a space before the "//" but not after. I mostly want to make it non-ambiguous where somebody might have wanted a literal "//" (in a reply for example) without it being confused, e.g. in the case of "http://something"

kirsle commented 8 years ago

I implemented that change to inline comments and released RiveScript.js v1.14.0

mariohmol commented 8 years ago

Now, it only requires a space before the //

sorry.. i misundertoood the comment.. perfect mate.. thanks!

kirsle commented 8 years ago

Yeah. It allows this to work:

! local concat = newline //!local concat = space

But not this:

! local concat = newline// ! local concat = space

The former syntax is more often seen in programming languages, e.g. if you have code at the left margin of your text editor and you're commenting it out by just putting a # or // before it while still having it touch the text being commented out.

On a side note, the Try Online page includes RiveScript from npmcdn so it's always running the latest version from npm.

mariohmol commented 8 years ago

uhm now it starting make sense.. because running local with node.. i have some erros but the error message always returnin undefined

var loading_error = function(batch_num, error) {
    console.log("Error when loading files: " + error);
};
bot.loadFile([
    "aiml/inicio.rive"
],loading_done, loading_error);

So maybe, i receive this errors ( about havin // inline) .. but the error is empty.. but for some reason.. in try page.. the errors are showing.. maybe a bug between serverside version and client side version?

mariohmol commented 8 years ago

uhmm the thing is.. loading_error the first argument is the error.. and not batch_num

if you check in readme.. is wrong there

kirsle commented 8 years ago

What errors do you get from the !local concat line? It looks from the code that it just silently ignores errors (it sets the local concat option to whatever the line says, but when it handles ^ commands it only inserts a concatenation character if the current setting is a valid one, otherwise it just ignores it silently. This should probably be fixed :wink: )

Where in the readme? Only loadFile() shows the function prototype for the onError handler and it says:

onError receives: string errorMessage[, int batchNumber]

And shell.{coffee,js} receive them in that order.

kirsle commented 8 years ago

When I tested putting a syntactically invalid trigger (+ hello/test), the Try Online page gives an alert box pop-up and the node shells print it to the terminal, e.g.

Loading error: Syntax error: Triggers may only contain lowercase letters, numbers, and these symbols: ( | ) [ ] * _ # { } < > = at stream() line 4 near + hello/test

mariohmol commented 8 years ago

Sent u a PR for readme fix https://github.com/aichaos/rivescript-js/pull/139

You are right.. the error was not shown..

the errors that i'm getting.. is when i use a special char in trigger

+ não

error: Triggers may only contain lowercase letters, numbers, and these symbols: ( | ) [ ] * _ # { } < > = at ai ml/inicio.rive line 72 near + não

kirsle commented 8 years ago

Enable UTF-8 mode to use special characters in triggers.

var rs = new RiveScript({ utf8: true });

It's not enabled by default because I still consider it "experimental" for RiveScript (it wasn't designed with it in mind from the beginning, and Unicode is a whole Pandora's box that brings a lot of its own problems to the table).

mariohmol commented 8 years ago

you are right to no be default.. tested here worked like a charm!! Thanks a lot!

kirsle commented 8 years ago

If curious, there was a bit of discussion about Unicode problems in rivescript-js here and a bug ticket for the Python version where UTF-8 mode might be able to break it (not with Unicode symbols, but because the rules are looser) here.