Community
Participate
Working Groups
Build ID: I20070917-0010 Since we've been having a little discussion[1] on how the Project set import dialog matching rule should work I am reporting a separate bug for it. Consider a following case: Repository from a project set file: :extssh:joe@dev.eclipse.org:/cvsroot/eclipse Known repositories: :pserver:localhost:/cvsroot/test :pserver:ann@dev.eclipse.org:/cvsroot/eclipse At this moment the Project set import dialog will build a combo-box for the extssh entry from the project set with values in the following order: :extssh:joe@dev.eclipse.org:/cvsroot/eclipse (selected, needs to be created) :pserver:ann@dev.eclipse.org:/cvsroot/eclipse (second position as it's compatible, even though the user differs) :pserver:localhost:/cvsroot/test (other repositories at the end) In addition to this, one can select an entry and click "Create" button which will open the "New Repository Location" dialog primed with the values from the selected entry. What Michael proposed is that the dialog should suggest a known and compatible location if such exists instead of the one from the project set. So, the combo-box would look like this: :extssh:joe@dev.eclipse.org:/cvsroot/eclipse (the one from the project set) :pserver:ann@dev.eclipse.org:/cvsroot/eclipse (select as it's compatible, only connection and user method differ) :pserver:localhost:/cvsroot/test (other repositories at the end) [1] bug 199108
Created attachment 78747 [details] Removing the "Show connection" button This patch has been copied from bug 199108 as showing the connection method issue is related to the best match strategy. The patch remove the button for showing/hiding the connection method in the left pane. Which doesn't mean that the connection method should be always hidden (or shown).
The last part of the best match strategy mentioned in comment 0 (except picking the perfect match or a compatible repo if such exists) is handling a situation when there is no known, compatible repository which match. Then (according to Michael's suggestion) we should match pick using the connection method. Starting from extssh, followed by pserver and finally ext. IMHO it's not the best solution. Having in mind that we are preselecting an entry from Project Set Import dialog combo-box I would rather see the repo from the project set selected. One can always select a different known repository (with any connection method) and use it to create a new connection. I would use the order of picking a repo based on the connection method to sort known repositories in the combo-box (both compatible and non-compatible). In other words, I do agree that we could pick a compatible repository if there is one (we could use Michael's order to decide which to pick if there is more then one available). I reckon that it's not the best idea to preselect a non-compatible repository.
I think you misunderstood what I said. We obviously should never automatically pick an incompatible repository location. I was referring to the case where there were multiple compatible repository locations but there was no match on the connection method.
Created attachment 78960 [details] The button removed + preselection This patch removes the "Show connection method" button (the connection is now always visible). Also, I've added a simple default selection mechanism which works as follows (actually this how Michael described to me the best match strategy): 1) if the connection method in the project set matches the location, pick it 2) Otherwise, if there is one compatible location, pick it 3) Otherwise, match pick the extssh method if it exists, followed by pserver and finally ext To achieve that I sort compatible connections as stated in 3). Then in a final array I check whether an entry at position 1 (at 0 there is always the location from the project) is a compatible with the one from project (if it's incompatible it means that there is no compatible locations at all). If yes I pick it, if no I pick the repository location at position 0. I will update the CVSProjectSetImportTest class with some additional tests with a separate patch. The patch will be attached to bug 196800.
1. CVSProjectSetCapability#isMatching() became public so it should have its javadoc. 2. I would replace strings in CVSProjectSetCapability#isAdditionalRepositoryInformationRequired ("pserver", "extssh") with static refs defined in ICVSConstants (probably doesn't exist yet), but we can raise another bug for that issue
Created attachment 79203 [details] Patch v3 Patch #2 modified as suggested in the previous comment. Instead of creating a new class I've created private constant fields for "extssh", "pserver" and "ext" connection method names. Also, I've removed the "pserverssh2" as Michael didn't mention about it when talking about the sorting order. At the end, I've removed some unnecessary collection sorts like for matchingList which contains only repository locations with identical connection methods.
Created attachment 79204 [details] mylyn/context/zip
My case is: 1) I have a team project set using two locations :extssh:dev.eclipse.org:/cvsroot/eclipse and :pserver:dev.eclipse.org:/cvsroot/eclipse 2) In the dialog I have :extssh:dev.eclipse.org:/cvsroot/eclipse -> :extssh:dev.eclipse.org:/cvsroot/eclipse :pserver:dev.eclipse.org:/cvsroot/eclipse -> :pserver:dev.eclipse.org:/cvsroot/eclipse 3) I create a new location :extssh:dev.eclipse.org:/cvsroot/eclipse using the "Create Location" button 4) Nothing changes in the table and as I understand I should have :extssh:dev.eclipse.org:/cvsroot/eclipse -> :extssh:dev.eclipse.org:/cvsroot/eclipse :pserver:dev.eclipse.org:/cvsroot/eclipse -> :extssh:dev.eclipse.org:/cvsroot/eclipse
I discussed the comment #8 with Tomek. We can't set a newly created location as a suggested location. Using my case from the comment #8, someone could create :extssh location but wanted to use :pserver anyway. So finally I think that we can leave it as it is now, but... I would like to see what are statuses of locations in the combo, e.g. existing, just created, non-existing. We can use decorators, but AFAIK there is a bug related to decorators in tables that blocks fixing it. Tomek will elaborate more on it.
Yep, that's the point. This is exactly how I see it: once a new location has been created using the dialog the only thing I would do is picking it in the selected row and adding (not picking) it to rest of rows (if compatible). Referring to the second part of Szymon comment I filed bug 205167 (Indicate status of a suggested repository location...) and set it depended on bug 162497 (need a ControlDecoration for table rows).
The following scenario failed for me: 1) Have a project set with three projects from 3 different repositories. All the connection methods are extssh. 2) Start an eclipse with an empty workspace. 3) Define an extssh repository location that matches one of the project set repos directly. Define a pserver location that matches another. 4) Perform a project set import. The table I was presented with did not contain the direct match (I guess since it didn't need use riintervention), properly mapped the one to pserver and contained the other as the default extssh location without a username. 4) I clicked finish and was prompted for the username and password of the direct match. It looks like the table detects the direct match but the checkout itself doesn't find it.
See the bug 205167 for the issue described in the comment #9.
Created attachment 79765 [details] Patch v4 My fault, sorry. Here is the patch for it plus additional fixes for updating selection when switching the "Show compatible" option on and off. I decided to add newly created repository location (made with "Create location" button) at the end of lists/combo-boxes. Other options could be to add them at the beginning or sort lists/combo-boxes every time new location is added. I checked the dialog on Linux and it looked fine. Szymon could you review the patch and check for any mean cases I perhaps did not cover?
The recent version looks fine. But of course the bug 205167 should be fixed to have the dialog looked perfect.
Patch released to HEAD
Verified in I20071029-1800 and by a code inspection.
