Community
Participate
Working Groups
In TextViewer: Before Bug 466532, protected void selectionChanged(int offset, int length) was called by TextViewer. Subclasses could override the method. After Bug 466532, a new private method private void selectionChanged(IRegion[] widgetRanges) is now called, which, however, does not call the old method anymore. Overridden implementation in subclasses are not called anymore. The javadoc suggests that the method is supposed to be called *from* subclasses However, since it is protected and non-final, overriding it can also be considered 'API', which is now broken. - Is this considered an API breakage? - Is there a way to still have the old method called? But N times? Without significant performance impact?
It's the grey zone of breaking change: API is not broken because code compiles, but the expectations built on top of them are, although they are not specified... But in practice, some clients are broken without them getting a clear sign that they "abuse" the API. I'm willing to fix that decently, keeping the method invoked. I imagine we can add if (regions.length == 1) { selectionChanged(regions[0].offset, regions[0].range); } either at the beginning or the end of the new selectionChange(IRegion[]) method. What do you think?
I'm not sure this would be a good idea. Usually, overwriting the selectionChanged() method, is not about being notified of such an evet (that's what we already have the listeners for), but to change behavior, e.g.: @Override protected void selectionChanged(int offset, int length) { fireSelectionChanged(offset, length); firePostSelectionChanged(offset, length); // call directly (don't delay); } Just calling the method if length == 1 makes things more complicated and less consistent and modifying superclass behavior would not be possible. Not sure what the best way forward is here. We can probably get that behavior above by some other means.
This broke a feature in our product after upgrading the target platform. Not sure I'd consider this a grey zone. The product targets all Eclipse versions since 4.8 and so far that worked nicely but on the latest platform, there doesn't seem to be a great way to change the implementation of selectionChanged anymore.
(In reply to Sebastian Ratz from comment #2) > Just calling the method if length == 1 makes things more complicated and > less consistent and modifying superclass behavior would not be possible. I'm thinking TextViewer, around line 2334, should decide to call selectionChanged(int, int) if there is only 1 range to keep calling the possible customizations, and call selectionChanged(IRegion[]) otherwise.
(In reply to Mickael Istria from comment #4) > (In reply to Sebastian Ratz from comment #2) > > Just calling the method if length == 1 makes things more complicated and > > less consistent and modifying superclass behavior would not be possible. > > I'm thinking TextViewer, around line 2334, should decide to call > selectionChanged(int, int) if there is only 1 range to keep calling the > possible customizations, and call selectionChanged(IRegion[]) otherwise. Please also consider making the equivalent to protected void selectionChanged(int offset, int length) based on ranges selectionChanged(IRegion[]), protected, too. In our case, we'd like to avoid the queuing of PostSelectionChanges in some case, so it'd be great if all three methods would be accessible in addition. updateSelectionCache(); <- currently private queuePostSelectionChanged(true); <- currently private fireSelectionChanged(widgetRanges); <- this one is already protected
