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

Conditionally restrict available operators #16

Closed
Itangalo opened this issue Jul 4, 2014 · 13 comments
Closed

Conditionally restrict available operators #16

Itangalo opened this issue Jul 4, 2014 · 13 comments

Comments

@Itangalo
Copy link
Contributor

Itangalo commented Jul 4, 2014

I'm using this project in a project used for training math (and some other things).

It would be useful for me to be able to restrict which operators the parser accepts – in particular it would be useful if I could make the parser accept "5/6" as an expression, but not (say) "1/2+1/3".

(I've tried to fiddle with ops2 to conditionally unset some operators, but so far without success.)

@Itangalo
Copy link
Contributor Author

Itangalo commented Jul 4, 2014

(Ok, I now have some success. It's a hack, but it works. If anyone is interested, check out the function 'evaluate' here: https://github.com/Itangalo/waxon/blob/1.x/utils.gs#L173)

EDIT: Link now fixed. Thanks @pajcho for noting that the link was broken.

@pajcho
Copy link

pajcho commented Dec 23, 2015

+1 for adding support for conditionally restricting some operators. It can be useful in lots of situations when we only want to evaluate against certain array of operators.

@Itangalo btw, your link from comment above is not working any more...

@pajcho
Copy link

pajcho commented Dec 23, 2015

Thanks @Itangalo

I have just used your solution as a guideline to make a fix for my project. Here is how it looks - maybe you or someone else will find it useful, although its still very dirty and not a solution I was hoping to get :)

https://gist.github.com/pajcho/c5bbfa805f8993b66900

@silentmatt
Copy link
Owner

I'm thinking about ways to implement this. One possibility would be to pass an object that maps from an operator to a boolean to enable/disable them. For example, { 'in': true, '+': false, ... } to enable in and disable addition.

Another (maybe better) option would be to use the existing unaryOps, binaryOps etc. which can already be used to override the behavior or add new "named" operators (like not, ln, and sqrt, etc.). The advantage to that is consistency with the existing functionality. The way it works now though, it requires you to modify the parser after you create it to delete, change, or add individual properties. There's also not a nice way to have something turned off by default and require explicit opt-in.

Maybe something like this could work:

var parser = new Parser({
  unaryOps: {
    'in': true // Explicitly opt into the default implementation
  },
  binaryOps: {
    '%': false // Disable remainder operator
    '+': (a, b) => a + b // Override the default with your own implementation
  }
});

Neither option is great if you want to only enable specific white-listed operations, because they require you to list everything you don't want. That would require either modifying or replacing the objects after the fact.

Pinging @milouse and @queequac in case you have any thoughts/ideas.

@silentmatt
Copy link
Owner

Another thing to note is that neither of these address other possible options like disabling custom variables and/or functions. Those would probably need be properties on the top-level options object like the current allowMemberAccess option.

@queequac
Copy link

Personally I'd not need any differentiation between unary, binary and so on, I'd be fine with a bucket where all operators are put in. This would only be usefull if you'd have the same symbol for different operator use cases (which is unlikely) while making the options object verbose.

Maybe there is also some useful group for the variables, so you'd not have options at the root level which are hard to group later if needed.

And I'd not mix enable/disable and override. If someone has a function to determine whether the operator is enabled or not, you do not know whether this is a flag or the overriden method. Or at least give it a specific signature, such as an object having a field custom.

Last but not least I'd give these options regular names and won't use the operator symbol itself. Why? Because at least the ternary operator would require to have two symbols or you have to tell people just to use the question mark. And you could even allow people changing the symbols.

new Parser( {
operators: {
'in': true,
'remainder' false,
'add': { custom: (a, b) => a + b; },
'ternary': { symbols: ['?',':'] }
}});

@silentmatt
Copy link
Owner

That's true, I probably could combine them. The original parser needed them to be separate, but right now the only reason they really need to be separate is because + and - are usable as prefix and infix operators, so they have two different implementations. That should be fixable though, which would allow them to be combined. And for configuration purposes it's probably sufficient to have a single object anyway.

I'll play around with your idea. It feels pretty clean, and allows for extra flexibility later if it's needed.

@milouse
Copy link

milouse commented Aug 22, 2017

(just a quick message to apologize for my silence about your poke. I don't take time to think about it, but I'm always willing to answer on this one. I'll try to find time this week!)

@silentmatt
Copy link
Owner

Not a problem at all. I've actually been gone/busy anyway.

@silentmatt
Copy link
Owner

I implemented a simple version of what @queequac suggested in the disable-operators branch. It's just the ability to enable/disable operators for now, but extending it in the future to allow more configuration should be doable. I think I like how it works, so I'll probably merge it into master for the next release.

I grouped the comparison and logical operators for convenience, so { comparison: false } disables, <, >, <=, >=, ==, and != as a group, and { logical: false } disables not, and, and or.

@milouse
Copy link

milouse commented Aug 29, 2017

I think your implementation is good. It's simple, easy to remind and it does the job. In fact I appreciated/agree the proposal of @queequac in his last comment.

Regarding the override feature (not yet implemented, I understand and have no problem with that), I think it leverages another question/feature we not yet discuss: does it sounds usefull for you to have the possibility to restrict the field of operators over one particular operand type?

That way, it may be possible to enable comparison on Numbers, but not on String. And if we are crazy, one may think about a way to map + operator to add function with Numbers and concatenate function with String.

Just to be clear: I currently don't need at all this kind of stuff, but was just thinking about it.

@silentmatt
Copy link
Owner

@milouse That's an interesting idea, but I'm not sure I want to get into the complexities of that at this point, but it might be worth considering for the future. It would certainly be possible to override the operators to add run-time type checks for more advanced use cases.

@silentmatt
Copy link
Owner

I just released 1.1.0 with this enabled. Thanks everyone!

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

No branches or pull requests

5 participants