Bug 83240 - [Import/Export] Importing Archive Files and File System as configured projects
Summary: [Import/Export] Importing Archive Files and File System as configured projects
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: IDE (show other bugs)
Version: 3.2   Edit
Hardware: PC All
: P5 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Platform UI Triaged CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted
Depends on: 82988
Blocks:
  Show dependency tree
 
Reported: 2005-01-19 15:34 EST by Aaron Luchko CLA
Modified: 2019-09-06 16:16 EDT (History)
3 users (show)

See Also:


Attachments
sample patch (10.41 KB, patch)
2005-02-22 00:59 EST, Aaron Luchko CLA
no flags Details | Diff
completed patch (19.03 KB, patch)
2005-12-29 01:58 EST, Aaron Luchko CLA
no flags Details | Diff
updated patch (20.78 KB, patch)
2006-05-02 16:00 EDT, Aaron Luchko CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aaron Luchko CLA 2005-01-19 15:34:27 EST
Importing from Archive File and from File System should include options for
importing as configured projects (similar to CVS).  I've already done so for RPM
plug-in here, bug 83135, this bug is a branch off of bug 82988 where some of the
functionality has been moved to another wizard.
Comment 1 Aaron Luchko CLA 2005-02-22 00:59:33 EST
Created attachment 18162 [details]
sample patch

This is just a preview patch.  I only did Archive files in this patch for
simplicities sake.

There are two issues that need to be adressed.	The UI issue(s) is the "Into
Folder" box should be grayed out when the import as configured check box is
selected, the problem with this is it requires changing the corresponding
variable(s) in WizardResourceImportPage to protected rather than private (I
already had to make validateDestinationGroup() non-final), can we do these
minor (and non-breaking) api changes?  The other option is to basically inline
WizardResourceImportPage or go with a flawed UI but these are both undesirable
for obvious reasons.

The second issue is the way the project is imported.  Classically when a
project is put in an archive it is in the form <project_folder>/<project_files>
and the default behavior of the import is to import it as
<selected_folder>/<project_folder>/<project_files> (where <selected_folder> is
the folder the user has selected or created for the import), this is not very
useful however as your project now contains an extra directory you probably
didn't intend.	Now the import as configured behavior in CVS Import will import
as <selected_folder>/<project_files> leaving out <project_folder> entirely
which works quite well when importing a project.  Since changing import level
behavior for import as configured is obviously a bad idea from a usability
standpoint I suggest an additional checkbox "Strip Leading Directory" or
something equally unintuitive ;) where the user will not import the leading
directory in an archive file (or a folder).  For example something that would
of been imported as <selected_folder>/<project_folder>/<project_files> will now
be <selected_folder>/<project_files>, this option of course would only be
enabled when there is a single outer folder available.	The ability to not
import the outer directory in an import while not currently in present is in my
patch for bug 82988.
Comment 2 Aaron Luchko CLA 2005-03-16 05:44:28 EST
Changing to depending on 82988 since it adds some functionality that this patch
needs
Comment 3 Aaron Luchko CLA 2005-12-29 01:58:26 EST
Created attachment 32313 [details]
completed patch

I've made a new (seems to be finished) version of the patch which adds a checkbox to "Import as a configured project" to the Archive File and File System import dialogs.

As well I needed to change some variables in WizardResourceImportPage from private to protected and I removed the determinPageCompletion() method from there since it only did a simple check to see if no projects were open (which is no longer correct when importing into a configured project).

Also I made a minor change to createOptionsGroup to eliminate some code duplication however I didn't propogate this to WizardZipFileResourceImportPage since that seems to no longer be in use (should the source file be removed entirely?).
Comment 4 Aaron Luchko CLA 2006-02-01 02:13:55 EST
I was wondering if it would be possible for this patch to go in for M4 before the API freeze?

It has some minor changes to
extensions/org/eclipse/ui/dialogs/WizardResourceImportPage.java

These include removing two methods (which are inherited from parent classes), changing two variables from private to protected, and removing a "final" modifier from another method

These are fairly minor API changes but I'm worried they cause the patch to miss 3.2 entirely if they don't get in for m4.

thank you

BTW. I've run through org.eclipse.ui.tests.datatransfer JUnit tests and didn't get any errors from this patch
Comment 5 Karice McIntyre CLA 2006-02-01 11:12:23 EST
I will see what I can do, we are quite swamped with M5 work we've committed to.  I marked it M5 so it doesn't get lost in the shuffle.  Should there be any JUnit tests added to the data transfer test suite to go along with this?
Comment 6 Aaron Luchko CLA 2006-02-01 15:17:12 EST
Thanks for looking into this. I took a real quick look at doing some JUnit tests when I wrote the patch but wasn't sure how to handle the interactive part of the New Project Wizard. I'll take another look at them if I get some time but unfortunately I'm taking a full course load and am very swamped as well and doubt I would get a chance within the M5 timeframe.
Comment 7 Karice McIntyre CLA 2006-02-05 19:37:53 EST
After a cursory look at the patch I have a few questions/concerns:

1. I am not sure why the method in WizardResourceImportPage was originally marked final (there is always a reason for doing this), so I am not sure what is the impact of changing it to be non-final.  Seems like the intent was not to be able to override this method.  I will have to check with the resident API experts and former component owners to see if this is ok.
2. I don't think you can remove the determinePageCompletion() method from WizardResourceImportPage.  This would change the behavior of this method if anyone has overridden this method in a subclass.  An error message should show if there are no projects in the workspace.  Subclasses will no longer get this behavior for free and they should.
3. What does import as configured project mean to an average user?  I had to go look at the cvs import wizard to get an idea of what this means.  If I didn't get it right away (and I am not a new user) I am not convinced the average (and esp the new) user will understand what this means if they don't know about the CVS option that is capable of doing this.  The user could get very confused if they choose this option not knowing what it means - they may think they have lost their project configuration data.  Perhaps the option could be re-phrased to be more understandable?
4. I think you should be overriding createOptionsGroupButtons(), not createOptionsGroup.  Now that there are 2 checkboxes in the options area, they should be contained in the options group box.  I think the createBaseOptionsGroup() method was there to override the group that is created in createOptions() group because there was only one option, so having a group for one checkbox would be overkill.  Now that there are multiple options, they should be in a group.  Need to change WizardFileSystemResourceImportPage1 and WizardArchiveFileResourceImportPage1 to reflect this.
5. In WizardFileSystemResourceImportPage1.importResources(), if a new project can't be created false is returned but no error message is reported.  
6. Instead of assigning the contianerPath at the beginning of WizardFileSystemResourceImportPage1.importResources(), you should override the getContainerPath() method so that it returns super.getContainerPath() if the checkbox is not checked, and the new code if it is checked.  That way, anyone else who calls getContainerPath() in this class (or any subclasses) will always get the right answer.
7. I noticed that in the CVS import wizard, the configured project option only works if the .project file is not present.  This doesn't seem to be the case in this patch.  What if the project in the archive file (or file system) has a .project file present?  We will just override this without telling them?  Also, all of the new project wizard choices don't seem to make sense when the configured project option is chosen. (e.g. Project from CVS).  
8. I didn't see anything in the Eclipse guidelines about this but I thought the platform tried to avoid having a wizard pop up from within a wizard.  I have seen examples of this in other Eclipse based products but not in the SDK.  I will check with the resident UI guideline folks to see what is the accepted practice in this situation.  I know you can do chaining of wizards somehow but  I haven't actually had to implement a wizard like this yet - maybe that is the answer.
Comment 8 Karice McIntyre CLA 2006-02-06 17:48:55 EST
Aaron, I should mention that what you are providing to the user is useful and a good idea.  But it might be better designed/implemented in a broader manner as a full plan item in the next release.  Tod had some good initial thoughts on this - maybe you would like to mention them here, Tod?
Comment 9 Tod Creasey CLA 2006-02-07 07:59:13 EST
With all of the options we have now (import from file, tar and archive as files or projects) it is getting pretty hard to figure out where the wizard you want is.

Assuming we get some time at some point I think we need to amalgamate these. Aaron work here should also be included.

The main issue with the configured project import is the idea of two wizards. No one designs thier wizards with the expectation that it could be opened nested within another one - I think this would break a lot of the new wizards out there in subtle ways.

What would be good is two ideas

1) Optionally create a simple project on import 
2) Provide a facility to transform a project into another one - we can't just use the new wizard as they are not designed for this. This would be another extension of some kind.
Comment 10 Karice McIntyre CLA 2006-02-08 10:06:17 EST
Changing target - this will not be fixed for 3.2
Comment 11 Aaron Luchko CLA 2006-05-02 16:00:49 EDT
Created attachment 40110 [details]
updated patch

Karice, thanks for the analysis, sorry it took so long to get back but I've just finished school and should have a bit more time to work on this now.

I updated the patch so it applies cleanly to cvs head, as well as answered your questions and made a few of the changes you suggested. I'll submit a more thoroughly tested patch when I solve the rest of the problems. 

1.
I'm not sure why it's marked final either. I'll see if there's another way to accomplish what I need.

2.
Good point, I'll look for another way to get around this.

3. 
Good point, I changed it to use the same description as the cvs import wizard.

4.
Yeah, the main idea with createBaseOptionsGroup was because while it was fine replicating code for just the one button I didn't want to be replicating it for both buttons. 

I've changed the method that is overwritten to createOptionsGroupButtons().

5.
I've changed it to throw up an error though the message could probably be made clearer.

6.
Good idea, though I changed it to getDestinationPath() since overriding getContainerFullPath() causes problems since getContainerFullPath() is called by validitateDestinationGroup().

7.
The current functionality when importing into an existing project is a prompt asking if you want to overwrite the .project file. The patch in its current state will create the new project, ask it you want to overwrite the .project, and if you cancel it will go back to the wizard without doing the import but the new project will still be there.
I can think of a couple ways to keep this kind of functionality. The first is to delete the new project if the user cancels during the copy. This solution has the advantage of also applying to files such as the .classpath as well. The other solution is to prompt the user before they start the creation of the new project though you're still left with the prospect of forcing them to re-affirm that decision again during the actual copy or overwriting the .project, as well as the .classpath or other project files during the copy.

As to the extra options like new project from CVS I agree it's not ideal though I don't think it's a huge problem as how they make the new project shouldn't cause a problem since the new project is just standing in for an existing project.

8.
I like the idea of chaining wizards here, particularly as it solves #5. Though from Tod's comments it sounds like this may be problematic.
Comment 12 Susan McCourt CLA 2009-07-15 12:11:34 EDT
"As per http://wiki.eclipse.org/Platform_UI/Bug_Triage_Change_2009"
Comment 13 Eclipse Webmaster CLA 2019-09-06 16:16:39 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.