| Summary: | [Import/Export] Create working sets when importing several projects. | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Michael Illgner <fillg1> | ||||||||||
| Component: | IDE | Assignee: | Prakash Rangaraj <prakash> | ||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||
| Severity: | enhancement | ||||||||||||
| Priority: | P5 | CC: | eclipse, georg.sendt, prakash, pwebster | ||||||||||
| Version: | 3.2 | Keywords: | helpwanted | ||||||||||
| Target Milestone: | 3.5 M6 | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Windows XP | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Michael Illgner
Created attachment 119488 [details]
Patch 01
Patch: Added the working set group in the import wizard.
Created attachment 119489 [details]
Screenshot
Can we have this for M4? Created attachment 119491 [details] Patch v02 Initial selection is also taken care of. This fixes Bug# 248241 Hi Kim, Could I get you to check out Patch v02? Thanx, PW *** Bug 206441 has been marked as a duplicate of this bug. *** (In reply to comment #5) > Hi Kim, > > Could I get you to check out Patch v02? > > Thanx, > PW > This is problematic. It contains hard coded references to the java working set. How about this. Instead of having the working set control on the first page, we add a second page. when the user goes to it we look at all projects they want to import and ask each type of known working set whether or not they can be accepted (via the IWorkingSet.adaptElements() method). For any type that returns some value we use that to populate the candidate list for the configuration block. Provided we can get at the project info prior to importing this should work. I dont know what we could do otherwise - the hardcoded list of working set types isn't going to fly. The bone-stupid option would be to allow it to be added to ANY working set, whether legitimate or not... (In reply to comment #7) > How about this. Instead of having the working set control on the first page, > we add a second page. when the user goes to it we look at all projects they > want to import and ask each type of known working set whether or not they can > be accepted (via the IWorkingSet.adaptElements() method). For any type that > returns some value we use that to populate the candidate list for the > configuration block. > > Provided we can get at the project info prior to importing this should work. I > dont know what we could do otherwise - the hardcoded list of working set types > isn't going to fly. The bone-stupid option would be to allow it to be added to > ANY working set, whether legitimate or not... Even if we create a second page only for the workingset control, we don't have any IProjects yet - just the IProjectDescription (which are not adaptable) IProjects will be created only when the Finish is pressed. So it might not be possible. I went thru how the New Project wizard (BasicNewProjectResourceWizard) and the Import Team Set(ImportProjectSetMainPage)wizard works. They seem to hard code the working set ids. Team Set wizard hard codes both Resource and Java ids and New Project Wizard hard codes just the Resource Id (but it can be overridden). I like the New Project Wizard way. (In reply to comment #8) Kim, Any thoughts on how to proceed? (In reply to comment #9) > Kim, > Any thoughts on how to proceed? > After discussion on the call, it was decided that there are currently no APIs to decide the working set types that can hold instances of IProject. I have raised a bug (Bug# 262933) for that. When the API is created, we will raise a bug to remove the hard coded ids in the New Project wizard, Import Team Set wizard and in this wizard as well. Can we push this patch for now? (In reply to comment #10) > > Can we push this patch for now? Yes, mark this for M6. Do you still want us to review patch v02? PW (In reply to comment #11) > Yes, mark this for M6. Do you still want us to review patch v02? Though the changes are trivial, would be better if reviewed (In reply to comment #4) > Created an attachment (id=119491) [details] > Patch v02 This patch removes "public WizardProjectsImportPage()". Unless there is a reason to do so, we should probably put it back. Is this something we can write a test for? (import and then check to see a working set was generated)? PW (In reply to comment #13) > This patch removes "public WizardProjectsImportPage()". Unless there is a > reason to do so, we should probably put it back. Its not been used and the class is internal. > Is this something we can write a test for? (import and then check to see a > working set was generated)? We just add the org.eclipse.ui.dialogs.WorkingSetGroup dialog and use it. Its that class, which provides the functionality of creating/using the working set and adding the projects. If we were to write a test, that would be more of testing that class, which I'm pretty sure done already elsewhere. (In reply to comment #14) > (In reply to comment #13) > > This patch removes "public WizardProjectsImportPage()". Unless there is a > > reason to do so, we should probably put it back. > > Its not been used and the class is internal. We don't often remove public methods/constructors, even from internal classes, unless they weren't supposed to be there in the first place. I'd hesitate on the side of caution with a public constructor and simply leave it there. So there's already a test that does an import from archive and then finds a named working set that contains those projects? I'm not dead set on this test if it requires a lot of fiddling with the internal dialog model to fill in fields. If that's the case, then this is probably ready to go. PW (In reply to comment #15) > So there's already a test that does an import from archive and then finds a > named working set that contains those projects? I'm not dead set on this test > if it requires a lot of fiddling with the internal dialog model to fill in > fields. If that's the case, then this is probably ready to go. I just need a "go" on this (or a test) PW Created attachment 127703 [details]
wizard patch v03
Modified so it would apply over the initialPath change.
PW
(In reply to comment #17) > Created an attachment (id=127703) [details] > wizard patch v03 Released to HEAD >20090305 PW (In reply to comment #16) > (In reply to comment #15) > > So there's already a test that does an import from archive and then finds a > > named working set that contains those projects? I'm not dead set on this test > > if it requires a lot of fiddling with the internal dialog model to fill in > > fields. If that's the case, then this is probably ready to go. > > I just need a "go" on this (or a test) Sorry, I thought I'd update in yesterday's call and forgot. All I'm doing is to add the WorkingSetGroup to the UI. Its the job of that class to handle the initial selection, opening up the 'Select working sets' dialog, and identifying the working sets that the user has selected. When finish is pressed, we get the selected working sets and add the imported projects to them (just one API call to IWorkingSetManager) Initially I thought there would be some tests for the WorkingSetGroup (its an API), but neither me nor Hitesh could find any tests. So if I were to write a test, it should cover all the scenarios for that control (like handling of initial selection), which is a lot to do and also would be out of scope for this bug. Its more of testing that API than this specific bug. Would it be make sense to add tests for WorkingSetGroup separately? Verified in I20090310-0100 |