Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to set a default (brain-wide) concat flag #209

Merged
merged 3 commits into from
Mar 5, 2017

Conversation

julien-c
Copy link
Contributor

wdyt @kirsle? cc @n1t0

@kirsle
Copy link
Member

kirsle commented Feb 23, 2017

I didn't make the concat mode a global option because I want to keep as much control as possible in the hands of the RiveScript source files, so that users could share RiveScript brains by just passing the files around and have them work as the author intended.

So if the author was relying on the default behavior of concatenation (not inserting spaces or newlines), and you load their brain with a global concat override option to make every concatenation use newlines, then you'd change the behavior of the bot as intended by its writer.

That's why I added the ! local option to set the concat mode on a per-file basis; this way an author can specify how they want concatenation to work on a file-by-file basis without globally applying that setting to other files that they may not have personally written.

@julien-c
Copy link
Contributor Author

I get your point, but thought I'd precise one point.

As it is implemented here, this is not a global override option: the local option still takes precedence over the global one. So everyone who writes or uses Rivescript files and use the local option in their files will never see a difference. Also, we do already have a few global options (so the parsing is already not totally self-contained in the .rive files).

The way I see it, it is a fairly minor change, but one that we need because of our custom markup on top of Rivescript (we have a custom ^| syntax for example for multiple replies, that rely on continuation characters being transformed into newlines).

We will be using this fork for now, but let me know if you think of a slightly different way of doing it that would enable us to use the mainline again.

Thanks!

@julien-c
Copy link
Contributor Author

julien-c commented Mar 3, 2017

@kirsle Any idea on how we could fix this? I'd love to get this merged (so as not to rely on a fork) or find an alternative solution of doing the same thing.

@kirsle
Copy link
Member

kirsle commented Mar 5, 2017

I'll go ahead and merge this. 😄

I'll add documentation to warn of the possible implications that changing this global option may cause. I'd prefer that RiveScript personalities be entirely self-descriptive, so if an author was relying on the default behavior (and didn't explicitly spell out the concat mode in their files), it could lead to formatting or matching problems when a different botmaster uses that personality, if they change the global setting.

@kirsle kirsle merged commit b46d703 into aichaos:master Mar 5, 2017
@julien-c
Copy link
Contributor Author

julien-c commented Mar 6, 2017

Great! Let me know if I can help with documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants