rombert / ereviewboard

A mylyn-based Eclipse integration for Review Board
46 stars 31 forks source link

Unable to View Diff Comments #89

Closed RunnersReign closed 12 years ago

RunnersReign commented 12 years ago

Comments made on a diff file are not viewable when viewing the diff in eclipse.

Steps to Reproduce:

  1. Open A Review within Review Board (Either in the Web Site or in eReviewBoard)
  2. View the Diff for the review (Either in the Web Site or in eReviewBoard)
  3. Make a comment on any line for that diff. (Either in the Web Site or in eReviewBoard)
  4. Post changes. (Either in the Web Site or in eReviewBoard)
  5. Try to view the comment made using .

Expected: Should be able to see comment made within the diff.

Actual : Only shows a little marker where the comment should be. Unable to see the comment.

I can also provide a screen shot if required.

rombert commented 12 years ago

Thanks for detailed report. Can you confirm that you tried to hover over the marker and no text shows up? If it does not, do you have any relevant information in your error log?

RunnersReign commented 12 years ago

Yes, when I try to hover over the marker, no text is being displayed.

When I hover over the marker, the following error gets logged in eclipse.

null Error Mon Jan 16 15:37:20 CST 2012 Unhandled event loop exception

java.util.NoSuchElementException at java.util.Collections$EmptySet$1.next(Collections.java:2912) at org.eclipse.mylyn.internal.reviews.ui.editors.parts.AbstractCommentPart.createReadOnlyText(AbstractCommentPart.java:188) at org.eclipse.mylyn.internal.reviews.ui.editors.parts.AbstractCommentPart.createCommentArea(AbstractCommentPart.java:157) at org.eclipse.mylyn.internal.reviews.ui.editors.parts.AbstractCommentPart.createSectionContents(AbstractCommentPart.java:135) at org.eclipse.mylyn.internal.reviews.ui.editors.parts.CommentPart.createSectionContents(CommentPart.java:77) at org.eclipse.mylyn.internal.reviews.ui.editors.parts.ExpandablePart.createControl(ExpandablePart.java:111) at org.eclipse.mylyn.internal.reviews.ui.editors.parts.TopicPart.createControl(TopicPart.java:55) at org.eclipse.mylyn.internal.reviews.ui.annotations.CommentPopupDialog.setInput(CommentPopupDialog.java:181) at org.eclipse.mylyn.internal.reviews.ui.annotations.CommentInformationControl.setInput(CommentInformationControl.java:73) at org.eclipse.jface.text.AbstractInformationControlManager.internalShowInformationControl(AbstractInformationControlManager.java:1170) at org.eclipse.jface.text.AbstractInformationControlManager.presentInformation(AbstractInformationControlManager.java:1139) at org.eclipse.jface.text.AbstractHoverInformationControlManager.presentInformation(AbstractHoverInformationControlManager.java:902) at org.eclipse.jface.text.AbstractInformationControlManager.setInformation(AbstractInformationControlManager.java:418) at org.eclipse.jface.text.source.AnnotationBarHoverManager.computeInformation(AnnotationBarHoverManager.java:379) at org.eclipse.jface.text.AbstractInformationControlManager.doShowInformation(AbstractInformationControlManager.java:1120) at org.eclipse.jface.text.AbstractHoverInformationControlManager$MouseTracker.mouseHover(AbstractHoverInformationControlManager.java:519) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:201) 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:4066) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3657) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2640) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2604) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2438) at org.eclipse.ui.internal.Workbench$7.run(Workbench.java:671) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:664) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:115) 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:369) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:620) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:575) at org.eclipse.equinox.launcher.Main.run(Main.java:1408) at org.eclipse.equinox.launcher.Main.main(Main.java:1384)

rombert commented 12 years ago

I think this is a bug in an old version of Mylyn Reviews. Can you try updating to the latest stable version?

RunnersReign commented 12 years ago

When I go to the Mylyn Reviews update site in eclipse (http://download.eclipse.org/reviews/updates) the following is displayed install packages are displayed.

-Mylyn Reviews Integrations ----Mylyn Reviews Connector: Gerrit (Incubation) 0.8.0.v20110608-1400 -Mylyn Reviews SDK and Framework ----Mylyn Reviews (Incubation) 0.8.0.v20110608-1400 ----Mylyn Reviews SDK (Incubation) 0.8.0.v20110608-1400

I currently have the "Mylyn Reviews (Incubation) 0.8.0.v20110608-1400" installed in my eclipse. I believe that this version was installed while installing the eReviewBoard plugin. Is there a different version that I should have?

rombert commented 12 years ago

Hm, I know for sure that 0.8.4 was released, but I'm not sure where to pick it up from ( git tag is at http://git.eclipse.org/c/mylyn/org.eclipse.mylyn.reviews.git/tag/?id=v0.8.4 ) . You might want to ask on a Mylyn support forum.

Alternatively you can add http://download.eclipse.org/reviews/nightly/ as an update site. The nightly 0.9.0 builds should have all the fixes.

RunnersReign commented 12 years ago

I will give this a shot hopfully later tonight and let you know.

My guess is that I am using Eclipse version 3.6 which might only support an older version of Mylyn, which might be why I am only seeing the 8.0 version. Again, I am only speculating at this and will have to verify.

rombert commented 12 years ago

http://download.eclipse.org/mylyn/releases/latest/ should have Mylyn Reviews 0.8.4

RunnersReign commented 12 years ago

Updating Mylyn to the newest version resolved the issue.

I found another package that I had installed in eclipse had the older version. This caused mylyn never to update to the newer version. After uninstalling the older version of mylyn, I was able to install the newer version.

Thanks for your help.

rombert commented 12 years ago

Great! I'll close this report then.

RunnersReign commented 12 years ago

Interestingly enough, I have just recently grabbed the new version of eclipse and reinstalled eReviewBoard plugin within it. I have found that I have run into this issue again. I have found out that when installing the eReviewBoard plugin it is automatically pulling in the following version of Mylyn Reviews.

Mylyn Reviews (Incubation)  0.8.0.v20110608-1400    org.eclipse.mylyn.reviews.feature.feature.group

This version is the older version and causes the comments not to be viewed within eclipse. Is there any way to have the Update Site for eReviewBoard automatically retrieve the newer version of Mylyn Reviews so that people do not have to manually install the new version of it before installing eReviewBoard?

rombert commented 12 years ago

You're right, and that's a bit of a shame. I see at ftp://ftp.iasi.roedu.net/mirrors/eclipse.org/mylyn/drops/ that the 3.7.0 release will have Mylyn Reviews 0.9.0 included, so once that is live the problem will be gone and I'll be able to remove the reference to the old reviews update site.

In the mean time it would be great if you could test the connector with Mylyn 3.7.0 from ftp://ftp.iasi.roedu.net/mirrors/eclipse.org/mylyn/drops/3.7.0/v20120319-0200/ and let me know if everything works as expected.

RunnersReign commented 12 years ago

Sorry for the late reply, I finally got around to testing out the newer version. I have pulled Mylyn 3.7.0 from the link you provided and installed it within my eclipse. I then used the update site for eReviewBoard to install the eReviewBoard plugin.

Things seems to work with some miner issues.

First issue: When submitting a review from within eclipse using the plugin, two error now appear within my eclipse error log. The reviews are still created on the review board server with all of the information and the diff there, so I am unsure why these errors occurred or if they effect anything.

The first error was : Problems occurred when invoking code from plug-in: "org.eclipse.mylyn.tasks.ui". The second error was: Error creating task editor part: "org.review_board.ereviewboard.ui.editor.ReviewRequestEditorPage.parts.diff"

Both issues had the following stack trace:

java.lang.NoClassDefFoundError: org/eclipse/mylyn/internal/reviews/ui/operations/ReviewCompareEditorInput at java.lang.ClassLoader.defineClass1(Native Method) at java.lang.ClassLoader.defineClassCond(ClassLoader.java:632) at java.lang.ClassLoader.defineClass(ClassLoader.java:616) at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.defineClass(DefaultClassLoader.java:188) at org.eclipse.osgi.baseadaptor.loader.ClasspathManager.defineClass(ClasspathManager.java:601) at org.eclipse.osgi.baseadaptor.loader.ClasspathManager.findClassImpl(ClasspathManager.java:567) at org.eclipse.osgi.baseadaptor.loader.ClasspathManager.findLocalClassImpl(ClasspathManager.java:490) at org.eclipse.osgi.baseadaptor.loader.ClasspathManager.findLocalClass_LockClassLoader(ClasspathManager.java:478) at org.eclipse.osgi.baseadaptor.loader.ClasspathManager.findLocalClass(ClasspathManager.java:458) at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.findLocalClass(DefaultClassLoader.java:216) at org.eclipse.osgi.internal.loader.BundleLoader.findLocalClass(BundleLoader.java:400) at org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(BundleLoader.java:476) at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:429) at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:417) at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.loadClass(DefaultClassLoader.java:107) at java.lang.ClassLoader.loadClass(ClassLoader.java:248) at org.review_board.ereviewboard.ui.editor.ReviewboardDiffPart.createSubsection(ReviewboardDiffPart.java:151) at org.review_board.ereviewboard.ui.editor.ReviewboardDiffPart.createControl(ReviewboardDiffPart.java:113) at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.initializePart(AbstractTaskEditorPage.java:1308) at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.access$7(AbstractTaskEditorPage.java:1300) at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage$14.run(AbstractTaskEditorPage.java:857) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.createParts(AbstractTaskEditorPage.java:848) at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.createParts(AbstractTaskEditorPage.java:833) at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.createFormContentInternal(AbstractTaskEditorPage.java:720) at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.createFormContent(AbstractTaskEditorPage.java:665) at org.eclipse.ui.forms.editor.FormPage$1.run(FormPage.java:152) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70) at org.eclipse.ui.forms.editor.FormPage.createPartControl(FormPage.java:150) at org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage.createPartControl(AbstractTaskEditorPage.java:618) at org.eclipse.ui.forms.editor.FormEditor.pageChange(FormEditor.java:471) at org.eclipse.ui.part.MultiPageEditorPart.setActivePage(MultiPageEditorPart.java:1067) at org.eclipse.ui.forms.editor.FormEditor.setActivePage(FormEditor.java:603) at org.eclipse.ui.forms.editor.SharedHeaderFormEditor.setActivePage(SharedHeaderFormEditor.java:110) at org.eclipse.mylyn.tasks.ui.editors.TaskEditor.addPages(TaskEditor.java:414) at org.eclipse.ui.forms.editor.FormEditor.createPages(FormEditor.java:138) at org.eclipse.ui.forms.editor.SharedHeaderFormEditor.createPages(SharedHeaderFormEditor.java:98) at org.eclipse.mylyn.tasks.ui.editors.TaskEditor.createPages(TaskEditor.java:196) at org.eclipse.ui.part.MultiPageEditorPart.createPartControl(MultiPageEditorPart.java:348) at org.eclipse.ui.internal.EditorReference.createPartHelper(EditorReference.java:670) at org.eclipse.ui.internal.EditorReference.createPart(EditorReference.java:465) at org.eclipse.ui.internal.WorkbenchPartReference.getPart(WorkbenchPartReference.java:595) at org.eclipse.ui.internal.EditorReference.getEditor(EditorReference.java:289) at org.eclipse.ui.internal.WorkbenchPage.busyOpenEditorBatched(WorkbenchPage.java:2945) at org.eclipse.ui.internal.WorkbenchPage.busyOpenEditor(WorkbenchPage.java:2850) at org.eclipse.ui.internal.WorkbenchPage.access$11(WorkbenchPage.java:2842) at org.eclipse.ui.internal.WorkbenchPage$10.run(WorkbenchPage.java:2793) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70) at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:2789) at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:2773) at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:2756) at org.eclipse.mylyn.tasks.ui.TasksUiUtil.openEditor(TasksUiUtil.java:182) at org.eclipse.mylyn.internal.tasks.ui.util.TasksUiInternal.openTask(TasksUiInternal.java:917) at org.eclipse.mylyn.internal.tasks.ui.OpenRepositoryTaskJob$2.run(OpenRepositoryTaskJob.java:118) at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35) at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:135) at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4140) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3757) 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(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) 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) Caused by: java.lang.ClassNotFoundException: org.eclipse.mylyn.internal.reviews.ui.operations.ReviewCompareEditorInput at org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(BundleLoader.java:513) at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:429) at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:417) at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.loadClass(DefaultClassLoader.java:107) at java.lang.ClassLoader.loadClass(ClassLoader.java:248) ... 78 more

The other issue, which may not be related directly to eReviewBoard, is eclipse now randomly stops responding when it tries to build my workspace and/or synchronize with SVN. Now I have no proof that this is belongs to the eReviewBoard plugin other than the fact that before installing eReviewBoard and the newer version of Mylyn I did not experience this issue. There are no errors in the error log when eclipse does freeze. I am going to investigate this a bit more before I jump to conclusions, but I thought it was still worth mentioning.

RunnersReign commented 12 years ago

Seems like the two errors I have stated in the previous post are thrown when ever you try to view an existing Review Request.

It seems like the Diff sections is not created/shown when viewing the Request within Eclipse. I can supply a screen shot if you need me to to get a better idea what I mean.

Also, since Mylyn 3.7 is now available through the update site http://download.eclipse.org/mylyn/releases/latest , I have also tried to install eReviewBoard through that. I still receive the both errors. Though creating reviews still works.

rombert commented 12 years ago

I probably need to release a new version which is compatible with Mylyn 3.7+ . The 3.7 release shuffled some previously internal packages and I will adjust to match them.

rombert commented 12 years ago

The changes are more substantial than expected ; I'll have to defer this for a couple of days.

rombert commented 12 years ago

This should now be fixed in master. Please try an interim build with should be compatible with Mylyn 3.7

https://github.com/downloads/rombert/ereviewboard/ereviweboard-0.13.0-SNAPSHOT.zip

RunnersReign commented 12 years ago

I was able to install it and everything seems to be working great. Thanks for fixing the issues.

rombert commented 12 years ago

Thanks for confirming.