Skip to content
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

Missing pattern-matching outputs #1220

Merged
merged 3 commits into from
Apr 30, 2020
Merged

Missing pattern-matching outputs #1220

merged 3 commits into from
Apr 30, 2020

Conversation

alexcjohnson
Copy link
Collaborator

@alexcjohnson alexcjohnson commented Apr 30, 2020

Fixes #1216

Pattern-matching callbacks had some issues with ALL wildcards in their Output, reported by user @jaser on the forum. Everything worked well as long as the ALL matched something, but if it didn't we would throw an error.

There are a number of additional cases to consider, involving various combinations of ALL and MATCH wildcards and multiple outputs, as enumerated in #1216 - I will comment on the tests which of these cases each covers. And as discussed in #1216, we will NOT trigger a callback simply because one or more output items was removed. If you want that to happen, include the output component set as inputs as well - our suggestion is to just change the prop to 'id' to avoid circular dependencies, and as a side benefit you get a clean list of the IDs involved.

  • I have run the tests locally and they passed. (refer to testing section in contributing)
  • I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR
  • I have added entry in the CHANGELOG.md

ANY was the original name of the MATCH wildcard...
some variable names didn't make the switch
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be honest, not 100% certain I understand all the intricacies of the code change, but the tests seem 👌 -- 💃

@alexcjohnson alexcjohnson merged commit 5b2ced7 into dev Apr 30, 2020
@alexcjohnson alexcjohnson deleted the missing-outputs branch April 30, 2020 17:40
@alexcjohnson
Copy link
Collaborator Author

Thanks @Marc-Andre-Rivet - this part of the code probably still is more convoluted than it would need to be, though I was pleased to see that this particular change actually slightly reduced LOC in src. I'm glad you're poking around in it, as we continue updating things I'd appreciate any suggestions you have to further clean it up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] pattern-matching callbacks with empty or partly-empty Output
2 participants