Bug 349224

Comment 1 Neil Hauge 2011-07-07 14:14:13 EDT

Additional background:

This hard dependency is causing very serious issues for users of Dali.  Perspectives and Views that use the Common Navigator are unable to be opened due to the hard dependency described in the initial comment (see bug 349108 and bug 351463 as examples).  The workaround is very undesirable for users, requiring the installation of additional, unwanted features.

Please consider fixing this for the Indigo SR1 release.  If it seems likely that this won't be fixed for SR1, please communicate this so we can work around this issue in Dali for SR1.

Comment 2 Francis Upton IV 2011-07-07 16:50:42 EDT

I will try to get to it, but if you could provide a patch that would make it much easier.

Comment 3 Neil Hauge 2011-07-07 16:58:52 EDT

(In reply to comment #2)
> I will try to get to it, but if you could provide a patch that would make it
> much easier.

Thanks.  We will certainly take a look at providing a patch.

Comment 4 Paul Fullbright 2011-07-11 14:36:42 EDT

Created attachment 199439 [details]
proposed patch

I went for the minimal patch changes.

private int findId(List, String) - (which seems to only be used for finding appears before id's in the list) now returns -1 if the descriptor does not exist, rather than throwing a RuntimException.  This is in keeping with the typical list index paradigm.

private void computeSequenceNumbers() - now checks if the index (returned from findId(..) is greater than or equal to zero before its normal checks when determining whether to alter descriptor order.


Tested within my workspace in a number of scenarios, but please feel free to test further.  Any feedback is appreciated.

Comment 5 Francis Upton IV 2011-08-07 13:25:37 EDT

> Tested within my workspace in a number of scenarios, but please feel free to
> test further.  Any feedback is appreciated.
This initially looks good, I will do some testing on it on Tuesday and will check it in then for the 3.7.1 RC1 build.

Thanks for your contribution.

Comment 6 Francis Upton IV 2011-08-07 13:26:30 EDT

Paul Webster, I will want to get your +1 on this on Tuesday sometime for 3.7.1, I think that's the protocol now that you are my boss

Comment 7 Paul Webster 2011-08-08 13:25:11 EDT

+1

comment:  Francis, when returning -1 do you want to add a log?  We used to throw an exception (which I'm sure showed up in the logs) so we definitely consider it a noteworthy problem, and the log won't kill anything else like the RuntimeException.

PW

Comment 8 Paul Webster 2011-08-08 13:36:34 EDT

Francis, if you want to go ahead with this list, could you post to platform-ui-dev?  Tuesday is probably the last day for fixes like this, http://www.eclipse.org/eclipse/development/plans/freeze_plan_3_7_1.php

PW

Comment 9 Francis Upton IV 2011-08-08 13:47:40 EDT

(In reply to comment #8)
> Francis, if you want to go ahead with this list, could you post to
> platform-ui-dev?  Tuesday is probably the last day for fixes like this,
> http://www.eclipse.org/eclipse/development/plans/freeze_plan_3_7_1.php
> 

I will add a warning. And get it done tomorrow as I'm out of the office today.

Comment 10 Francis Upton IV 2011-08-09 18:35:40 EDT

Released to 3.7 SR1 RC1 

http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R3_7_maintenance&id=58c749808fad1beb088539a58de7636b382b26ab

Comment 11 Francis Upton IV 2011-08-09 20:45:24 EDT

Released to R4_Development (Juno M2) 

http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_development&id=eeee11dffc7156b07f30d5d8783c67f67dbd4ac2

Thank you for your contribution Paul F.

Comment 12 Francis Upton IV 2011-08-09 20:46:05 EDT

Paul F. would you mind verifying that the fix is included in the 3.7.1 build that's happening tomorrow morning and that it works, so we can mark the bug verified?

Comment 13 Paul Fullbright 2011-08-09 23:27:43 EDT

Thank *you* guys for getting this in so quick.

I may have to wait for a WTP build on this in order to fully verify, but will do so once I have one.

Comment 14 Remy Suen 2011-08-10 11:19:24 EDT

(In reply to comment #11)
> Released to R4_Development (Juno M2)

Francis, since you pushed this to R4_development, I presume the same thing should go into R3_development, correct? That is, you don't plan to correct this problem differently between the maintenance and development streams?

Comment 15 Francis Upton IV 2011-08-10 11:21:50 EDT

(In reply to comment #14)
> (In reply to comment #11)
> > Released to R4_Development (Juno M2)
> 
> Francis, since you pushed this to R4_development, I presume the same thing
> should go into R3_development, correct? That is, you don't plan to correct this
> problem differently between the maintenance and development streams?
Oh, I understood things wrong then, I thought R3_development was used only for stuff related to o.e.ui.workbench or something like that.

Did I read this wrong?

http://wiki.eclipse.org/Platform_UI/Git

Comment 16 Remy Suen 2011-08-10 11:36:40 EDT

(In reply to comment #15)
> I thought R3_development was used only for
> stuff related to o.e.ui.workbench or something like that.
> 
> Did I read this wrong?
> 
> http://wiki.eclipse.org/Platform_UI/Git

I guess you are referring to the "All Juno related work, both 4.2 and 3.8 that doesn't include the org.eclipse.ui.workbench. For now this also includes 4.1.1 work." line. It certainly gives the impression that you do not have to take any further action. However, this is not how I understand the build is setup right now.

<paulweb515_> rcjsuen: it needs to be cherrypicked into R3_development

This was also said on IRC. :)

Comment 17 Paul Fullbright 2011-08-15 18:09:05 EDT

Looks good to me.  See no problems (other than warning to log.)

Verified in 
Version: 3.7.1
Build id: M20110810-0800

Comment 18 Paul Webster 2012-01-09 07:38:54 EST

Released to R3_development

PW