Change Expressive.Expressions.Binary.BinaryExpressionBase scope from internal to public

[graniero]

Loving this library! It is making my life SO much easier for a big project - except for one small keyword.

I'm setting up a custom Context subclass for our project so that it automatically registers some new custom functions and replaces some other built-in functions on construction. Creating the custom functions was was a breeze to do, just by making implementations of FunctionBase. The constructor also unregisters the bitwise operators, paving the way to use '^' as an exponent operator so we can respect the expression syntax defined in our user interface spec.

• First step: make an implementation of OperatorBase - no problem.
• Next step: make an implementation of BinaryExpressionBase - problem.

Overriding EvaluateImp was straightforward, but unlike FunctionBase and OperatorBase which are declared public, BinaryExpressionBase is declared internal.

Can BinaryExpressionBase be made public instead of internal?

I really don't want to fork and modify just for this keyword - I would really prefer leaving the Expressive package intact and just extend it!

[bijington]

@graniero sorry about that! I had intended to make as much public as possible but it seems I missed this one. I suspect I probably missed it as I have an InternalsVisibleTo exposing the internal parts to the unit tests.

So in summary yes of course we can fix that for you 😄!

I will mark it as a bug as I had intended to expose this and make sure it comes out in the next release.

It's great to hear that you are using it and to good level of customisation. I have to hold my hand up and admit the documentation has fallen short recently so you did well to piece it all together! Would you mind letting me know how you discovered parts like the Context customisation ability? Or if you have any suggestions on what documentation you might like, e.g. would examples have been the best way to describe it or some level of API documentation?

[graniero]

Awesome, thanks! And thanks for such a wicked fast response!

I worked it out by good old-fashioned code crawling 😄! Your code is nice and clean, so it was really easy to follow the pathways through it, so thank you for that - much respect. I also did some quick code experimentation and jotted down some behaviour notes for a bunch of the in-built functions. I'd be happy to turn my notes into some function reference updates or some quick-reference material about customization - fair exchange for the ton of time this library will save me on our project!

If you're interested, let's chat about timing so you could include it in your next release.

[bijington]

Thanks I appreciate the compliment.

Yes I would definitely be interested to hear your notes on the in-built functions! I have started a discussion that is probably the best place to go over this (I think) here #86. I am definitely keen to see if there are ways to improve the base functionality for users. I have debated the use of ^ operator for bitwise or. Sadly it has been in for so long I don't know how dangerous it would be to remove but you aren't the first person to suggest it should be exponent. Hopefully now it is easy enough for the user to decide what it should be.

I am not in a particular rush to ship out a new version but I appreciate you may want this fairly soon... Ideally I wanted to try and get #77 resolved in the next big release but this will introduce a breaking change. It does expose a fundamental flaw though I believe. The changes for #77 will eventually result in some function updates (e.g. Contains, EndsWith and StartsWith will likely be enhanced to accept a boolean to decide whether to match case or not).

With the above said I see no reason why I couldn't ship a minor release with the behavioural improvement that you need first and then move on to the other issue.

[graniero]

Super! I'll continue the conversation in those two spots, but a couple quick teasers:

• The caret is crucial to preserve as standard bitwise XOR for people doing bit work, but for people using caret as standard exponent symbol, I'll guess most are probably not doing bit work. Based on that assumption I have an idea that can cover this without breaking code. I'll put details at #86.
• Yeah, IgnoreCase is mighty tricky. The idea of a boolean argument for Contains etc. is excellent, and I can think of cases where those functions' Ignore mode would need to be treated differently than a context-wide 'ignore case for parsing function names etc.` and 'ignore case for equality tests'. I also have an idea how to separate options for parsing and equality without introducing a breaking change. I'll put details at #77.

[bijington]

Great thanks!

[bijington]

Fixed and shipped in release v2.3.0