Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 319800 - Close/Close Others/Close All menu in editor
Summary: Close/Close Others/Close All menu in editor
Status: VERIFIED FIXED
Alias: None
Product: e4
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 1.0   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 1.0 RC2   Edit
Assignee: Oleg Besedin CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-13 19:22 EDT by Susan McCourt CLA
Modified: 2010-07-20 11:37 EDT (History)
6 users (show)

See Also:

Attachments
Patch (12.89 KB, patch)
2010-07-16 11:47 EDT, Oleg Besedin CLA 		no flags Details | Diff
Patch 2 (12.59 KB, patch)
2010-07-16 13:35 EDT, Oleg Besedin CLA 		no flags Details | Diff
Patch 3 (7.24 KB, patch)
2010-07-16 14:11 EDT, Oleg Besedin CLA 		no flags Details | Diff
Patch 4 (12.64 KB, patch)
2010-07-16 14:14 EDT, Oleg Besedin CLA 		no flags Details | Diff
Show Obsolete (3) View All 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 Susan McCourt CLA 2010-07-13 19:22:22 EDT 
It would be nice to have a submenu on editor tabs for the close functions.
For those who aren't used to the key bindings, we make it hard to figure out how to do it because:
- there is no menu
- there is no key assist dialog (to look it up)
- we've removed the keys pref page (to look it up)

close others doesn't have a binding by default anyway...
Comment 1 Oleg Besedin CLA 2010-07-16 11:47:07 EDT 
Created attachment 174510 [details]
Patch

Eric, could you review the patch?
Comment 2 Eric Moffatt CLA 2010-07-16 12:09:21 EDT 
Oleg, I haven't examined the patch itself yet but I see an NPE...

Open a few editors, use 'Close' on an editor that is *not* the active one then try Close All...
Comment 3 Oleg Besedin CLA 2010-07-16 13:35:52 EDT 
Created attachment 174520 [details]
Patch 2

Good point. I changed code to always get part service from the stack's context.
Comment 4 Remy Suen CLA 2010-07-16 13:42:49 EDT 
(In reply to comment #3)
> Good point. I changed code to always get part service from the stack's context.

Your second patch is getting the part service from the _application's_ context. I feel like this will cause problems (remember bug 315270 comment 36? :)).
Comment 5 Oleg Besedin CLA 2010-07-16 13:57:13 EDT 
(In reply to comment #4)
> (In reply to comment #3)
> > Good point. I changed code to always get part service from the stack's context.
> Your second patch is getting the part service from the _application's_ context.
> I feel like this will cause problems (remember bug 315270 comment 36? :)).

Hmm... So, is there a good way to get THE context to get part service? Other than walking up EMF tree looking for perspective / workbench window / application ?
Comment 6 Oleg Besedin CLA 2010-07-16 14:11:52 EDT 
Created attachment 174524 [details]
Patch 3

Remy suggested to use AbstractPartRenderer.getContextForParent(MUIElement); seems to be working for me.
Comment 7 Oleg Besedin CLA 2010-07-16 14:14:10 EDT 
Created attachment 174525 [details]
Patch 4

Previous patch did not have all the files.
Comment 8 Oleg Besedin CLA 2010-07-16 15:27:42 EDT 
Patch applied to CVS Head.
Comment 9 Eric Moffatt CLA 2010-07-19 08:31:03 EDT 
good one, I did test it on Friday but didn't hit the 'commit' for the comment saying it looked fine to me..;-).
Comment 10 Dani Megert CLA 2010-07-20 11:37:41 EDT 
Verified in SDK 4.0 - I20100718-2237.