Bug 84808 - (PatchAttached) Read project name from .project file in CVS checkout wizard
Summary: (PatchAttached) Read project name from .project file in CVS checkout wizard
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: CVS (show other bugs)
Version: 3.0.1   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 3.2 M2   Edit
Assignee: platform-cvs-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted
: 87633 (view as bug list)
Depends on: 88535
Blocks:
  Show dependency tree
 
Reported: 2005-02-09 13:27 EST by Keith Fetterman CLA
Modified: 2005-09-20 10:36 EDT (History)
4 users (show)

See Also:


Attachments
A first level of patch: work in progress, and widly buggy (12.72 KB, patch)
2005-07-21 14:19 EDT, Philippe Ombredanne CLA
no flags Details | Diff
Patch 1 or 2 for team.cvs.core (4.20 KB, patch)
2005-08-03 17:15 EDT, Philippe Ombredanne CLA
no flags Details | Diff
Patch 2 or 2 for team.cvs.ui (33.97 KB, patch)
2005-08-03 17:15 EDT, Philippe Ombredanne CLA
no flags Details | Diff
Updated patch (42.44 KB, patch)
2005-08-12 13:41 EDT, Philippe Ombredanne CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Fetterman CLA 2005-02-09 13:27:40 EST
I have some Eclipse Java projects in CVS.  The projects have a different project
name in the .project file than the module name in CVS.  

I would like a feature added when checking out a project in CVS in Eclipse,
where the checkout wizard will look into the .project file to get the project
name and use this name as the project name instead of the cvs module name.  The
actual directory name that the project is checked into could be the cvs module
name.  That would be ideal.

We have multiple projects in cvs that depend on each other.  The dependencies in
the .project or in the .classpath files are setup using the project name in the
.project file.  It would be nice that when checking out this name is used again.
Comment 1 Michael Valenta CLA 2005-02-11 11:49:45 EST
It's a good idea but we do no have enough manpower to address this issue. 
Patches will be accepted.
Comment 2 Michael Valenta CLA 2005-03-10 09:17:32 EST
*** Bug 87633 has been marked as a duplicate of this bug. ***
Comment 3 Aaron Luchko CLA 2005-03-18 20:24:16 EST
Bug 88535 should greatly simplify this correct?
Comment 4 Michael Valenta CLA 2005-03-20 16:34:38 EST
Bug 88535 is a necesary prerequisite.
Comment 5 Philippe Ombredanne CLA 2005-07-14 17:45:26 EDT
BTW, I am working on patch for this one.
It bugs me too much.
Comment 6 Philippe Ombredanne CLA 2005-07-21 14:19:30 EDT
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!
Comment 7 Philippe Ombredanne CLA 2005-08-03 17:15:23 EDT
Created attachment 25640 [details]
Patch 1 or 2  for team.cvs.core
Comment 8 Philippe Ombredanne CLA 2005-08-03 17:15:46 EDT
Created attachment 25641 [details]
Patch 2 or 2  for team.cvs.ui
Comment 9 Philippe Ombredanne CLA 2005-08-03 17:20:40 EDT
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.
Comment 10 Michael Valenta CLA 2005-08-04 08:58:57 EDT
Thanks for the patch.
Comment 11 Aaron Luchko CLA 2005-08-04 16:49:05 EDT
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.
Comment 12 Michael Valenta CLA 2005-08-04 17:31:32 EDT
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.
Comment 13 Philippe Ombredanne CLA 2005-08-04 18:16:01 EDT
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!
Comment 14 Michael Valenta CLA 2005-08-05 09:35:14 EDT
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.
Comment 15 Philippe Ombredanne CLA 2005-08-12 13:41:17 EDT
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!
Comment 16 Michael Valenta CLA 2005-08-15 11:32:53 EDT
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;-)
Comment 17 Bogdan Gheorghe CLA 2005-09-20 10:36:52 EDT
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)