Bug 506019

Summary: [regression] editors list shown with Ctrl+E lost some of the functionality
Product: [Eclipse Project] Platform Reporter: Andrey Loskutov <loskutov>
Component: UIAssignee: Patrik Suzzi <psuzzi>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: daniel_megert, gautier.desaintmartinlacaze, Lars.Vogel
Version: 4.7   
Target Milestone: 4.7 M6   
Hardware: All   
OS: All   
See Also: https://git.eclipse.org/r/91417
https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=48609f01e5c3c56c107a86258b6e4ced67d1d2ff
https://git.eclipse.org/r/91548
https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=ffd3eb8f7f1bd19cb0b36dffb4267f317d43e7fd
https://bugs.eclipse.org/bugs/show_bug.cgi?id=512608
Whiteboard:
Bug Depends on: 368977, 512609    
Bug Blocks: 506696, 511702, 512510    
Attachments:
Description Flags
image: MRU not working in Neon 1 (I20161007)
none
image: visual differentiation in tabs with the proposed fix none

Description Andrey Loskutov CLA 2016-10-15 12:24:16 EDT
Follow up on bug 368977, see https://dev.eclipse.org/mhonarc/lists/platform-ui-dev/msg07463.html.

The old editors list shown on Ctrl+E had two features which the current implementation lost:
1) If MRU was off, it showed the tabs in exact same order as they were placed on the screen, NOT in the MRU order
2) The hidden tabs were shown differently (in bold).

TO BE:
1) Very important: keep the order of the editors list as it is set in the MRU preference (general -> Appearance -> "show most recently used tabs").
2) Nice to have: please decorate hidden tabs differently (previously they were shown bold, but I would not like to have them bold, better in slightly different color or BG color).
3) This is not a regression, but is probably wort to fix at same time: show the current tab with some highlight (and here I would expect bold font).
Comment 1 Patrik Suzzi CLA 2016-10-17 06:53:39 EDT
This is feasible and not very complex: 

for (1), I can get the value of MRU from Preferences. If off, I'll list the editors as expected
for (2) and (3), I will add decorators that are easy to change.

However, reading bug 368977 c 17 [#1], we will be missing feature 1. Indeed:
- the current Ctrl+E searches on all the Open Editors, no matter where in the IDE they are.
- the old Ctrl+E searches only on editors in the current Part Stack. 

Hence, I suggest activating the "Search Editors on the current stack" only 
- if the MRU preference is disabled 
- and if the current part stack contains at least one editor 

	boolean mru = preferences.getBoolean(StackRenderer.MRU_KEY_DEFAULT,
			StackRenderer.MRU_DEFAULT);
	if(mru){
		// nothing changes
		return page.getSortedEditorReferences();
	} 
	else (if activeStackContainEditors()){
		// list editors in active stack
		return orderedListOfEditorsOnActiveStack();
	}

#1 https://bugs.eclipse.org/bugs/show_bug.cgi?id=368977#c17
Comment 2 Dani Megert CLA 2016-10-17 10:15:42 EDT
(In reply to Patrik Suzzi from comment #1)
> However, reading bug 368977 c 17 [#1], we will be missing feature 1. Indeed:
> - the current Ctrl+E searches on all the Open Editors, no matter where in
> the IDE they are.

I don't see this, e.g. in N20161014-2000.

> - the old Ctrl+E searches only on editors in the current Part Stack. 

And that's the expected behavior - at least for me.
Comment 3 Lars Vogel CLA 2016-10-28 07:19:05 EDT
(In reply to Dani Megert from comment #2) 
> > - the old Ctrl+E searches only on editors in the current Part Stack. 
> 
> And that's the expected behavior - at least for me.

New behavior is correct. For example, if you detach a editor it has no stack and you could not switch to it via Ctrl+E. That was a long outstanding bug.
Comment 4 Dani Megert CLA 2016-10-28 08:59:37 EDT
(In reply to Lars Vogel from comment #3)
> (In reply to Dani Megert from comment #2) 
> > > - the old Ctrl+E searches only on editors in the current Part Stack. 
> > 
> > And that's the expected behavior - at least for me.
> 
> New behavior is correct. For example, if you detach a editor it has no stack
> and you could not switch to it via Ctrl+E.

It does have a stack. It can contain many editors and Ctrl+E must only switch between those in that detached window.
Comment 5 Lars Vogel CLA 2017-01-23 11:44:16 EST
Mass move. Please move to a concrete milestone if you plan to work on this item.
Comment 6 Andrey Loskutov CLA 2017-02-04 17:39:16 EST
Patrik, do you plan to work on this?
Comment 7 Patrik Suzzi CLA 2017-02-05 21:38:18 EST
(In reply to Andrey Loskutov from comment #6)
> Patrik, do you plan to work on this?

Yes, I'm working on this now.
Comment 8 Patrik Suzzi CLA 2017-02-17 06:52:24 EST
Created attachment 266867 [details]
image: MRU not working in Neon 1 (I20161007)

The MRU preference was not considered in the previous version. 

The solution includes reading the preference "enableMRU" from the preferences node "org.eclipse.e4.ui.workbench.renderers.swt" and display tabs accordingly.
Comment 9 Patrik Suzzi CLA 2017-02-17 21:02:16 EST
Created attachment 266881 [details]
image: visual differentiation in tabs with the proposed fix

I'm submitting a change shortly.

tab order: now depends on the MRU tab preference
tab style: using a StyledCellLabelProvider to differentiate
- active tab: bold
- visible tabs: normal
- hidden tabs: italics, gray

Note: I decided to use italics for the hidden tabs, because the gray foreground color is not visible when selecting the item. (see image)
Comment 10 Eclipse Genie CLA 2017-02-17 21:37:55 EST
New Gerrit change created: https://git.eclipse.org/r/91417
Comment 11 Patrik Suzzi CLA 2017-02-19 15:02:20 EST
(In reply to Eclipse Genie from comment #10)
> New Gerrit change created: https://git.eclipse.org/r/91417

Based on comments, this first change addresses only the MRU order. 

Another following change will address the decoration with a proper disposal of resources.
Comment 13 Patrik Suzzi CLA 2017-02-21 10:23:59 EST
I'm now submitting the change which applies the requested styling to the list. Please note there is one open issue about the background color.
Comment 14 Eclipse Genie CLA 2017-02-21 10:25:28 EST
New Gerrit change created: https://git.eclipse.org/r/91548
Comment 16 Patrik Suzzi CLA 2017-02-22 17:33:31 EST
Closing as fixed