abstratt / eclipsegraphviz

EclipseGraphviz makes it easier to integrate Graphviz into Eclipse applications.
Eclipse Public License 1.0
23 stars 13 forks source link

Add support for using the tool as a debugging aid #1

Closed arcanefoam closed 8 years ago

arcanefoam commented 9 years ago

Added new classes and methods to allow the Graphviz View to be used to display debug variables as graphs.

arcanefoam commented 9 years ago

Some further understanding of the debug connection would be nice to also get updates when there is a value change in the variable. Currently you need to select it again to update the image.

abstratt commented 9 years ago

Thanks for your contribution, @arcanefoam. Glad things are working for your use case.

I won't be able to merge this as is though. The root problem is the scenario is a matter of lack of separation of concerns. In practical terms, the EclipseGraphviz plug-in should not have a dependency on the Eclipse Debug API (or any new dependencies in general).

It should be easy to address this issue though. I think the feature for showing debug objects in the Graphviz view that you built deserves its own plug-in, which would provide its own selection listener that understands the Debug API objects. The Graphviz view would only need to be changed to support an arbitrary content provider being injected into it, or an arbitrary input being injected into it. Then your debugging plugin can do the kind of stuff you currently placed into the base code of the EclipseGraphviz project and just feed the results into the view.

arcanefoam commented 9 years ago

I think I understand what you are saying. I will see if I can make the changes a provide an initial implementation and get back to you.

abstratt commented 9 years ago

Thanks, @arcanefoam. What I'd hope to see is a very small patch for GraphicalView that enables you to build your feature as a separate plugin (which will then have a dependency on Eclipse Debug etc). I'd suggest, for starters, that you did not contribute your plug-in at first, keep it separate for now. We can look into adding it later, if you'd prefer to do so.

Also, don't worry about updating version numbers, those can remain the same, I am using timestamp qualifiers to distinguish them.

arcanefoam commented 9 years ago

I changed the version number so I could install them in my eclipse and be recognized as a newer version :O. I will do a separate plugin not directly related to graphviz for starters first then.

arcanefoam commented 9 years ago

Hi, I've had some time to look into this again. The problem of separating the concerns is that, imho, the Viewer should now that it can handle different type of objects. The original implementation assumes that objects being represented are Files, and thus the View searches for a content provider based on the file content type. To support displaying any other kind of object, the view would need to know something about this object in order to get the correct content provider (even though "content provider" is particular for files). So, in order to be truly extendable, EclipseGraphviz shouldn't have a dependency on IContentType... perhaps the extension point registers an Class and a Reader. So the original EclipseGraphviz will register a reader (and a IGraphicalObjectRepresnetaion, that replaces IGraphicalContentProvider and does not extend content provider) for File.class. My plugin will register a reader for JDILocalVariable. What do you think?

abstratt commented 9 years ago

@arcanefoam Since your use case does not involve files, we need a way of loading contents obtained from wherever into the view. All the view should care is that the contents it is being fed with can be rendered as an image.

I just committed a change (89ea449d7f0e70d270ec10c79cdba830213f0b34) that allows that: you call setContents(myContents, someGraphicalContentProvider) and it uses the given graphical content provider to turn the raw contents into an image and loads that image into the view.

The idea is that now you can call that method whenever you feel like and pass your DOT string (as a byte array) and an instance of DOTGraphicalContentProvider. I believe that would cover your use case, with minimal impact to the existing functionality, right?

I have not tested it (other than ensuring that the existing code, which was changed to call setContents, still works). I don't have any external code that actually tries to use it.

If you think this will work for you, please give a try at invoking it from your debug plug-in. If you have any issues, please report them, and if you make your plug-in code available, I will make sure the explicit content loading feature works for your plug-in.

abstratt commented 9 years ago

@arcanefoam BTW, I think I understood where you were going with your proposal, but while I want to enable your use case, at this time I'd like to keep changes to the existing design to a minimum.

abstratt commented 9 years ago

@arcanefoam I went ahead and implemented a sample action that pushes the clipboard's contents into the view using setContents() as shown above. It works fine.

I also implemented support for disabling the automatic synchronization of the view with other the editor/other views so it would not lose the contents you just set (see #2). Just call setAutSync(false) on GraphicalView. The user can re-enable autosync later if they want using a new toolbar action.

arcanefoam commented 9 years ago

Thanks for looking into this! I will merge look at your changes and move my functionality to a separate plugin. Do you think you can share the code of the sample action you tried?

abstratt commented 9 years ago

@arcanefoam I ended up actually commmitting it as a feature:

https://github.com/abstratt/eclipsegraphviz/blob/master/plugins/com.abstratt.graphviz.ui/src/com/abstratt/graphviz/ui/RenderClipboardAction.java

It is an action contributed to the view itself, the view object is readily available. In your case, I believe the behavior is not implemented as an action to the GraphicalView, so you would have to retrieve the view by looking it up (id is com.abstratt.imageviewer.GraphicalView).

arcanefoam commented 8 years ago

Hi, Sorry for the late reply but had been unable to work more on this idea. I have craeted a separate plugin and used the new functionality you provided. I am using the same two liner as your Clipboard action but it is not working for me. Specifically I am getting an "org.eclipse.swt.SWTException: Unsupported or unrecognized format" exception when both setting the new content provider and setting the new input. Any ideas what might be causing this? (Code is at https://github.com/arcanefoam/dotdebugview, if you want to take a look. I am new to Eclipse UI and jobs so some of it might be dodgy ;) ).

abstratt commented 8 years ago

@arcanefoam could you please paste the entire stack trace(s) here?

arcanefoam commented 8 years ago

@abstratt Find the trace at the end. If you want to take a look at my code (just pushed some changes) the method of interest is DebugListener.syncWithUi(). Trace: !ENTRY org.eclipse.core.jobs 4 2 2015-10-02 10:16:09.960 !MESSAGE An internal error occurred during: "Image loading job". !STACK 0 org.eclipse.swt.SWTException: Unsupported or unrecognized format at org.eclipse.swt.SWT.error(SWT.java:4491) at org.eclipse.swt.SWT.error(SWT.java:4406) at org.eclipse.swt.SWT.error(SWT.java:4377) at org.eclipse.swt.internal.image.FileFormat.load(FileFormat.java:84) at org.eclipse.swt.graphics.ImageLoader.load(ImageLoader.java:146) at org.eclipse.swt.graphics.ImageDataLoader.load(ImageDataLoader.java:22) at org.eclipse.swt.graphics.ImageData.(ImageData.java:331) at com.abstratt.imageviewer.DefaultGraphicalContentProvider.loadImage(DefaultGraphicalContentProvider.java:20) at com.abstratt.imageviewer.AbstractGraphicalContentProvider$ContentLoader.run(AbstractGraphicalContentProvider.java:81) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55)

abstratt commented 8 years ago

Thanks. I am pretty sure this is due to arcanefoam/dotdebugview#1.

arcanefoam commented 8 years ago

Hi, Thanks a lot for taking the time to look into this. It is working nicley now :D. Next will be considering providing an update site/marketplace for this. Any recomendations?

abstratt commented 8 years ago

@arcanefoam Good to hear.

This is a good overview: https://wiki.eclipse.org/FAQ_How_do_I_create_an_update_site_(site.xml)%3F

Good luck!

abstratt commented 8 years ago

Cool, I guess we can close this. @arcanefoam open a new issue/PR if you have any other things you would like to change in eclipsegraphviz. Cheers!