Community
Participate
Working Groups
build i0926-0935 We have a .psf file set up with our project list in the equinox-incubator. But if a person from the community (e.g. someone who is accessing the repository via a pserver anonymous connection) loads our releng project and then tries to import the project set, it fails. If you manually edit all the psf files and replace "extssh" with "pserver" it works. Can we catch this error situation and prompt the user for which repository connection they would like to use to perform the checkout? Our code is in: repo: dev.eclipse.org/eclipse project: /equinox-incubator/org.eclipse.equinox.incubator.releng
We could bring up a repo connection box if we detect that no repo entry exists for the server specified in the project set. Also note that we don't bother trying for a different connection type if numerous types exist for a server (ie. project set specifies extssh connection, we have a pserver connection in our workspace -> don't bother trying pserver connection).
*** Bug 177292 has been marked as a duplicate of this bug. ***
Created attachment 60968 [details] My dialog proposition This is how in my opinion a dialog for alternative repository selection should look like. As you can see it's similar to "Add Resources" dialog which appears when you want to commit some unknown new files to a repository. I've already done a logic which is hidden in the dialog: checking for alternative, known repositories, extension point for the dialog. Just wanted to know your opinion before moving on, so I won't make a bad turn.
I think a table isn't the best way to present this. How about a page that has a description a the top that includes the location from the project set. You could then have a list that showed the known repositories that match (i.e. same host and repo path). The user should have the option to create a new location and the new location page should be used and pre-populated with the information from the project set. In essence, this would be very similar to the first two pages of the checkout wizard.
I haven't pointed that out in my previous post, but I was thinking that an alternative repository propositions will be presented as a drop-down list. There would be also "Create a new location" entry on that list, we can even make it default. If I'm still not making myself clear enough I can prepare another fake screen-shot in a second.
PS. Moreover, my dialog will work for multiple unknown repositories from a project set. It will prevent from opening the same dialog many times.
OK, I think I see what you are saying. If there are multiple repositories in the project set, you would want to have an entry for each one and be able to specify the repository to use for each. This prevents you from needing to prompt for each unique repository in the project set. And you would have a button that opened another dialog to create a new entry which would then be added to the list for any project set repository location that matched. I think that sounds reasonable.
Created attachment 61539 [details] First, rough version of the patch Let me show what I've been up to for last 2 days (excluding Test Day of course). This is the very first version of the patch file, however after applying you should be able to see the dialog I was talking about. This could be a better way to start a discussion than sending faked screenshots. To make the dialog show up you will need to import a project set which has different connection method that one from your Repositories list (host and repository path should be the same). The dialog doesn't change the way project set importing takes place. There are just some system.outs to show a selection one have made after the dialog is closed. Moreover, after yesterday's manual tests I bumped into another alternative repository selection dialog which already exist in Eclipse. I though I can combine these two and reuse that code. Screenshots will follow. There is also Mylar's Context attached.
Created attachment 61540 [details] mylar/context/zip
Created attachment 61541 [details] The dialog from CVS properties This is the dialog I mentioned in comment 8.
Created attachment 61542 [details] Screenshot with two dialogs working together The screenshot shows how I see these two dialogs working together. After clicking on [...] button on my dialog "Select a Repository" dialog shows up, when selection is made, dialog is closed. Selected repository is then displayed in the second column, next to the repository from a project set.
From the screen shots, the what you have looks good but we need to add the ability to create a new repository location as well. I tried the patch and my first task was to load a project set into a new workspace. In this scenario, I would still expect to be prompted to create an alternate repository location (i.e. I still may need to convert an extssh to a pserver or vica versa and it would also be helpful if I could provide a user name that would become a fixed part of the repository location. On the technical side of things, I think we can get away with adding a method to the IUserAuthenticator interface for prompting which saves us from needing to define another extension point.
Created attachment 61808 [details] Advanced configuration of an alternative repository Michael, I hope this screen shot is closer to your vision of the alternative repository configuration. Let me quickly describe it. First of all, the "Import Project Set" dialog from the screen shot will be displayed every time user imports a project sets. This can be changed by ticking a checkbox there. The second dialog pops up when user clicks [...] button. I belive that three radio buttons there are self-explanatory. Worth mentioning is the fact that fields from "Configure new repository" will be populated with values from "CVS Repository from the change set" or "'Compatible' repository" respectively. I was also thinking about adding a button to clear all these fields. Doesn't the new dialog look too complex to you?
That's a good starting point. Here's my comments: 1) I don't think we need the option to skip the dialog. Project Set imports happen infrequently enough that we can justify prompting for the repository information every time. 2) The use of the ... button is not really a standard Platform practice. Also, the location selection dialog is a bit busy. Instead, perhaps we could use a drop down list in the table to allow the user to select an existing repository location. If not matching locations exist, we can prime the field with the location from the project set. 3) To deal with repository location creation, we could add a Create Location button to the dialog that would launch the Create Location wizard. If an entry is selected when we launch the wizard, we could prime the fields with the values from the Project set. When the user has created the location, we could insert it into the location entry of the selected row. 4) Finally, I don't think we need the option to use the location from the project set. I think users only do this now because they have no option. Given the choice, they would rather use a location that has a fixed user name. How does that sound?
Created attachment 64314 [details] Second (still rough) version of the patch Instead of making another screen shot I've decided to create this patch. Although it still needs some polishing I belive it can be the answer for your suggestions Michael. List of things that need to be done: * I would like to have a drop down list for a combo-box made of compatible CVS locations + rest (in that order) * To decide which repository locations are alternative I will take a method used in "Change Sharing..." dialog from a project's properties * As you suggested in comment 12 I will need to use IUserAuthenticator's extension point (instead of creating second one) * I've removed final modifier removed for LoadInfo.repositoryLocation... I was also thinking about creating a new copy of LoadInfo based on an existing one, and then replacing them. I'm not sure which solution is better, less invasive * I would like to add validation of the newly configured, alternative CVS repository (as in NewLocationWizard) * I will need to work on columns width in the table * When a user decide not to overwrite a project from a project set I will need to hide it's repository location from the table (don't need to ask for an alternative repository if we won't import the project) Michael, if you could tell me if I'm on a good track...
Created attachment 64315 [details] mylar/context/zip
I think we are going in the right direction but I have a few comments: 1) It appears that only repository locations for which a matching location can be found are shown in the table. It is more likely that there are no repositories defined when the user imports so we need to handle that case. By default, you could file the right column with the location as it would appear if the user didn't change it. As a test for this, create a working set and then start Eclipse on a new workspace and import the working set. When I tried this I got the old prompt. If there are projects from multiple repositories in the project set and I have one of them defined, only the projects for the define repository are imported. 2) Perhaps we should use a TitleAreaDialog and add a description that explains to the user what is going on (i.e. the project set contains partial repository information and we need to get the rest of the information. 3) Canceling the dialog doesn't cancel the operation. 4) The table columns didn't take up all the available space but should. 5) I find the table rows very narrow. Can we add margins to the top and bottom of each row? 6) When creating the location, I had the validate checkbox checked but my location wasn't validated (i.e. I could enter an invalid repository).
Created attachment 64457 [details] Patch v3 Here it is. Michael, the patch is addressing all your last comments and my todos from comment 15.
Here are some comments: 1) For me, if extssh was in the project set, the New Location Wizard will say pserver. The repository and repository path are there but the connection method isn't. 2) It would be nice if the text on the New Location Wizard said Create instead of Finish. 3) You removed the code that made the repository known. The result of this is that, if the user supplies a password but doesn't save it, they will get prompted later in the operation for the password again. The good news is that you are most of the way there. The bad news is that I think the change is a bit too big to make at this point. I would like to put it aside for now and we can revisit it at the beginning of 3.4. That will provide plenty of opportunity to get feedback from users.
Created attachment 73350 [details] Patch v4 Michael, if you could take a look at the patch once again. Here are my comments/questions to the fourth version of it: 1) Can team project set file contain any passwords? If yes, should I use them to prime the dialog when creating a new location? 2) In order to change the Finish button label to "Create" I made finishButton field protected (was private). See org.eclipse.jface.wizard.WizardDialog. I'm not sure if it's acceptable. 3) What do you think about the dialog's initial height? Don't you think it's to small? Should I implement storing/loading of the dialog's size? 4) I was also thinking about using ControlDecoration to indicate whether suggested alternative repository is an existing one, needs to be created or it's a perfect match. I've found two bugs related to the topic: bug 162497 and 162770. How to you find the idea? 5) The wizard and the dialog are missing context help. I will be happy to hear your comments.
In general, I think the mapping works well but we still need some refinements to the patch. First, here are my answers to your questions: 1) The project set doesn't contain passwords 2) We can't change API in JFace. I think you can do what you want by overriding the createButton method and looking for the FINISH id. 3) Yes, the dialog is initially too small. We should set the initial size a bit bigger and use the API from Dialog to remember the users size settings (see Dialog#getDialogBoundsSettings) 4) I'm not sure if this would help. You may try it out if you like but I think it can wait until my points below are addressed. 5) Add the context constants and log a bug to provide the doc (although you can add the TOC entries now if you like) Here are some comments about the patch: A) I think the dialog needs to be explained well. It is not obvious from looking at the table what the purpose of the dialog is. I think users will be confused when they are first presented with the dialog. I think we either need to expand the description or add a label that says something like: The project set only contains partial repository information. Please use the table below to associate each project set repository with a repository location or click OK to use the default locations. It would also be good if we could convey somehow that, if the user clicks OK, the old behavior still occurs. B) In keeping with point A, I think we could exclude the connection method from the left column but leave the default in the right column the same (i.e. show the connection method). The column titles could be "Project set repository information" and "Repository Location". C) In the constructor of AlternativeRepositoryDialog, you make use of an inner class to convert a map to the entries for the table. I think a better approach is this: - make fAlternatives an instance variable of AlternativeRepositoryTable - create the instance of AlternativeRepositoryTable in the AlternativeRepositoryDialog constructor. The AlternativeRepositoryTable would take a map and translate it to the internal representation. - add a createControl(Composite) to AlternativeRepositoryTable that you call from AlternativeRepositoryDialog#createDialogArea I think this results in better encapsulation. D) Why does IUserAuthenticator#promptForAlternativeRepository return an Object. It should return a Map. Also, why does it take a Map as a parameter. Shouldn't it just take the list of repositories from the project set? E) I don't like the use of "Alternative" everywhere. I understand why you used it but I would prefer a different prefix. For instance, in the CVSProjectSetCapability I think that the isAlternativeRepositoryNeeded method could be changed to isAdditionRepositoryInformationRequired and the promptForAlternativeRespository could be changed to promptForAdditionRepositoryInformation. I think that's it for now.
Created attachment 73754 [details] Patch v5 Here is the fifth version of the patch. I'm pretty sure it's not a final one, but I hope I'm getting closer to a solution which will satisfy both of us :) Once again, here are my concerns/things to-do: 1) I removed final modifier for CVSProjectSetCapa$LoadInfo.location in order to be able replace it after the Alternative Repo dialog is closed. Is this acceptable? I can create a copy (clone) of project load object before replacing repository location. 2) I subclassed ConfigurationWizardMainPage and change its validation method a little bit. Now, when user tries to create an existing location an error message shows up saying that he/she should close the dialog and select the entry with that location in the combo box. The other solution is to allow create that location, instead of displaying an error (silently handle an error). I could select the existing location after the dialog is closed without creating a new location. 3) Answering to the second part of bullet D - the map is created in isAlternativeRepositoryNeeded. The fact that it's not empty (it contains repository locations from a project set mapped to a list of suggested known locations) is one of the factors which decide to prompt for an additional information (ask user to select an alternative repository from a list). Otherwise (the map is empty) we do not display the Alternative Repository dialog. Do you think I should postpone the creation of the map and check whether the dialog needs to display other way? 4) I will complete and polish my javadocs. 5) I will log a separate bug to extend the doc. 6) I will log a separate bug to provide some tests.
I think we're getting closer but I'm finding it hard to keep track of what changes from patch to patch. To simplify things, I've released the patch to branch "bug_159258". That way we can incrementally improve things more easily. You should load the branch and provide patches from that. Here are my latest comments - In regards to the use of a Map, it just seems odd to accept a map as a parameter and return one as well. However, I don;t view this as a big issue so I'll leave it to you to decide what to do. - I don't really like making instance variables directly accessible to subclasses. In the particular case of the NewLocationWizard, you should be able to get around it by creating a protected createMainPage method which you can override in the subclass. You can then call super.addPages() from the subclass add pages to avoid the duplication of the setting of the properties. - I still don't think that "alternative" is the proper term to use everywhere. How about the following renames: AlternativeRepositoryDialog -> ConfigureRepositoryLocationsDialog (or ConfigureProjectSetRepositoriesDialog) AlternativeRepositoryTable -> ConfigureReposiotryLocationsTable P.S. I didn't include the doc project in the branch but I did include the other 3 plug-ins.
Created attachment 74451 [details] Patch v6 The patch is meant to be applied to the "bug_159258" branch. It addresses your previous comments only. The patch got so big because I renamed two classes.
I've released the latest patch to the branch (and fixed a compile error due to a change in a non-branched project). Szymon can you load the branch (bug_159258) and have a look at the new feature. The branch projects are org.eclipse.team.cvs.core, org.eclipse.team.cvs.ui and org.eclipse.team.tests.cvs.core.
My first impression 1) the initial width is too big 2) we can add "Show only compatible repository location" checkbox like it is in Project Properties>CVS>Change sharing 3) Why do we use a dialog instead of a wizard page?
Moreover there is a problem with the "Create location" button and the check box. During resizing of the dialog they disappear and we have a white space instead of the widgets. Michael saw it too. Tomek, can you observe similar behavior?
(In reply to comment #26) > 1) the initial width is too big No problem, I will adjust it. > 2) we can add "Show only compatible repository location" checkbox like it > is in Project Properties>CVS>Change sharing Personally, I am not convinced of your idea. But I've got a different proposition: I can add named separators between each group of repo locations in a combobox. Similar to "--- Workspace matches ---" in Open Resource dialog. E.g. ------from project set---- :extssh@dev.eclipse.org:/cvsroot/eclipse (default selection) ----------compatible------ :pserver:anonymous@dev.eclipse.org:/cvsroot/eclipse ---------others----------- :extssh:tzarna@9.158.141.245:/cvsroot/eclipse :pserver:dev.eclipse.org:/cvsroot/eclipse > 3) Why do we use a dialog instead of a wizard page? I was thinking about using a wizard here, but my idea was to create a wizard for all operations which need to be taken place during a Team Project Set file (i.e. select a file, confirm overwrite, configure repo locations and so on). What do you think about that? If you agree with me I think we should rather report separate bug for it. (In reply to comment #27) > Moreover there is a problem with the "Create location" button and the check > box. During resizing of the dialog they disappear and we have a white space > instead of the widgets. Michael saw it too. Tomek, can you observe similar > behavior? Yep, I saw it too, but I thought it's an SWT issue. Do you think it's my fault?
(In reply to comment #26) Answering bullet 2): As you can see there are many approaches to make the dialog more intuitive. I'm not saying that I don't agree with, I just think maybe it will be better to open a separate bug for this issue. What do you think? > 3) Why do we use a dialog instead of a wizard page? I've logged bug 198258 to address this. (In reply to comment #27) > Moreover there is a problem with the "Create location" button and the check > box. During resizing of the dialog they disappear and we have a white space > instead of the widgets. Michael saw it too. Tomek, can you observe similar > behavior? As I mentioned in comment 28 I did saw it too, but since my reply I haven't been able to reproduce it. I don't it can be a reason but just to make sure, what version are you using Szymon? I tried it on N20070714-0010, N20070723-0010 and N20070729-0010. It looks fine on all of them.
Created attachment 74929 [details] Patch for default dialog's size I set the dialog's default size to 300px height and 600px width.
I've released your patch, merged the branch with HEAD and released the changes to HEAD. Also, I modified some of the wording in the dialog.
Verified in build I20070807-0010 but logged bug 199108 and bug 199110.
