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 support for multiplying parentheses #87

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

juca1331
Copy link

Hello, the package didn't have the capability to perform mathematical operations with parentheses, so I decided to add this functionality within the parse method of the Parser class. To enable this functionality, it's necessary to set the 'multiplyWithParentheses' attribute to true in the parse method.

@fkleon fkleon added the enhancement Indicates new feature requests label May 25, 2024
@fkleon
Copy link
Owner

fkleon commented May 25, 2024

Hi @juca1331, thanks for your pull request!

If I'm understand this change correctly, it will allow the parser to interpret a missing operator between parenthesis to be an implicit multiplication? To be honest, the code is a bit hard to follow. If that understanding is correct, I'm not sure that string manipulation on the input expression string is the best way to achieve this. Such functionality would be better implemented during or after the tokenization step within the parser.

A few further notes:

  • Could you please update this PR to only include relevant changes? There is quite a lot of noise, but I think the relevant files are only parser.dart and the CHANGELOG file.
  • Any new functionality requires unit tests to be added. This proves that the code is correct, ensures it is maintainable going forward, and also helps readers to understand how the feature is supposed to work.

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

Successfully merging this pull request may close these issues.

None yet

2 participants