Finished tracing out scope to its sources in drasil-code
#3888
Conversation
In `drasil-code`, I should be done assigning all variables their proper scope. There's definitely a good chance I made some mistakes, but hopefully we can spot them during testing.
There was a problem hiding this comment.
So I'm approving, in that the code is clean and works, but I'm not going to merge as I want a further investigation first: since this is all done in GenState, maybe this could / should be done that way? i.e. add a notion of scope to DrasilState and then use get/set to work with them. That would end up requiring many fewer code changes.
|
That's a good point. I had it in the back of my mind that there's probably a better way to do this, but I guess I didn't want to lose momentum. I agree that integrating it into |
|
I implemented these changes using |
|
Should we delete the branch? It has unmerged commits. |
|
That should be fine. It was a step in the wrong direction, so we don't want the changes. It was helpful in that it helped me know where the scope needs to be changed in #3899, but since that's done we should be good to delete it. |
For the most part this went pretty smoothly. There was definitely ample opportunity for mistakes though, so hopefully we can find a way to spot them.
In order to test them, I'm thinking of modifying one of the renderers to prepend
global.,local., ormainFn.to each variable. This will let us test everything and give us some empirical assurance.Contributes to #3821