WIP: Substitute symbols for expressions from optional scope #212
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| *swp | ||
| node_modules | ||
| *.log | ||
| .DS_Store | ||
| .idea |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| const ChangeTypes = require('../../ChangeTypes'); | ||
| const Node = require('../../node'); | ||
| const TreeSearch = require('../../TreeSearch'); | ||
|
|
||
| // Searches through the tree, prioritizing deeper nodes, and substitutes | ||
| // in-scope values for their respective expressions on a symbol node if possible. | ||
| // Returns a Node.Status object. | ||
|
There was a problem hiding this comment. awesome documentation :D |
||
| const search = TreeSearch.postOrder(scopeSubstitution); | ||
|
|
||
| function scopeSubstitution(node, options={}) { | ||
| if (Node.Type.isSymbol(node)) { | ||
| return substituteAndSimplifySymbols(node, options); | ||
| } | ||
| else { | ||
| return Node.Status.noChange(node); | ||
| } | ||
| } | ||
|
|
||
| // SUBSTITUTES | ||
| // Returns a Node.Status object with substeps | ||
| function substituteAndSimplifySymbols(node, options={}) { | ||
| if (!options.hasOwnProperty('scope')) { | ||
| return Node.Status.noChange(node); | ||
| } | ||
|
|
||
| if (!Node.Type.isSymbol(node)) { | ||
| return Node.Status.noChange(node); | ||
| } | ||
|
|
||
| let symbolName; | ||
| if (node.type === 'SymbolNode') { | ||
|
There was a problem hiding this comment. you can use There was a problem hiding this comment. WAIT does this return true for unary minus too? okay this makes sense https://github.com/socraticorg/mathsteps/blob/344d15b5d5e25251cef7d6976d6a73df16661d7d/lib/node/Type.js#L32 For any isSymbol thing you've added in this PR can you pass in Sorry, that's weird that we did that (though I'm assuming there's a reason I've now forgotten XD but feel free to investigate if you're curious or open an issue) There was a problem hiding this comment. I left this alone for now since you followed up with the change to |
||
| symbolName = node.name; | ||
| } | ||
| else if (Node.Type.isUnaryMinus(node)) { | ||
|
There was a problem hiding this comment. now that I updated the so you can take out |
||
| symbolName = node.args[0].name; | ||
| } | ||
|
|
||
| const scope = options.scope; | ||
| if (scope.hasOwnProperty(symbolName)) { | ||
| // when declared at top, kept getting | ||
| // TypeError: <nameIGaveToFunction> is not a function | ||
|
There was a problem hiding this comment. what is There was a problem hiding this comment. ...this was my way of trying to communicate that, no matter what name I gave to the function that made this call, it would fail with that error message. There was a problem hiding this comment. which is the function making the call? is it in the PR here? what's it called right now? There's so many functions in the code haha so confused which one you're referring to! |
||
| const simplifyExpression = require('../../simplifyExpression'); | ||
|
There was a problem hiding this comment. so What you have here also means if you have My suggestion instead is:
I know it's a bit different from your idea, so we could chat more about this and make sure we're on the same page and both happy with whatever we decide to do. I think adding some tests that show multiple steps will also make it more clear what this changes. Let me know if you have more questions or comments about this! |
||
| const substeps = simplifyExpression(scope[symbolName]); | ||
| if (substeps.length === 0) { | ||
| const newNode = Node.Creator.constant(Number(scope[symbolName])); | ||
| return Node.Status.nodeChanged( | ||
| ChangeTypes.SUBSTITUTE_SCOPE_SYMBOL, node, newNode); | ||
| } | ||
| else { | ||
| const newNode = substeps.slice(-1)[0].newNode; | ||
| return Node.Status.nodeChanged( | ||
| ChangeTypes.SUBSTITUTE_SCOPE_SYMBOL, node, newNode, false, substeps); | ||
| } | ||
| } | ||
| else { | ||
| return Node.Status.noChange(node); | ||
| } | ||
| } | ||
|
|
||
| module.exports = search; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| function substituteScope(scope) { | ||
|
There was a problem hiding this comment. just copying from before so it's easier to keep track of what's left (no worries for not getting to it yet)
|
||
| // create a copy of the provided scope | ||
| const newScope = Object.assign({}, scope); | ||
|
|
||
| // iterate over symbols | ||
| for (var symbol in newScope) { | ||
| // store the symbol's current value in a variable | ||
| const targetVal = newScope[symbol].toString(); | ||
| // iterate through scope (again) | ||
| for (var sym in newScope) { | ||
| // extract the value of a symbol as a string | ||
| const valStr = newScope[sym].toString(); | ||
| // within the stored value, replace symbol (from the outer loop) | ||
| // with its respective value | ||
| const replaced = valStr.replace(symbol, targetVal); | ||
| // store the substituted value in the new scope under the same symbol | ||
| newScope[sym] = replaced; | ||
| } | ||
| } | ||
|
|
||
| return newScope; | ||
| } | ||
|
|
||
| module.exports = substituteScope; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think with my unary minus change, it doesn't have to be checked first - can you switch it back and see if that works? Before, symbols with unary minuses were counted as symbols too by default, but now it
Node.Type.isSymbol(node)on a unary minus node should return false by default :D