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

Use LR parser to parse expressions coming from the Clipboard and generate commands #344

Closed
rudyhuyn opened this issue Mar 20, 2019 · 7 comments
Labels
codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability) needs attention

Comments

@rudyhuyn
Copy link
Contributor

rudyhuyn commented Mar 20, 2019

The current code to manage the clipboard only supports a subset of functions managed by the calculator, has some issues to parse them (many false positives or issues with locales due to the conversion to en-us format but ignoring that '.' and ',' can be used for other purposes in the original locale) and very complicated to maintain/add new features (sin, pin, square, etc..).

In order to simplify and make this function more robust, we should instead use a language parser, more suited to our needs.

My proposal would be to use a custom Lexer to support all the different number formats (hex, bin, dec, bin, dec with exponent, ...) and modes (standard, programmer, etc...) we support and use a LR parser to verify and generate the list of commands. This solution would have many advantages:

  • very easy to add new operator whatever their types (binary, pre-unitary or post-unitary)
  • the parser will generate the sequence of commands while verifying the text, easier to maintain
  • no false positive possible

In my proto-PR, I used Bison, a GPL v3 tool but providing an explicit exception to allow the use of the generated code without license.

It already manages a variety of features not currently available:

sinus/cos/tan
complex operations

** Power, square, cube ** (#299)
Power

more sign operators
moresigns

Binary operations: OR, NOT, AND, XOR
binaryOp
binary

and fix some reported bugs:

Proto-PR available here

https://github.com/rudyhuyn/calculator/tree/AddExpressionParser

remaining:

  • add unit tests
  • it only supports 0;0 and 0;3 grouping for the moment
@rudyhuyn rudyhuyn changed the title Use LR to parse expressions coming from the Clipboard Use LR parser to parse expressions coming from the Clipboard and generate commands Mar 20, 2019
@MicrosoftIssueBot
Copy link
Collaborator

This is your friendly Microsoft Issue Bot. I've seen this issue come in and have gone to tell a human about it.

@grochocki
Copy link
Contributor

Thanks for digging into this, Rudy! Even though this would seem to fix a number of bugs we have been seeing reported, as you point out, since it seems to fundamentally change how we handle pasted input, I am going to track it as a feature. Let's leave this proposal open for discussion.

While not directly related, we are also investigating MathML input (#72) and rendering (#71). If we overhaul our lexing/parsing approach, I think it is worth considering options that may support those efforts as well.

@grochocki grochocki added needs pitch review codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability) and removed Enhancement needs pitch review labels Apr 2, 2019
@Draco18s
Copy link

Draco18s commented Apr 22, 2019

I don't suppose we can include ^ as the exponent command while we're at it, instead of/in addition to y?

3^8 results in invalid input instead of 6561.

@rudyhuyn
Copy link
Contributor Author

rudyhuyn commented Apr 22, 2019

I don't suppose we can include ^ as the exponent command while we're at it, instead of/in addition to y?

3^8 results in invalid input instead of 6561.

It's alread managed by my prototype:
https://github.com/rudyhuyn/calculator/blob/AddExpressionParser/src/CalcViewModel/ExpressionParser/BisonParser.y#L121

3^8 in Scientific mode: 3 power 8
3^8 in Programmer mode: 3 xor 8

@HowardWolosky
Copy link
Member

@rudyhuyn - It seems that this issue and #526 are getting at the same thing and that we should try close one of them out. Is there something in here that you feel isn't getting covered by #526?

@rudyhuyn
Copy link
Contributor Author

rudyhuyn commented Sep 4, 2019

This issue is a sub-task of the other one, you can close it.

Is anyone working on #526?

@HowardWolosky
Copy link
Member

Great, closing this one down.

No one is actively working on the speci'ing of #526 at the moment. If you're interested, just drop your name in over there and @grochocki should be able to get you a working branch in https://github.com/Microsoft/calculator-specs.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability) needs attention
Projects
None yet
Development

No branches or pull requests

5 participants