Bug 491273 - Expose Window.getClosestMonitor() as API
Summary: Expose Window.getClosestMonitor() as API
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.6   Edit
Hardware: PC Linux
: P3 enhancement (vote)
Target Milestone: 4.10 M3   Edit
Assignee: Andrey Loskutov CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
Depends on:
Blocks: 538594
  Show dependency tree
 
Reported: 2016-04-07 15:48 EDT by Stefan Xenos CLA
Modified: 2018-11-20 05:02 EST (History)
4 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Xenos CLA 2016-04-07 15:48:22 EDT 
Window.getClosestMonitor() has been copied-and-pasted many times in the Eclipse platform. We should move this to an API method somewhere (either in SWT or JFace) and delete all the copies.
Comment 1 Lars Vogel CLA 2016-04-07 16:05:20 EDT 
For 4.6 we could move this method to a separate util class (as it Javadoc) suggests and start using it without making it API, for example with the @noreference Javadoc annotation. 

IIRC @noreference does not create compile warnings but still avoids creating API.
Comment 2 Stefan Xenos CLA 2016-04-07 17:13:38 EDT 
We could, but we could also just wait for 4.7. I don't think there's any need to rush this cleanup since the code works fine even with the duplication.
Comment 3 Lars Vogel CLA 2016-04-07 17:18:38 EDT 
(In reply to Stefan Xenos from comment #2)
> We could, but we could also just wait for 4.7. I don't think there's any
> need to rush this cleanup since the code works fine even with the
> duplication.

+1
Comment 4 Stefan Xenos CLA 2016-04-07 22:36:06 EDT 
While merging the code, we might also consider switching from using a "closest center point" algorithm to using a "closest perimeter point" algorithm.
Comment 5 Lars Vogel CLA 2018-04-27 09:14:24 EDT 
Nikita, something for you to add to SWT?

Note for myself: Once we have such an API, I can delete WBWRenderer#getClostestMonitor.
Comment 6 Andrey Loskutov CLA 2018-10-16 11:49:50 EDT 
Nikita seem to not work on this anymore.

I have another one patch "on hold" trying to copy/paste same code (https://git.eclipse.org/r/#/c/128617/).

Stupid question: why do we want to have this method to a separate class? Why not just let it be in Window?

If not, I would move ot to jface.util.Util class - the method uses jface.util.Geometry, so same package.

My intent is to release this code and merge the patch above without code duplication, also remove remaining duplicates afterwards.

Any objections?
Comment 7 Lars Vogel CLA 2018-10-16 12:02:02 EDT 
No objections
Comment 8 Andrey Loskutov CLA 2018-10-16 12:04:27 EDT 
(In reply to Lars Vogel from comment #7)
> No objections

To which one: keep in jface Window or move to jface Util?
Comment 9 Lars Vogel CLA 2018-10-16 12:07:55 EDT 
(In reply to Andrey Loskutov from comment #8)
> (In reply to Lars Vogel from comment #7)
> > No objections
> 
> To which one: keep in jface Window or move to jface Util?

Whatever you prefer ;-)
Comment 10 Eclipse Genie CLA 2018-10-17 05:30:24 EDT 
New Gerrit change created: https://git.eclipse.org/r/131043
Comment 12 Eclipse Genie CLA 2018-10-17 08:17:29 EDT 
New Gerrit change created: https://git.eclipse.org/r/131052
Comment 13 Eclipse Genie CLA 2018-10-17 08:19:42 EDT 
New Gerrit change created: https://git.eclipse.org/r/131053
Comment 14 Eclipse Genie CLA 2018-10-17 08:25:22 EDT 
New Gerrit change created: https://git.eclipse.org/r/131056
Comment 16 Eclipse Genie CLA 2018-10-17 11:06:54 EDT 
New Gerrit change created: https://git.eclipse.org/r/131073
Comment 20 Andrey Loskutov CLA 2018-10-19 17:22:17 EDT 
I believe to removed all duplicates I see in my workspace.
Comment 21 Lars Vogel CLA 2018-11-20 05:02:04 EST 
Andrey, could you add this to the N&N 4.10?