Skip to content
This repository has been archived by the owner on Aug 29, 2024. It is now read-only.

Parenthesis on both sides #251

Closed
wants to merge 2 commits into from
Closed

Conversation

GabrielMahan
Copy link
Contributor

Follow-up from #250 .

In #250 I updated isolateSymbolOnLeftSide but neglected to also update removeSymbolFromRightSide.

I think because https://github.com/socraticorg/mathsteps/pull/250/files#diff-2dc14e1c8322de958fc4c52d204c30b5R42-R45, the right side (rightly) returns a node with a symbol, but because I didn't modify removeSymbolFromRightSide, an error was occurring when the right side couldn't handle the parenthesis node.

Thanks @evykassirer !

@evykassirer
Copy link
Contributor

hm ok so as I was laying in bed last night trying to sleep, I remembered there's a 'removeUnnecessaryParens` that theoretically should solve all these issues, and it's called here https://github.com/socraticorg/mathsteps/blob/master/lib/solveEquation/stepThrough.js#L79 which I think should be before the point where your error comes up?

would you mind looking into this and seeing why you were having those problems despite 'removeUnnecessaryParens`? I remember now that we were trying to avoid putting parenthesis code throughout the codebase, we mostly assumed that unnecessary parens just wouldn't be there

@xZGit
Copy link

xZGit commented Apr 27, 2020

https://github.com/socraticorg/mathsteps/blob/master/lib/solveEquation/stepThrough.js#L58

Set the value of the param "rootNode" to true can fix it

 equation.leftNode = removeUnnecessaryParens(equation.leftNode, true);
 equation.rightNode = removeUnnecessaryParens(equation.rightNode, true);

@GabrielMahan
Copy link
Contributor Author

hey @evykassirer sorry for the delay! will close this PR and address separately

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

Successfully merging this pull request may close these issues.

3 participants