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 math-parser library #2033

Merged
merged 41 commits into from
Nov 21, 2024
Merged

Add math-parser library #2033

merged 41 commits into from
Nov 21, 2024

Conversation

urisinger
Copy link
Contributor

@urisinger urisinger commented Oct 12, 2024

Partly closes #2026

For more info check out the refenced issue.

@Keavon Keavon changed the title Implement math parser Math parser/calculator library Oct 12, 2024
@urisinger urisinger deleted the branch GraphiteEditor:master October 18, 2024 07:57
@urisinger urisinger closed this Oct 18, 2024
@urisinger urisinger reopened this Oct 31, 2024
Copy link
Member

@0HyperCube 0HyperCube left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good; I'm sorry that it has taken so long to merge anything related to this.

"invsec" => Some(Value::Number(Number::Complex(complex.recip().acos()))),
"invcot" => Some(Value::Number(Number::Complex((Complex::new(PI / 2.0, 0.0) - complex).atan()))),

_ => None, // Handle unknown function names
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A warning here (just to the console) might be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when a function lookup fails we should check if an ident with that name exists, for now i changed it to be a hashmap lookup instad.

libraries/math-parser/src/context.rs Outdated Show resolved Hide resolved
@Keavon
Copy link
Member

Keavon commented Nov 21, 2024

It looks like I don't have permission to push to your branch. Can you delete the test at the very bottom of editor_api.rs which is failing CI?

@Keavon
Copy link
Member

Keavon commented Nov 21, 2024

!build

Copy link

📦 Build Complete for 2f0a0d5
https://28d203e7.graphite.pages.dev

@Keavon
Copy link
Member

Keavon commented Nov 21, 2024

From my tests, this seems to work well when replacing meval! Thank you. And sorry it took so long to get to.

@urisinger
Copy link
Contributor Author

From my tests, this seems to work well when replacing meval! Thank you. And sorry it took so long to get to.

no problem, this still needs quite a bit of work to work as its own node, mainly becuase it needs some sort of compile time node generation

@Keavon
Copy link
Member

Keavon commented Nov 21, 2024

That'll be a good step for the next PR. I'm looking forward to that! This one is good to merge now.

@Keavon Keavon changed the title Math parser/calculator library Add math-parser library Nov 21, 2024
@Keavon Keavon merged commit 9fb4947 into GraphiteEditor:master Nov 21, 2024
3 of 4 checks passed
@urisinger urisinger deleted the master branch November 21, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking Issue: Fully-featured math expression parser and calculator
3 participants