search

google / mathsteps

Step by step math solutions for everyone
https://socratic.org
Apache License 2.0
2.11k stars 276 forks source link

Parenthesis on both sides #251

Closed GabrielMahan closed 4 years ago

GabrielMahan commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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