Bugzilla – Full Text Bug Listing
| Summary: | (PatchAttached) Read project name from .project file in CVS checkout wizard | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Keith Fetterman <kfetterman> | ||||||||||
| Component: | CVS | Assignee: | platform-cvs-inbox <platform-cvs-inbox> | ||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||
| Severity: | enhancement | ||||||||||||
| Priority: | P3 | CC: | aaron, francois, ian, pombredanne | ||||||||||
| Version: | 3.0.1 | Keywords: | helpwanted | ||||||||||
| Target Milestone: | 3.2 M2 | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Whiteboard: | |||||||||||||
| Bug Depends on: | 88535 | ||||||||||||
| Bug Blocks: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Keith Fetterman
It's a good idea but we do no have enough manpower to address this issue. Patches will be accepted. *** Bug 87633 has been marked as a duplicate of this bug. *** BTW, I am working on patch for this one. It bugs me too much. Created attachment 25145 [details]
A first level of patch: work in progress, and widly buggy
This is a first patch. Just a work in progress that works so well that it
introduces tons of UI quircks, and ignores completely what you type, just gets
the metafile project name, and creates the project with the .project project
name.
It does not work for multiple projects, and the place where the .project file
retrieval is done is all wrong.
There is :
*a new operation:
FetchProjectDescriptionOperation which returns an IProjectDescription (could
have been returning a file or stram, but I thought that was what was most
sensible). This I think is fine, unless we combine it with the
HasProjectMetaFileOperation and refactor that class to provide both a test for
existence and teh retrieval of the IProjectDescription.
*A few UI messages .
* a small change to CheckoutAsWizard to invoke a new CheckoutAsMainPage
constructor of CheckoutAsMainPage.
* temps changes to CheckoutAsMainPage to retrive the metafile for testing for
now. This is probably not the place to do that.
I think the invocation should take place in the CheckoutAsAction instead, like
for HasProjectMetaFileOperation.hasMetaFile and that the object passed to the
wizard should not be a ICVSRemoteFolder but some object ( a map ?) that maps
one ICVSRemoteFolder with a possible existing IProjectDescription project name.
So just food for thought for now, revised patches will come later.
And some unit test will not be bad too.
Just a first step that I wanted to share to put a stake in the ground.
:-)
Feedback welcomed!
Created attachment 25640 [details]
Patch 1 or 2 for team.cvs.core
Created attachment 25641 [details]
Patch 2 or 2 for team.cvs.ui
The submitted patches work fien now: - there is a new preferences flag on the Tem/CVS page, general tab to set if you want to use .project project name, or module name. If left unchecked (the default) then the behavior is the same as before. - the checkout, checkout as, and checkout wizard have all been ptached. - it works for one or multiple projects. - When no metafile is available, I fallback on the regular module name. This could surely be refinedd a bit on the UI in some occasions, but is very functional as is. And is based on the latest CVS HEAD ass of 2005-08-03 14:00 PST. For me this is pretty much fixed with this. Feedback is welcomed. Enjoy it. Thanks for the patch. I gave this patch a whirl and it worked great. At some point it might be worthwhile to move the option into the wizard itself so it can be done on a by import basis but that would be another patch. I took a quick glance at the patch and I want you to make a few changes before I release it. Basically, there is no need to modify the core plugin at all. You added a boolean flag to the Core plugin class but it is only refrenced by the UI and the UI can query the preference directly. You also modified the general RemoteFolder to add a project name. I think this is too specific a field to be added to the general RemoteFolder class. The CVS UI plugin is not constrainted to use the ICSVRemoteFolder interface. One possibility is to subclass RemoteFolder and clone the retrieved folder into it when a custom projetc name is required. Other than that, the patch looks good (although I haven't run it through any paces yet). Given that it has no impact when the preference is not set, I don't forsee any problems releasing it once the requested changes are made. Michael, Thanks for the comments! I did use mostly a moneky see, monkey do approach looking at what was being done in the plugin for other areas. I woudl prefer to limit the impact on other plugins and interfaces, as you suggest. You wrote: >You added a boolean flag to the Core plugin class but it is only >refrenced by the UI and the UI can query the preference directly. I will modify it along that line. Honesly I did like the idea to modify the core. >You also modified the general RemoteFolder to add a project name. >I think this is too specific a field to be added to the general >RemoteFolder class. The CVS UI plugin is not constrainted to use the >ICSVRemoteFolder interface. One possibility is to subclass RemoteFolder >and clone the retrieved folder into it when a custom >projetc name is required. Another un-released version of the patch used a similar approach. I will modify along that line. Would you make that subclass part of the UI plugin then? And that would replace checking for a boolean with an instanceof in that case, as there would be no need for the flag anymore. Being an instance of something like "RemoteProjectFolder" should guarantee that there is a valid (non-null, non-empty) project name that is different from the module name that can be used instead of the module name. >Other than that, the patch looks good Thanks! >(although I haven't run it through any >paces yet). Go kick the tires a bit. I was a bit at loss to run just a subset of the tests sucessfully. So I am not sure that regression wise it does not introduce weird things. But the way it is coded it should not. >Given that it has no impact when the preference is not set, I >don't forsee any problems releasing it once the requested changes are made. That was the intent with using a preference so that it would not change anything in the UI of wizards, and so that th default bahavior is not altered, regardless of checing out, checking out as, import/new from CVS for a single or multiple project at once. I am merging the NON-NLS commits in my local CVS copy, and will come back with something leaner quick! Yes, I think the sublcass of RemoteFolder should be in UI since it is only used by the checkout. If it turns out to have more usefulness in the future, we can move it. Created attachment 26058 [details]
Updated patch
This attached updated patch is of CVS HEAD as of this morning.
Everything is in cvs.ui now using a subclass of RemoteFolder
I have also added a comment in the header for contributors, as I seen done on
some other code committed on the same plugin. Feel free to remove it if you do
not like it.
The only that I do not like if the way the progress monitor is being displayed
when several projects metafiles are retrieved. The behavior is the same as what
is in HEAD for HasMetafileOperation, but the progress monitor works is almost
done after the metafile retrieval, and does not move much anymore after that
for other files. The ultimate refinement (imho not needed) would be to have a
progress monitor that shows something proportionnaly to the numberr of remote
folders to check.
Thanks for looking at it while it is fresh!
I have released the patch. I fixed up the progress monitoring a bit. There are still some issues but to fix them required more refactoring than I care to do right now. I also removed the CVSUIPlugin instance variable as it was not needed. Thanks again. Now what would be neat is if someone used this work to warn the user if the project doesn't have a .project file and even offered to run the New Project wizard (bug 73590 if anyone is interested;-) Verified in I20050920-0010: - Checked out modules normally, checked out modules using .project name - Checked out multiple modules normally, checked out multiple modules using .project name - checked out modules using .project name where .project file is deleted (falls back to using module name) |