Perl::Critic integration

jploski wrote on Sat Oct 28 19:15:44 MEST 2006:

Jae: we can discuss the Perl::Critic integration here. I had a look at your
checked in code and also committed some minor fixes myself. Here are some
outstanding issues which I did not fix (I do realize that it is work in
progress...):

1) The markers do not appear on the margin like the warning markers. I think
they should.
2) The Critique action runs on the UI thread. However, it can take several
seconds to complete (see my favorite example Twig.pm in org.epic.perleditor-test),
so it should run as a job (preferably cancelable).
3) The Clear Critic Markers action is in the context menu, but not in the
Source menu.
4) The Clear Critic Markers action does not work (the Problems view is not
emptied as expected).

jploski wrote on Sat Oct 28 19:17:01 MEST 2006:

I am also confused by the references to Perl::Critic in epicPodChecker.pl.
I thought Pod::Checker and Perl::Critic were two different things?

jploski wrote on Sat Oct 28 19:19:17 MEST 2006:

Also, the book references produced by Perl::Critic should not be discarded
by EPIC (they should appear either in the message text or more discreetly
in a property of the problem marker).

jgangemi wrote on Sun Oct 29 17:58:16 CET 2006:

the podchecker and critic are two seperate things, that was a fat finger
pasting mistake. 

the book references are there, but aren't currently being displayed. that
will be next on the list after the background thread change.

i'm not sure why things aren't working correctly for you - i'm wondering
if there is some difference between 3.1.x and 3.2 that is causing the problem.
did you try starting w/ the '-clean' option just to be sure?

i do not get an error that you committed a fix for - there actually isn't
supposed to be a "command" for the clear markers action b/c there is no
keyboard shortcut for it. also - the definitionId for the clear markers
action has to be the name of the marker owner b/c that is used to clear
the markers. the idea was to have a generic action that could clear any
type of marker, with an eventual "clear all" action planned. changing that
value would definately cause clearing of the markers not to work. 

i'm quite confused by this odd behavior nonetheless. i think i'll post in
the other forum to see if others are seeing the same issues. 

jploski wrote on Sun Oct 29 18:23:42 CET 2006:

Ok, it does not work for me because I broke it. The action's definitionId
turns out to be needed by your implementation...

I think that the 'owner' attribute is unnecessary - why not just use the
marker type (see references to org.epic.core.Constants.PROBLEM_MARKER to
see what I mean)? The definitionId attribute of an action should be used
to associate it with a command and not for other purposes.

jgangemi wrote on Sun Oct 29 18:44:53 CET 2006:

yeah - that is probably the better idea - it would make adding a "clear
all" action much easier as well. 

upon further investigation, it seems that if i change the clear marker action
to be a "command", then i can specify a command parameter and pass the marker
type that way. 

jploski wrote on Sat Nov  4 18:14:37 CET 2006:

FYI, I just released the current version of your Perl::Critic integration
code in 0.5.18.

jploski wrote on Sun Nov 12 20:31:44 CET 2006:

I'm still getting these "command not found" errors in the log with the current
code. The error appears upon the first opening of a Perl editor:

The command ("org.epic.perleditor.commands.clearMarker") is undefined

java.lang.Exception
	at org.eclipse.jface.action.ExternalActionManager$CommandCallback.isActive(ExternalActionManager.java:300)
	at org.eclipse.jface.action.ActionContributionItem.isCommandActive(ActionContributionItem.java:579)
	at org.eclipse.jface.action.ActionContributionItem.isVisible(ActionContributionItem.java:631)
	at org.eclipse.ui.internal.PluginActionContributionItem.isVisible(PluginActionContributionItem.java:146)
	at org.eclipse.jface.action.SubContributionItem.isVisible(SubContributionItem.java:131)
	at org.eclipse.jface.action.MenuManager.update(MenuManager.java:571)
	at org.eclipse.jface.action.MenuManager.update(MenuManager.java:549)
	at org.eclipse.jface.action.MenuManager.fill(MenuManager.java:232)
	at org.eclipse.jface.action.SubContributionItem.fill(SubContributionItem.java:65)
	at org.eclipse.jface.action.MenuManager.update(MenuManager.java:627)
	at org.eclipse.jface.action.MenuManager.updateAll(MenuManager.java:721)
	at org.eclipse.ui.internal.WorkbenchWindow.updateActionBars(WorkbenchWindow.java:2485)
	at org.eclipse.ui.internal.WorkbenchWindow.largeUpdateEnd(WorkbenchWindow.java:2533)
	at org.eclipse.ui.internal.Workbench.largeUpdateEnd(Workbench.java:2429)
	at org.eclipse.ui.internal.WorkbenchPage.busyOpenEditor(WorkbenchPage.java:2298)
	at org.eclipse.ui.internal.WorkbenchPage.access$9(WorkbenchPage.java:2287)
	at org.eclipse.ui.internal.WorkbenchPage$9.run(WorkbenchPage.java:2273)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:69)
	at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:2268)
	at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:2249)
	at org.eclipse.ui.ide.IDE.openEditor(IDE.java:371)
	at org.eclipse.ui.ide.IDE.openEditor(IDE.java:334)
	at org.eclipse.ui.ide.IDE.openEditor(IDE.java:751)
	at org.eclipse.ui.views.markers.internal.ActionOpenMarker.run(ActionOpenMarker.java:76)
	at org.eclipse.ui.views.markers.internal.MarkerView.handleOpenEvent(MarkerView.java:544)
	at org.eclipse.ui.views.markers.internal.TableView$2.open(TableView.java:168)
	at org.eclipse.jface.viewers.StructuredViewer$2.run(StructuredViewer.java:741)
	at org.eclipse.core.internal.runtime.InternalPlatform.run(InternalPlatform.java:1044)
	at org.eclipse.core.runtime.Platform.run(Platform.java:783)
	at org.eclipse.ui.internal.JFaceUtil$1.run(JFaceUtil.java:44)
	at org.eclipse.jface.util.SafeRunnable.run(SafeRunnable.java:148)
	at org.eclipse.jface.viewers.StructuredViewer.fireOpen(StructuredViewer.java:739)
	at org.eclipse.jface.viewers.StructuredViewer.handleOpen(StructuredViewer.java:968)
	at org.eclipse.jface.viewers.StructuredViewer$6.handleOpen(StructuredViewer.java:1067)
	at org.eclipse.jface.util.OpenStrategy.fireOpenEvent(OpenStrategy.java:243)
	at org.eclipse.jface.util.OpenStrategy.access$2(OpenStrategy.java:238)
	at org.eclipse.jface.util.OpenStrategy$1.handleEvent(OpenStrategy.java:277)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:66)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1021)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:2867)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:2572)
	at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:1699)
	at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:1663)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:367)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:143)
	at org.eclipse.ui.internal.ide.IDEApplication.run(IDEApplication.java:103)
	at org.eclipse.core.internal.runtime.PlatformActivator$1.run(PlatformActivator.java:226)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:376)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:163)
	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:324)
	at org.eclipse.core.launcher.Main.invokeFramework(Main.java:334)
	at org.eclipse.core.launcher.Main.basicRun(Main.java:278)
	at org.eclipse.core.launcher.Main.run(Main.java:973)
	at org.eclipse.core.launcher.Main.main(Main.java:948)

The stack trace is from 3.1.2.