Bug 420956 - [Perspectives] Fix perspective customization on 4.x
Summary: [Perspectives] Fix perspective customization on 4.x
Status: CLOSED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.3.1   Edit
Hardware: All All
: P3 major with 8 votes (vote)
Target Milestone: 4.5 M7   Edit
Assignee: Andrey Loskutov CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 384349 (view as bug list)
Depends on: 134659 280033 334391 348331 371934 373007 376046 378849 383569 387821 391957 393391 395601 397960 404009 404348 407453 407522 411577 413329 413657 414738 418022 419188 421178 421388 424638 426020 428729 437326 445538 447684 448873 456727 456729 457198 459641 460039 460503
Blocks: 441512 493138
  Show dependency tree
 
Reported: 2013-11-03 05:28 EST by Andrey Loskutov CLA
Modified: 2016-05-06 09:30 EDT (History)
19 users (show)

See Also:


Attachments
Patch for false "This item cannot be ... in unavailable command groups: null" (904 bytes, patch)
2014-11-17 13:38 EST, Petr Bodnar CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Loskutov CLA 2013-11-03 05:28:40 EST
Perspective customization is very important for Eclipse but is currently (4.3.1) seriously broken.

This is an overview bug to track issues with perspective customization in 4.x stream. I hope that this bug could help to get "stabilize perspective customization story" item into Eclipse 4.4 plan (as of today there are no plans for 4.4 (see [1] and [2]) at all).

Current issues with perspective customization:

1) Some items are not appearing in the Customize Perspective Dialog (CPD).
2) Some items have wrong state in CPD compared with the workbench.
3) Some items can not be configured in CPD (changes have no effect).
4) Changes of some items affects other perspectives.
5) "Reset perspective" seems to be broken.
n) To be continued...

Here is one query to see *some* of the bugs mentioned above:

https://bugs.eclipse.org/bugs/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&classification=Eclipse&component=IDE&component=UI&list_id=7365697&longdesc=customize%20perspective&longdesc_type=allwordssubstr&product=Platform&query_format=advanced&version=4.0&version=4.1&version=4.2&version=4.2.1&version=4.2.2&version=4.3&version=4.3.1&version=4.4&order=bug_id


[1] http://wiki.eclipse.org/Eclipse/Luna_Plan
[2] http://www.eclipse.org/projects/project-plan.php?planurl=eclipse/development/plans/eclipse_project_plan_4_4.xml
Comment 1 Paul Webster CLA 2013-11-04 12:27:25 EST
The Platform UI plan is http://wiki.eclipse.org/Platform_UI/Plan/4.4 and the more specific plan is http://wiki.eclipse.org/Platform_UI/Plan/4.4/Milestones

PW
Comment 2 Andrey Loskutov CLA 2013-11-04 17:48:38 EST
(In reply to Paul Webster from comment #1)
> The Platform UI plan is http://wiki.eclipse.org/Platform_UI/Plan/4.4 and the
> more specific plan is http://wiki.eclipse.org/Platform_UI/Plan/4.4/Milestones

Thanks for the link. Nice to see that some of the linked bugs are fixed or at least planned to be fixed.

A bit off topic:
Paul, as someone from the "outside" of the planning council and platform team how one should now that there are any plans if the "projects plans" page is utterly out-of-date or out-of-content? Could you please add a link from http://www.eclipse.org/projects/project-plan.php?planurl=eclipse/development/plans/eclipse_project_plan_4_4.xml to http://wiki.eclipse.org/Platform_UI/Plan/4.4? 
Or I'm missing the point of having empty "project plans" page? How an average user is supposed to find links to project plans then?
Comment 3 Paul Webster CLA 2013-11-06 14:35:17 EST
The planning bug that you link to should be filled in soon, but it will be with very high level items (over-arching themes at the PMC level).

Components (like Platform UI) run their own planning exercises on their own dev mailing lists (ex: http://dev.eclipse.org/mhonarc/lists/platform-ui-dev/msg05487.html )

Listening to the dev mailing list is how to be notified about planning (or anything that the dev community for that component is discussing).  One of the outcomes of the Platform UI planning meetings was an effort to make it more predictable, Platform UI planning should be the 2 weeks after XXXX SR1 is delivered.

PW
Comment 4 Tony Weddle CLA 2014-03-20 00:34:21 EDT
There are all sorts of perspective customization problems, as this bug attests to. Just try all combinations one can think of and many of them will go wrong. A few recent ones I tried: reset perspective sometimes works immediately, sometimes takes a restart of the perspective and sometimes never works; sometimes, clearing all of the command groups will clear all or most of the command groups in other perspectives also. It can be impossible to get the command groups and commands back again, even with the workarounds mentioned on other bugs, without switching to a new workspace.

Basically, the customize perspective function is a mess (even on 4.4M6) and almost unusable.
Comment 5 Andrey Loskutov CLA 2014-04-23 07:22:45 EDT
Really bad that it doesn't make into 4.4...
Comment 6 Petr Bodnar CLA 2014-10-28 14:06:54 EDT
Suggestion #1: What about killing maybe a third of the linked bugs as DUPLICATES? It seems like people keep reporting the same problems again and again. This makes the overview of the problems with perspectives customization (and the work on them!) hard to track.

Suggestion #2: Why not identify about 3 main problems (like 1) the preferences dialog itself, 2) the toolbar rendering, 3) persistence of the preferences) and then start working on them? Or is there some common discriminator to all of the problems? How could it get so broken when it worked for years before (there was no abstraction or it was that bad it was all written from scratch)? Could someone give at least a hint where in the code base to look for the individual problems so maybe someone outside of the "core" team which doesn't seem to have time to fix this could look at it? I know one can also watch some mailing list but...

Thanks for the efforts of the ones who have already tried to somehow manage this!
Comment 8 Petr Bodnar CLA 2014-11-09 07:09:20 EST
Well, it looks like somebody got to be kidding us in here: Just tried with Luna SR1 and with the head from R4_4_maintenance (the last change in CustomizePerspectiveDialog.java is 7 months ago if EGit shows the history correctly). And it didn't get any better, maybe even worse, with the VERY BASIC functionality: once I hide a toolbar item, I cannot re-enable it:

* Eclipse warns me about "This item cannot be ... in unavailable command groups: null" (bug: a) it isn't!, b) "null"!)
* Eclipse displays the possibility to "switch to the Command Group Availability tab", but it does nothing (bug: nothing happens!)

After 2 or 3 hours of debugging the *Kepler* version, it looks evident that there was basically 1 MAIN CAUSE of the most of the reported BASIC errors:

---
On CPD rendering, `toolbarMngrRenderer.getContribution(element)` was returning VISIBLE contribution item although the corresponding toolbar item was hidden. 

And consequently on CPD saving, when the user tries to re-hide the already hidden item, that item is already among `((WorkbenchPage) window.getActivePage()).getHiddenItems()` (notice that quite another approach is used in here, compared to rendering) => the dialog ends like no changes were made to the settings.
---

In Luna+, the problem got maybe corrected, but quite new problems (see above) arose on the way by the fix, or by adding some new functions (please don't do that, let's concentrate only on the fixing)? I wonder whether it is worth to checkout the trunk (master) branch for 4.5 and try to investigate against that? Any ideas?

Sorry for not reporting the above errors separately, but it really seems like wasting time when errors like these are so evident and I'm tired of navigating through all the duplicate bugs...
Comment 9 Petr Bodnar CLA 2014-11-17 13:38:31 EST
Created attachment 248707 [details]
Patch for false "This item cannot be ... in unavailable command groups: null"

Attaching a (simple!) patch for the problem described at my previous comment (try it for the "Forward" button, for example). Notes and questions:

* This patch expects that item can have no actionSet assigned. This was expected in the very first commit of CPD.java by Dean Roberts and is still expected at one other place in the code (simply search for the lines bellow "// Is it possible for this item to be visible?").

* I cannot say if this cannot break other things somewhere else (yet). Can somebody else?

* I think it is a question on Paul Webster who on 21.8.2012 did the commit efbb93c4d49831f615b0d31b506f2daa2097f335 for implementing bug 378845 (= adding back missing features to the dialog) why he changed these lines:

	private static boolean isAvailable(DisplayItem item) {
		if (item.getActionSet() == null)
			return true;
		if (item.getActionSet().isActive())
			return true;

to:

	private static boolean isAvailable(DisplayItem item) {
		if (item.getActionSet() != null && item.getActionSet().isActive())
			return true;

(accident when trying to make it simpler, or really a controlled intention?)

... plus put at the end (why not start?) of the method (orig.: "return false;"):

	return item.getIContributionItem() != null && item.getIContributionItem().isVisible();

* Similar change from the same commit results in incorrectly informing the user about the item unavailability:

-if (item.getChildren().isEmpty()) {
+if (item.getChildren().isEmpty() && item.getActionSet() != null) {
//i.e. is a leaf
Comment 10 Petr Bodnar CLA 2014-11-17 14:03:22 EST
This maybe relates to bug 378849. But this takes so long already... And I'm a bit scared (hopefully only because I don't understand it ;)) of the way it is about to be resolved (only new and new lines and complexity added to the CPD.java) - see https://git.eclipse.org/r/#/c/25821 (linked from the mentioned bug)
Comment 11 Andrey Loskutov CLA 2014-12-28 14:35:26 EST
I've debugged the code a lot over the Christmas time and I think I've got some useful state, here are patches which should fix (I hope) most of the issues with perspective customization on 4.x.

The work is based on two great patches:
1) from bug 391430 by Sopot Cela (patch 18556) and René Brandstetter (patch 24126/25565)
2) from bug 378849 by Erik Chou and Paul Paul Webster (patch 25821), which is now part of patch 38803.

The required patches in the logical order:
https://git.eclipse.org/r/25565 (Bug 391430)
https://git.eclipse.org/r/38791 (don't use hardcoded menu id's)
https://git.eclipse.org/r/38636 (aded lot of toString()'s for debugging)
https://git.eclipse.org/r/38634 (Perspective cleanup)
https://git.eclipse.org/r/38635 (Perspective refactoring)
https://git.eclipse.org/r/38801 (CPD refactoring/cleanup)
https://git.eclipse.org/r/38802 (Bug 404348, 421178)
https://git.eclipse.org/r/38803 (the final one)

Fetching the https://git.eclipse.org/r/38803 from Gerrit should also get all the required patches.

I haven't much tested it in real life - the most time I've spent in debugger. I would really appreciate if the people on CC could grab the patches and start testing them. The CPD codebase is ... is not the nicest code I've seen in my life, so there ARE bugs, and it would be great to find the obvious ones early.

Merry Christmas & Happy New Year!
Comment 12 Andrey Loskutov CLA 2014-12-29 05:43:50 EST
One hour later ... first bug found: the patch https://git.eclipse.org/r/25565 from Bug 391430 was doing too much in MenuManagerRenderer.java, so that the "Team" menu was only shown once and never again. 

As I don't really understand the "dynamic" part of the patch, I've rebased the patches so that they don't use the entire patch 25565 and included a smaller fix for MenuManagerRenderer.java in the last patch 38803, without all the "dynamic" stuff.


The required patches in the logical order:
https://git.eclipse.org/r/38791 (don't use hardcoded menu id's)
https://git.eclipse.org/r/38636 (aded lot of toString()'s for debugging)
https://git.eclipse.org/r/38634 (Perspective cleanup)
https://git.eclipse.org/r/38635 (Perspective refactoring)
https://git.eclipse.org/r/38801 (CPD refactoring/cleanup)
https://git.eclipse.org/r/38802 (Bug 404348, 421178)
https://git.eclipse.org/r/38803 (the final one)
Comment 13 Andrey Loskutov CLA 2015-01-02 04:48:58 EST
I've just added fix for bug 383569, and so the full patch list is now:

https://git.eclipse.org/r/38791 (don't use hardcoded menu id's)
https://git.eclipse.org/r/38636 (aded lot of toString()'s for debugging)
https://git.eclipse.org/r/38634 (Perspective cleanup)
https://git.eclipse.org/r/38635 (Perspective refactoring)
https://git.eclipse.org/r/38801 (CPD refactoring/cleanup)
https://git.eclipse.org/r/38802 (bug 404348, bug 421178)
https://git.eclipse.org/r/38803 (major set of CPD related fixes)
https://git.eclipse.org/r/38896 (bug 383569)
Comment 14 Lars Vogel CLA 2015-01-05 15:30:49 EST
I reviewed several Gerrit reviews from Andrey. Great work. I merged one and left comments for several (minor) issues in several others. Once they are updated, I can review again.
Comment 15 Lars Vogel CLA 2015-01-06 15:13:39 EST
(In reply to Andrey Loskutov from comment #13)
> I've just added fix for bug 383569, and so the full patch list is now:

FYI - Andrey opened for several of his Gerrit reviews new bugs. @Andrey could you add them as blocking for this bug?
Comment 16 Andrey Loskutov CLA 2015-01-06 15:48:48 EST
(In reply to Lars Vogel from comment #15)
> FYI - Andrey opened for several of his Gerrit reviews new bugs. @Andrey
> could you add them as blocking for this bug?

Done, some were already blocking.

Now as the smaller patches are already merged, and one new patch is added, here is the updated *pending* list:

https://git.eclipse.org/r/38801 (bug 456729, move CPD to "cpd" package)
https://git.eclipse.org/r/38802 (bug 404348, bug 421178)
https://git.eclipse.org/r/38803 (this bug - major set of CPD related fixes)
https://git.eclipse.org/r/38896 (bug 383569)
https://git.eclipse.org/r/39074 (bug 448873)
Comment 17 Andrey Loskutov CLA 2015-01-10 19:11:36 EST
Short update on current state: I think I'm almost done, one known issue (which I'm planning to address soon, hope the last one): on next startup toolbars disabled in one perspective aren't shown anymore in *all* perspectives, however resetting *any* perspective (or enabling those toolbars) restores they state for all perspectives again. This affects only *entire* toolbars, single buttons are OK.

Pending patches: 
https://git.eclipse.org/r/39345 - bug 457198
https://git.eclipse.org/r/38802 - bug 404348
https://git.eclipse.org/r/38803 - this bug - major set of CPD related fixes
https://git.eclipse.org/r/39349 - bug 383569
Comment 18 Andrey Loskutov CLA 2015-01-11 13:19:34 EST
(In reply to Andrey Loskutov from comment #17)
> one known issue: on next startup
> toolbars disabled in one perspective aren't shown anymore in *all*
> perspectives, however resetting *any* perspective (or enabling those
> toolbars) restores they state for all perspectives again. This affects only
> *entire* toolbars, single buttons are OK.

OK, this last issue is now fixed (part of the fix for bug 383569), plus I've added fix for bug 448873.

Pending patches: 
https://git.eclipse.org/r/39345 - bug 457198
https://git.eclipse.org/r/38802 - bug 404348
https://git.eclipse.org/r/38803 - this bug - major set of CPD related fixes
https://git.eclipse.org/r/39349 - bug 383569

Pending patch independent from the changes above:
https://git.eclipse.org/r/39352 - bug 448873

Please review & test!
Comment 19 Andrey Loskutov CLA 2015-01-13 15:25:19 EST
(In reply to Andrey Loskutov from comment #18)
> OK, this last issue is now fixed (part of the fix for bug 383569)

Did I already said "last issue fixed"? Seems that I've missed another one, thanks to new patch from Noopur Gupta bug 383569 is *really* fixed now.

Pending patches: 
https://git.eclipse.org/r/38802 - bug 404348 (action sets persistence)
https://git.eclipse.org/r/38803 - this bug - major set of CPD related fixes
https://git.eclipse.org/r/39349 - bug 383569 (reset toolbars)
https://git.eclipse.org/r/39470 - bug 383569 (reset action sets)
 
Pending patch independent from the changes above:
https://git.eclipse.org/r/39352 - bug 448873 (reset view actions)

Please review & test!
Comment 20 Paul Webster CLA 2015-01-14 16:13:04 EST
(In reply to Andrey Loskutov from comment #19)
> Pending patches: 
> https://git.eclipse.org/r/38802 - bug 404348 (action sets persistence)

Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=d94ad70154d8c9dc22c67cbec3b6c505ab563b0f.  Thanks Andrey.

> https://git.eclipse.org/r/38803 - this bug - major set of CPD related fixes

I think this is pretty close, I still had one comment in the review.  But with this patch, I get an extra failure in the UITestSuites:


UI Test Suite
org.eclipse.ui.tests.UiTestSuite
org.eclipse.ui.tests.api.ApiTestSuite
org.eclipse.ui.tests.api.IAggregateWorkingSetTest
testWorkingSetSaveRestoreAggregates(org.eclipse.ui.tests.api.IAggregateWorkingSetTest)
junit.framework.AssertionFailedError: B has lost data in the process of save/restore
	at junit.framework.Assert.fail(Assert.java:57)
	at junit.framework.TestCase.fail(TestCase.java:227)
	at org.eclipse.ui.tests.api.IAggregateWorkingSetTest.testWorkingSetSaveRestoreAggregates(IAggregateWorkingSetTest.java:273)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:606)
	at junit.framework.TestCase.runTest(TestCase.java:176)
	at junit.framework.TestCase.runBare(TestCase.java:141)
	at junit.framework.TestResult$1.protect(TestResult.java:122)
	at junit.framework.TestResult.runProtected(TestResult.java:142)
	at junit.framework.TestResult.run(TestResult.java:125)
	at junit.framework.TestCase.run(TestCase.java:129)
	at junit.framework.TestSuite.runTest(TestSuite.java:255)
	at junit.framework.TestSuite.run(TestSuite.java:250)
	at junit.framework.TestSuite.runTest(TestSuite.java:255)
	at junit.framework.TestSuite.run(TestSuite.java:250)
	at junit.framework.TestSuite.runTest(TestSuite.java:255)
	at junit.framework.TestSuite.run(TestSuite.java:250)
	at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:131)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
	at org.eclipse.pde.internal.junit.runtime.RemotePluginTestRunner.main(RemotePluginTestRunner.java:62)
	at org.eclipse.pde.internal.junit.runtime.PlatformUITestHarness$1.run(PlatformUITestHarness.java:47)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:136)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3793)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3431)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1151)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:337)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1032)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:156)
	at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:648)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:337)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:592)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:138)
	at org.eclipse.pde.internal.junit.runtime.NonUIThreadTestApplication.runApp(NonUIThreadTestApplication.java:54)
	at org.eclipse.pde.internal.junit.runtime.UITestApplication.runApp(UITestApplication.java:47)
	at org.eclipse.pde.internal.junit.runtime.NonUIThreadTestApplication.start(NonUIThreadTestApplication.java:48)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:380)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:235)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:606)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:648)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:603)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1465)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1438)
Comment 21 Andrey Loskutov CLA 2015-01-14 17:39:41 EST
(In reply to Paul Webster from comment #20)
> > https://git.eclipse.org/r/38803 - this bug - major set of CPD related fixes
> 
> I think this is pretty close, I still had one comment in the review.  But
> with this patch, I get an extra failure in the UITestSuites:
> 
> 
> UI Test Suite
> org.eclipse.ui.tests.UiTestSuite
> org.eclipse.ui.tests.api.ApiTestSuite
> org.eclipse.ui.tests.api.IAggregateWorkingSetTest
> testWorkingSetSaveRestoreAggregates(org.eclipse.ui.tests.api.
> IAggregateWorkingSetTest)
> junit.framework.AssertionFailedError: B has lost data in the process of
> save/restore

I can't reproduce, at least starting IAggregateWorkingSetTest from Eclipse doesn't show any failures. The test itself is unrelated to the changes in this patch, so may be some sporadic issue?
Comment 22 Paul Webster CLA 2015-01-14 20:43:00 EST
(In reply to Andrey Loskutov from comment #21)
> 
> I can't reproduce, at least starting IAggregateWorkingSetTest from Eclipse
> doesn't show any failures. The test itself is unrelated to the changes in
> this patch, so may be some sporadic issue?

It always fails for me (linux 64 bit), with the patch. But I'm running the whole UITestSuite.  Does it fail for you when you do that?

PW
Comment 23 Andrey Loskutov CLA 2015-01-15 15:31:08 EST
(In reply to Paul Webster from comment #22)
> (In reply to Andrey Loskutov from comment #21)
> > 
> > I can't reproduce, at least starting IAggregateWorkingSetTest from Eclipse
> > doesn't show any failures. The test itself is unrelated to the changes in
> > this patch, so may be some sporadic issue?
> 
> It always fails for me (linux 64 bit), with the patch. But I'm running the
> whole UITestSuite.  Does it fail for you when you do that?
> 
> PW

Yep.
This is caused by WorkbenchPage.resetPerspective() called in the IPageServiceTest before.

This one line is responsible for the failure: 
oldPersp.updateActionBars();

This was added as first attempt to fix reset perspective (bug 383569 comment 17) but another fix line below (after 383569 comment 23) seem to be sufficient:
legacyWindow.updateActionSets();

I will fix&rebase the patch and push it ASAP.
Comment 24 Nick Lodewijks CLA 2015-01-19 08:17:41 EST
The target for this issue is set to version 4.5. I was wondering if this is still valid? Meaning that it will be fixed in Eclipse Mars (~June 2015)?

- Nick Lodewijks
Comment 25 Andrey Loskutov CLA 2015-01-21 17:09:13 EST
(In reply to Nick Lodewijks from comment #24)
> The target for this issue is set to version 4.5. I was wondering if this is
> still valid? Meaning that it will be fixed in Eclipse Mars (~June 2015)?

@Nick: the fixes are there, but they need to be reviewed and approved. The lack of the experienced committers from platform ui (or they time) is the main issue here.

@Paul: is there any chance that you can continue the review of pending patches soon, or delegate this to one of your colleagues? I would prefer to see the code in M5 (and if this will cause any regression to kick it out in M6) as to wait until M6 and hear "now it's too late for such big change".
Comment 26 Paul Webster CLA 2015-01-22 09:09:52 EST
(In reply to Andrey Loskutov from comment #25)
> 
> @Paul: is there any chance that you can continue the review of pending
> patches soon, or delegate this to one of your colleagues? I would prefer to
> see the code in M5 (and if this will cause any regression to kick it out in
> M6) as to wait until M6 and hear "now it's too late for such big change".

I think that review I commented on is pretty close.  I just had the one question.  If we can get that in tomorrow morning, what reviews should we look at?

PW
Comment 27 Andrey Loskutov CLA 2015-01-22 13:11:35 EST
(In reply to Paul Webster from comment #26)
> I think that review I commented on is pretty close.  I just had the one
> question.  If we can get that in tomorrow morning, what reviews should we
> look at?
Perfect. I've answered on your last review question on the first review below.
Reviews to make the CPD story nearly complete:
https://git.eclipse.org/r/38803 - this bug - major set of CPD related fixes
https://git.eclipse.org/r/39349 - bug 383569 (reset toolbars)
https://git.eclipse.org/r/39470 - bug 383569 (reset action sets)
(independent from the changes above):
https://git.eclipse.org/r/39352 - bug 448873 (reset view actions)
Comment 28 Paul Webster CLA 2015-01-23 11:29:59 EST
(In reply to Andrey Loskutov from comment #27)
> Reviews to make the CPD story nearly complete:
> https://git.eclipse.org/r/38803 - this bug - major set of CPD related fixes

I've opened https://dev.eclipse.org/ipzilla/show_bug.cgi?id=9197 for this CQ.

PW
Comment 29 Andrey Loskutov CLA 2015-01-23 17:25:18 EST
(In reply to Paul Webster from comment #28)
> (In reply to Andrey Loskutov from comment #27)
> > Reviews to make the CPD story nearly complete:
> > https://git.eclipse.org/r/38803 - this bug - major set of CPD related fixes
> 
> I've opened https://dev.eclipse.org/ipzilla/show_bug.cgi?id=9197 for this CQ.

Is this CQ required (or is there some tooling which shows "CQ required" status)? The patch 38803 changes less than 1000 lines (+535, -296), so it should not need the CQ IMHO.

Beside this, thanks for the review, I've just uploaded fixed patches for all pending reviews.
Comment 30 Paul Webster CLA 2015-01-23 18:50:33 EST
(In reply to Andrey Loskutov from comment #29)
> Is this CQ required (or is there some tooling which shows "CQ required"
> status)? The patch 38803 changes less than 1000 lines (+535, -296), so it
> should not need the CQ IMHO.
> 

You're correct, I miscounted the lines.  Given that, I've withdrawn the CQ.

PW
Comment 31 Paul Webster CLA 2015-01-23 20:05:18 EST
(In reply to Andrey Loskutov from comment #27)
> https://git.eclipse.org/r/38803 - this bug - major set of CPD related fixes
> https://git.eclipse.org/r/39349 - bug 383569 (reset toolbars)
> https://git.eclipse.org/r/39470 - bug 383569 (reset action sets)
> https://git.eclipse.org/r/39352 - bug 448873 (reset view actions)

I've released the changes.  Thanks a lot Andrey.

I've done some testing, and it seems to work for me.  But there is still a state I can get into after doing a couple of things (hide a toolbar command, hide an action set, reset, hide a toolbar command, etc) where the actionSets no longer accept a change in state.  It's happened twice but my attempts to peg down the specific sequence of steps with restarts and resets are unsuccessful.  I'll keep my eye out, and I suggest others do as well.

Great work Andrey.

PW
Comment 32 Klaus Klaus CLA 2015-01-31 13:15:28 EST
I tested 4.5M5 (Windows 8.1) in the meantime and the changes which where made in the "Customize Perspective Dialog" looking fine to me. 

For testing I created my own Perspective and saved it (Windows -> Perspective -> Save Perspective As...)  under a new name. Then I exported the setting of my new Perspective and tried to import it into a other workspace, but this does not import the new Perspective! :-(

What i did:
File -> Export... -> General -> Preferences -> Export all.

This generates a preference file and this file should include the settings of my newly created Perspective - but it does not :-(
The Perspective is not included in the settings file.

This works in older Eclipse versions (at least in 3.x Version) like a charm.
Comment 33 Dani Megert CLA 2015-02-06 03:38:19 EST
(In reply to Klaus Mr from comment #32)
> For testing I created my own Perspective and saved it (Windows ->
> Perspective -> Save Perspective As...)  under a new name. Then I exported
> the setting of my new Perspective and tried to import it into a other
> workspace, but this does not import the new Perspective! :-(
> 
> What i did:
> File -> Export... -> General -> Preferences -> Export all.
> 
> This generates a preference file and this file should include the settings
> of my newly created Perspective - but it does not :-(
> The Perspective is not included in the settings file.
> 
> This works in older Eclipse versions (at least in 3.x Version) like a charm.

This is bug 378811.
Comment 34 Andrey Loskutov CLA 2015-02-16 16:43:25 EST
@Platform UI committers: there are still reviews pending which should fix many of the remaining bugs blocking this umbrella bug. Most of this patches are pretty small (except https://git.eclipse.org/r/40449), so it shouldn't be a big deal to quickly review them to avoid duplicated bugs entered, like bug 460039).

https://git.eclipse.org/r/40448
https://git.eclipse.org/r/40449
https://git.eclipse.org/r/41383
https://git.eclipse.org/r/41385
https://git.eclipse.org/r/41386
https://git.eclipse.org/r/41387
https://git.eclipse.org/r/41411
https://git.eclipse.org/r/41922

Can we cleanup the pending reviews please?
Comment 35 Petr Bodnar CLA 2015-02-20 13:47:09 EST
I tested 4.5M5 and even the simplest scenario of perspectives affecting each other still does not work: select only "New Wizards" for the "Java EE" perspective, select only "Save" for the "Debug" perspective -> only the "Save" button gets visible in both perspectives. This doesn't seem like related to any of the pending gerrit reviews... And finding other basic errors after another 5 minutes. This is really funny...
Comment 36 Andrey Loskutov CLA 2015-02-20 14:12:06 EST
(In reply to Petr Bodnar from comment #35)
> I tested 4.5M5 and even the simplest scenario of perspectives affecting each
> other still does not work: select only "New Wizards" for the "Java EE"
> perspective, select only "Save" for the "Debug" perspective -> only the
> "Save" button gets visible in both perspectives. This doesn't seem like
> related to any of the pending gerrit reviews... And finding other basic
> errors after another 5 minutes. This is really funny...

This is most likely https://git.eclipse.org/41378/ (bug 395601 and bug 404009) and it was merged *after* M5. You have to try with latest nightly build, please verify if your issue is fixed there.

Please also try to discuss your findings on existing bugs, or, if you find new issues, report those and link to this bug.
Comment 37 Petr Bodnar CLA 2015-02-21 03:39:49 EST
(In reply to Andrey Loskutov from comment #36)

OK, sorry for the false report. I does work in the nightly build. Plus I have filed a new bug 460503 - hopefully it won't be another duplicate...
Comment 39 Eclipse Genie CLA 2015-04-15 04:15:43 EDT
New Gerrit change created: https://git.eclipse.org/r/45846
Comment 41 Andrey Loskutov CLA 2015-04-22 15:52:03 EDT
I'm removing the dependency to bug 394386 since this is not related to the "customization" of the perspectives, and seems more part management/SWT related, see bug 394386 comment 31.

With this, the list of *known* *open* issues related to the perspective customization on 4.x seem to be empty and I would like to close this umbrella bug.

For all other new (or newly discovered) perspective customization issues please open dedicated bugs.
Comment 42 Lars Vogel CLA 2015-04-22 15:55:22 EDT
Congratulations and big thanks to Andrey for fixing most of these bugs.
Comment 43 Dani Megert CLA 2015-04-23 05:08:31 EDT
(In reply to Lars Vogel from comment #42)
> Congratulations and big thanks to Andrey for fixing most of these bugs.

+1!
Comment 44 Andrey Loskutov CLA 2015-11-11 03:18:19 EST
*** Bug 384349 has been marked as a duplicate of this bug. ***