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
Add SSReflect contextual pattern UNDER
#19011
base: master
Are you sure you want to change the base?
Conversation
17acc1c
to
6948d55
Compare
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.
Wording suggestions
Thanks for reviving this, Erik. I like the fact that this approach is simpler, but it requires the user to understand what goes wrong and it is not easy to explain. For example the changelog entry is unclear to me (if one does not know the implementation of under). I'm wondering if we could instead hide the evar as in:
In this way the evar and its local context are hidden. Would this work? If not I'll just click merge ;-) |
@gares thanks for your comment!
Yes I was also thinking about this idea, but I didn't try it... (and AFAIAC, I won'd be able to experiment this before the end of the month).
Thanks. I don't know yet, but I agree this would be worth it to try. So let's postpone merging this PR for the moment... |
What was the issue with the |
That would also work but needs to be implemented, and can be used outside of the under tactic. |
Just to correct: a prototype has already been implemented: but the implementation looked nontrivial and it still needs to be refined (read: fixed) to fully work... but if it looks nicer to you, maybe it would be worth it to put forth #11200 anew... |
Close #11118
This PR proposes a simpler workaround than PR #11200.
Cc @gares @CohenCyril @proux01 @mrhaandi FYI