-
Notifications
You must be signed in to change notification settings - Fork 44
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
Implement static event registration for controls in Vireo. #455
Merged
+976
−184
Merged
Changes from all commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
3e46bc0
JS C API for registering control events
segaljared 35911d7
Update Xcode project
spathiwa f897fc2
Refactor RefNumVal type to not carry its type with its data.
spathiwa 09cefde
Add controlEvents module to vireo loader.
spathiwa b5e3c8d
First pass at Vireo-side of static control event registration.
spathiwa 8dd0a36
Fix lint issues.
spathiwa cba8029
Add example static control reg test file (not in testList yet, subjec…
spathiwa 8dadea5
Fix ESLint errors
spathiwa 8ca79a8
Ignore non-numeric controlIDs
spathiwa b13ac87
Add VI UnregisterForStaticEvents, called when top-level VI finishes.
spathiwa 1dc847b
Store controlRef value in EventOracleObj indexed by eventOracleIndex
spathiwa 0a8004e
Make ControlReferences to the same item in the same VI always be the …
spathiwa a41a3d2
Update test to ensure ControlRefs are unique for a given controlID
spathiwa fffa31d
Fix construction of previous refnum from index (and previous refnum h…
spathiwa 9142267
Add error checking in JS OccurEvent to make sure eventOracleIndex mat…
spathiwa 798d069
JS wrapper for the OccurEvent C API
siddhukrs 910829e
Add first set of tests
siddhukrs 895a5e0
Tests for multiple registrations and valuechange on unregistered control
siddhukrs fc94e86
Fix linting errors
siddhukrs bad5298
Merge pull request #1 from siddhukrs/valueChangedEvents
spathiwa d239dfc
Add simple Vireo unit test for Value Changed event.
spathiwa 2c72c1a
Bug fix: Different events on same control should use same eventOracle…
spathiwa a739686
Address Carlos' feedback.
spathiwa 7c3bad6
Address review feedback: Remove exception handling code around js reg…
spathiwa 4fa6c64
Address review feedback from Sankara.
spathiwa f70b2ad
Address rajsite feedback; make default registerForControlEvents callb…
spathiwa 650ae4f
Address Sankara feedbak: use const & instead of * in refnum method.
spathiwa 5a7372f
Fix eslint error
spathiwa d5aa407
Lint is well-named, because it also describes the intelligence of the…
spathiwa 4588fc2
Back out of throw in reg/unreg because it fails control event karma t…
spathiwa c6b3d9f
Update .eslintignore, exclude Documents folder and temp folder in tes…
spathiwa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,3 +8,5 @@ emsdk/* | |
coverage/* | ||
objs/* | ||
gh-pages/* | ||
Documents/* | ||
test-it/temp |
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
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.
"info->controlTag->Delete()" is confusing. I would use the class name directly as this is a static method.
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.
Sorry, I disagree -- controlTag is a StringRef but the static method is in the TypedArray base class; I think accessing the method via the variable type is more robust to changes in the class hierarchy. What's confusing about it?
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.
The way TypedArrayCore and String are implemented now, I don't understand how accessing a static method using an object gives any robustness to changes in the class hierarchy.
If the intent is to keep Delete() as static in TypedArrayCore and have another static/non-static method by the same name "Delete()" in the derived class, that would be a strong code smell.
OTOH, derived class itself can be passed as a template argument to base class and the base class method can in turn call the derived class static implementation. Though it would work, unless there is a strong reason not to make the method in base class virtual, it is not likely a good idea to do (and nothing of that sort is done here anyway currently).
In C# (not sure but likely Java too), it is a compiler error to access a static method using object.
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.
It's more robust because it uses the (declared, not virtual) type of the class to locate the static method to call. If String was refactored to have a more efficient implementation which did not depend on TypedArrayCore but still conformed to the same API, this call would not need to change.
This is not C#.