| Summary: | Provide extension point for EGit CloneSourceProvider | ||||||
|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Sascha Scholz <sascha.scholz> | ||||
| Component: | Mylyn | Assignee: | Sascha Scholz <sascha.scholz> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | enhancement | ||||||
| Priority: | P3 | CC: | steffen.pingel | ||||
| Version: | unspecified | Keywords: | noteworthy | ||||
| Target Milestone: | 0.9 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | 370646 | ||||||
| Bug Blocks: | |||||||
| Attachments: |
|
||||||
|
Description
Sascha Scholz
I submitted a patchset for review: http://review.mylyn.org/#change,115 Merged 1ce9caed3b11e2a6d4832c83ce0fe423183f951f I ran into this exception when I tried it on Windows: java.lang.NullPointerException at org.eclipse.mylyn.internal.gerrit.ui.egit.GerritRepositorySearchPageContentProvider.hasChildren(GerritRepositorySearchPageContentProvider.java:49) at org.eclipse.jface.viewers.AbstractTreeViewer.isExpandable(AbstractTreeViewer.java:2127) at org.eclipse.jface.viewers.TreeViewer.isExpandable(TreeViewer.java:588) at org.eclipse.jface.viewers.AbstractTreeViewer.isExpandable(AbstractTreeViewer.java:2153) at org.eclipse.jface.viewers.AbstractTreeViewer.updatePlus(AbstractTreeViewer.java:2835) at org.eclipse.jface.viewers.TreeViewer.updatePlus(TreeViewer.java:852) at org.eclipse.jface.viewers.AbstractTreeViewer.createTreeItem(AbstractTreeViewer.java:834) at org.eclipse.jface.viewers.AbstractTreeViewer$1.run(AbstractTreeViewer.java:808) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70) at org.eclipse.jface.viewers.AbstractTreeViewer.createChildren(AbstractTreeViewer.java:782) at org.eclipse.jface.viewers.TreeViewer.createChildren(TreeViewer.java:644) at org.eclipse.jface.viewers.AbstractTreeViewer.createChildren(AbstractTreeViewer.java:753) at org.eclipse.jface.viewers.AbstractTreeViewer.internalInitializeTree(AbstractTreeViewer.java:1533) at org.eclipse.jface.viewers.TreeViewer.internalInitializeTree(TreeViewer.java:833) at org.eclipse.jface.viewers.AbstractTreeViewer$5.run(AbstractTreeViewer.java:1517) at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1443) at org.eclipse.jface.viewers.TreeViewer.preservingSelection(TreeViewer.java:403) at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1404) at org.eclipse.jface.viewers.AbstractTreeViewer.inputChanged(AbstractTreeViewer.java:1510) at org.eclipse.ui.dialogs.FilteredTree$NotifyingTreeViewer.inputChanged(FilteredTree.java:1187) at org.eclipse.jface.viewers.ContentViewer.setInput(ContentViewer.java:280) at org.eclipse.jface.viewers.StructuredViewer.setInput(StructuredViewer.java:1690) at org.eclipse.mylyn.internal.gerrit.ui.egit.GerritRepositorySearchPage.createControl(GerritRepositorySearchPage.java:93) at org.eclipse.jface.wizard.WizardDialog.updateForPage(WizardDialog.java:1247) at org.eclipse.jface.wizard.WizardDialog.access$4(WizardDialog.java:1239) at org.eclipse.jface.wizard.WizardDialog$8.run(WizardDialog.java:1228) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70) at org.eclipse.jface.wizard.WizardDialog.showPage(WizardDialog.java:1226) at org.eclipse.jface.wizard.WizardDialog.nextPressed(WizardDialog.java:915) at org.eclipse.jface.wizard.WizardDialog.buttonPressed(WizardDialog.java:428) at org.eclipse.jface.dialogs.Dialog$2.widgetSelected(Dialog.java:624) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:240) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4163) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3752) at org.eclipse.jface.window.Window.runEventLoop(Window.java:825) at org.eclipse.jface.window.Window.open(Window.java:801) at org.eclipse.ui.internal.handlers.WizardHandler$Import.executeHandler(WizardHandler.java:150) at org.eclipse.ui.internal.handlers.WizardHandler.execute(WizardHandler.java:277) at org.eclipse.ui.internal.handlers.HandlerProxy.execute(HandlerProxy.java:293) at org.eclipse.core.commands.Command.executeWithChecks(Command.java:476) at org.eclipse.core.commands.ParameterizedCommand.executeWithChecks(ParameterizedCommand.java:508) at org.eclipse.ui.internal.handlers.HandlerService.executeCommand(HandlerService.java:169) at org.eclipse.ui.internal.handlers.SlaveHandlerService.executeCommand(SlaveHandlerService.java:241) at org.eclipse.ui.internal.actions.CommandAction.runWithEvent(CommandAction.java:157) at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:584) at org.eclipse.jface.action.ActionContributionItem.access$2(ActionContributionItem.java:501) at org.eclipse.jface.action.ActionContributionItem$5.handleEvent(ActionContributionItem.java:411) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4163) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3752) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2696) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2660) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2494) at org.eclipse.ui.internal.Workbench$7.run(Workbench.java:674) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:667) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:123) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:344) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.lang.reflect.Method.invoke(Unknown Source) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:622) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:577) at org.eclipse.equinox.launcher.Main.run(Main.java:1410) This is probably due to old configuration data that doesn't have project information. We should be more tolerant.
For unknown reason I'm currently not able to push, but the fix should be to change getProjects in GerritConfiguration to
public List<Project> getProjects() {
if (projects != null) {
return projects;
} else {
return Collections.emptyList();
}
}
Sascha
Pushed b04019c4125622f76a549bf7b963b65eebb247e1 that should fix the NPE. Could you please verify it, Steffen? I published another weekly build. It works now! I noticed that if I select the Eclipse.org Gerrit instance two identical https:// URIs are listed for each repository. Should we filter the duplicates? I was also wondering if it makes sense to default the URI selection to https:// or git://. Those protocols often works for anonymous access whereas ssh:// produces quirky errors when authentication fails. Created attachment 211215 [details]
screenshot
> I published another weekly build. It works now! Great, thanks! > I noticed that if I select the Eclipse.org Gerrit instance two identical > https:// URIs are listed for each repository. Should we filter the duplicates? This could happen if you haven't been authenticated when you did the last refresh of the repository configuration. A solution would be to show only URLs for DownloadScheme.ANON_HTTP and DownloadScheme.ANON_GIT if not authenticated (as the Gerrit UI does). > I was also wondering if it makes sense to default the URI selection to https:// > or git://. Those protocols often works for anonymous access whereas ssh:// > produces quirky errors when authentication fails. For anonymous access this would be the case if the above proposal is implemented. For authenticated access I chose SSH to be on the top of the list because that's the most common protocol at SAP and my intention was to to push the Gerrit connector forward here. I don't use HTTPS for the SAP Gerrit either because I haven't managed to set up SSL certificates correctly... :-( Refreshing got indeed rid of the duplicate URLs. The username is now properly added. Makes sense to mirror the Gerrit web UI and I'm okay with leaving ssh as the default when authenticated. The errors that I encountered were just very strange but it's a separate problem that should be addressed in EGit. Pushed a fix for duplicate clone URIs: 721316a2d56a08a720bec451246def78a46b0d58 A few minor suggestions came up in the UI review on the last conference call: * The label of the "Add" button should be changed to "Add..." since it opens a dialog. * We should consider nesting repository nodes that have a "/" e.g. show an "egit" node with "egit", "egit-pde" etc. as children. On large servers such as android-review.googlesource.com the list or repositories gets very long otherwise. * When refreshing, the entire operation fails if any repository fails to refresh (e.g. due to a network error). That can make it impossible to refresh the list of repositories. * Gerrit servers that are marked as "Disconnected" should probably be filtered from the list. Thanks for the review! * The label of the "Add" button should be changed to "Add..." since it opens a dialog. Ok, I will have a look today. * We should consider nesting repository nodes that have a "/" e.g. show an "egit" node with "egit", "egit-pde" etc. as children. On large servers such as android-review.googlesource.com the list or repositories gets very long otherwise. A very good idea, but takes a bit more time. Probably, I won't have time for that before end of next week. * When refreshing, the entire operation fails if any repository fails to refresh (e.g. due to a network error). That can make it impossible to refresh the list of repositories. The button only refreshes the currently selected repository. Seems that we have to make it clearer in the UI. Any good ideas? * Gerrit servers that are marked as "Disconnected" should probably be filtered from the list. Ok, I will have a look today. (In reply to comment #12) > * When refreshing, the entire operation fails if any repository fails to > refresh (e.g. due to a network error). That can make it impossible to refresh > the list of repositories. > > The button only refreshes the currently selected repository. Seems that we have > to make it clearer in the UI. Any good ideas? Great. Then this is a probably a non issue. I'll try again, maybe I always had the failing repository selected and didn't realize it. (In reply to comment #13) > (In reply to comment #12) > > * When refreshing, the entire operation fails if any repository fails to > > refresh (e.g. due to a network error). That can make it impossible to refresh > > the list of repositories. > > > > The button only refreshes the currently selected repository. Seems that we > have > > to make it clearer in the UI. Any good ideas? One simple way to make it more obvious would be to show the label of the repository in the progress message. I think it only says "Refresh Configuration" now, if it said "Refreshing Mylyn Code Reviews" this would already help. Pushed 78204c1d3f35392269b0fd9dd0b90fa3f2540a92 and ed2e9617483a49139646424fc0291c8c7a82a435: - Label: "Add" -> "Add..." - Hide disconnected task repositories - show repository label during refresh Open: * We should consider nesting repository nodes that have a "/" e.g. show an "egit" node with "egit", "egit-pde" etc. as children. On large servers such as android-review.googlesource.com the list or repositories gets very long otherwise. Don't know when I'll have time to look into making the project list hierarchical. To much other work... :-( No worries, we can just file another bug and mark it as helpwanted to encourage someone from the community to pick it up. |