-
Notifications
You must be signed in to change notification settings - Fork 66
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
fix crash when same variable is cached in multiple flyouts #242
Conversation
Looks like build is failing due to recent travis update that went from ubuntu14->16.04. Updated |
Fixed build by pegging it to use 14 ('trusty') instead of the new one (noticed that it was possible from google/blockly#2403) |
This reverts commit 138f24c, as we switched back to the previous ubuntu distro and it is no longer needed
'id, "' + opt_id + '".'); | ||
} | ||
// The variable already exists and has the same ID. | ||
// pxt-blockly: no error if the wrong ID is passed, as we do not allow duplicate variable names |
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.
does blockly have some kind of options where we could disable this check, instead of erasing it. will that be a problem when trying to keep in sync with blockly maintream?
Fixes microsoft/pxt-arcade#980
My understanding is we don't need this crash condition, cause we already only allow a single variable per variable name (versus default blockly which can have multi variables of different types with the same name). I can look into fixing this by invalidating the cached flyouts or something if that approach is preferred, but I don't think this is an an error that really applies to us. Holding off on changing any tests till that's confirmed though.
One other thing: I needed to
npm link
blockly from pxt for my changes to apply, not sure if that's intentional (I didn't see it mentioned anywhere in the docs, can add it).