DYN-982 - Fix issues with nested language blocks#12658
Merged
aparajit-pratap merged 8 commits intoDynamoDS:masterfrom Mar 2, 2022
Merged
DYN-982 - Fix issues with nested language blocks#12658aparajit-pratap merged 8 commits intoDynamoDS:masterfrom
aparajit-pratap merged 8 commits intoDynamoDS:masterfrom
Conversation
aparajit-pratap
commented
Mar 1, 2022
mjkkirschner
reviewed
Mar 1, 2022
mjkkirschner
reviewed
Mar 1, 2022
| var cbn = node.CodeBlockNode as CodeBlockNode; | ||
| if (cbn == null) | ||
| { | ||
| var assoc_cbn = node.CodeBlockNode as AST.AssociativeAST.CodeBlockNode; |
Member
There was a problem hiding this comment.
so this handles associative blocks inside of imperative blocks, what about the other way around?
Contributor
Author
There was a problem hiding this comment.
Ah, good catch! Will make a similar change for the associative class.
Contributor
Author
There was a problem hiding this comment.
I had made the change but had commented it out for some reason and then cleaned up the commented code 🤷♂️
Member
|
@aparajit-pratap just had two questions and thanks for the tests! |
mjkkirschner
approved these changes
Mar 1, 2022
pinzart90
approved these changes
Mar 2, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
DYN-982 - Fix issues with nested language blocks
There were 2 issues with nested language blocks. The following script typed in a code block node is an example of this.
aandxare global variables as they are in the outermost scope and are renamed to have globally unique names likea_<guide>, andx_guid, where guid is the same as that assigned to the node model. This is so that the user can use the same names in another code block node without running into name conflicts. The issue was that although the variables were renamed inside theImperativeblock as well, the names were not propagating to the nestedAssociativeblock, due to which these symbols were undefined in that scope. The variable renaming is done using theIdentifierInPlaceMapperclasses, which have been refactored in this PR to take care of the renaming issue.ImperativeIdentifierInPlaceMapperfromCodeBlockNodeModeltoProtoCoreprojectAssociativeIdentifierInPlaceMappertoProtoCoreDeclarations
Check these if you believe they are true
*.resxfilesRelease Notes
Fix issues with nested language blocks
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of