fix <<command()>> and <<command("parameters")>> crashing bondage.js

[blurymind]

This pull prevents commands with js-esque syntax from crashing bondage.js
It addresses bug reported here:
#35

It also adds additional parser tests to make sure the syntax is not crashing it.
The parameters in () dont get passed yet - as it would require some refactoring of nodes.js which was requested by @jhayley to be done in a separate pull.

[hylyh]

Hey thanks! This is looking really good. One thing that I think is worth considering is allowing weirder syntax such as <<blah(blah blah blah)>>. the idea is that anything in between << >> should be returned as strings so I don't think it should be limited to just the classical understanding of "arguments". I'm not entirely sure how that might be gone about though hmmm.

[blurymind]

@jhayley I wonder if we can just add
['BeginCommand Text EndCommand', '$$ = new yy.CommandNode($2);'], /// << anything goes even(this and this)>>
in the parser and treat it the same way as the textNode
:p
Edit: guess not hmm

[blurymind]

Making it completely flexible might interfere with some of the other features though, such as conditional statements. Not sure if I want the text to be returned if its a condition setting. It could be useful for debugging/logging - but it will make it more complicated to filter out commands if they get mixed with these statements - when bondage.js is used in a game. Narrowing it down to some extend is a good thing imo :)

[hylyh]

Hopefully we could treat the conditionals as keywords and allow anything else to be passed as strings to the handler.

This behaviour is handled in lexer/states.js, specifically the command and commandOrExpression nodes. It treats Identifiers differently than text (an identifier is basically what amounts to a valid variable name). Now that I think about it, I'm not sure why this behaviour is there? I dont think yarn functions are valid in the case of them being called like <<function()>> so this may be where this should be fixed.

An interesting side effect that demonstrates this is while <<foo()>> shows the error we're trying to fix <<foo bar()>> does not and works as expected.

Dropping that special identifier behaviour might be the answer

[blurymind]

Very interesting. @jhayley I will try to approach the problem from that angle then :)

[blurymind]

@jhayley
There are still a lot of things to consider when identifying a command. It needs to not be a number of things:
https://github.com/jhayley/bondage.js/blob/master/tests/yarn_files/conditions.json

<<if>>
<<endif>>
<<set>>
<<else>>

I wonder if I am missing something

That functionality is very important to me, so I don't want to break it, nor return any conditional statements as commands. Right now the two are kind of mixed up in the same syntax pattern

Do you think it would be safer if we only return a command if a key word is used?
For example <<command anything() after command>>
could return anything() after command
It would fit more with the currently established syntax, but would require a lot more typing in yarn.
I think you suggested it earlier

Perhaps we can use a shorter key word like "call"?
<<call returned text>>
other key words suggested:

emit
signal
return

An alternative approach would be to use something different to encapsulate commands
<(commands blah blah)>
or
<<{my commands here}>>
How is it done in yarnspinner? :) There has got to be a more ellegant solution to this

[hylyh]

The lexer should already account for those keywords. It first checks to see if the first word is a defined operator, otherwise it continues on to treat it like regular text.

I'd rather not add special syntax since that would diverge too much from how yarnspinner operates. But the lexer behaviour should prevent this from becoming an issue

[blurymind]

@jhayley I am sorry - I am having trouble understanding how exactly the lexer works. Can you help with this change please

[hylyh]

Hi! What are you having trouble with? Feel free to email me at [email protected] since I imagine that'd be easier than github comments!

[blurymind]

thank you for the patience @jhayley :)

[blurymind]

gave this another shot here:
#40

closing this one