Avoid executing unnecessary operations #105
Conversation
- Filter empty change bounds and change routing points operations - Filter empty compound commands - Ensure we only submit a new model if actual changes were applied Relates to eclipse-glsp/glsp#1477
| return this.submitModel(); | ||
| } | ||
| return this.modelSubmissionHandler.submitModel('operation'); | ||
| return []; |
There was a problem hiding this comment.
This may change the behavior in a way that the GModel is not re-generated after an empty operation. Adopters may have relied on that so this would constitute a breaking change as the previous behavior is also valid and may not strictly be classified as a bug. So I am happy to discuss whether this should be changed or not.
There was a problem hiding this comment.
IMO this is more of a bugfix to ensure that the operation handler works as expected.
Nevertheless your are right in certain edge cases adopters may have relied on a model update
(I can`t think of any particular usecase where a effectivly no-op update would be required but who knows).
Maybe we should at least refactor this in overrideable method or property to configure whether update on empty commands should be enabled?
We disable it by default but atleast it gives adopters an easy entry point to change the behavior.
There was a problem hiding this comment.
I agree now that we can consider this more of a bugfix since the Java server was behaving like this all along. So I would almost argue that we do not need an additional flag. The method as-is is already very short and could be overwritten, don't you think?
tortmayr
left a comment
tortmayr
left a comment
There was a problem hiding this comment.
Changes look good to me.
I guess we need a follow-up for the Java server as well?
Follow-up: eclipse-glsp/glsp-server#253 |
What it does
Relates to eclipse-glsp/glsp#1477
How to test
Follow-ups
Changelog