Bugzilla – Full Text Bug Listing
| Summary: | Extract PDF export from DOT Graph view (and turn it into a global DOT editor action) | ||
|---|---|---|---|
| Product: | [Tools] GEF | Reporter: | satyagraha.1956 |
| Component: | GEF DOT | Assignee: | Alexander Nyßen <nyssen> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | miklossy, nyssen, steeg |
| Version: | unspecified | ||
| Target Milestone: | 4.0.0 / 3.11.0 (Neon) M4 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| Attachments: | |||
|
Description
satyagraha.1956
Changing component to GEF4 Zest as it affects Zest.FX.UI. From my perspective, the following behavior would be appropriate when being in linked-mode: 1) If the user selects a *.dot resource in the Project Explorer, Navigator, etc., i.e. the active selection points to a respective IResource, and the link-button is on, it would be nice to have the contents of that resource rendered in the viewer. 2) If the DOT editor is the active part and the link-button is on, it would be nice if the live-editing state of the editor would be reflected in the viewer (up to now it only seems to reflect the persisted state). Changing back to GEF4 DOT, as the DotGrahView has been moved to DOT.UI. Having re-thought this, I now think the behavior of the Graph viewer should really be comparable to that of the Outline view. That is, it should be bound to the contents of a DOT-editor, visualizing the actual edit state of the editor similar to the outline view. This way, also dirty edit states of DOT files could be visualized. Of course it would mean that a DOT editor would have to be opened on a file to show it in the Graph viewer, but the same holds for any other input file and the outline (nobody expects the outline to load a java class without having it opened in the editor). As DOT editor and graph viewer are both are provided by the DOT component, they are always available as a pair. As the project explorer already provides a sync-button to synchronize editors to selected files, this mechanism could simply be used to (indirectly) change the input of the graph viewer as well. I would thus propose to remove the file-based load and sync functionality from the Graph viewer and refactor it to be bound to the active DOT editor (exactly the same behavior as the outline view). (In reply to Alexander Nyßen from comment #3) Pushed to master: update graph view in sync mode when DOT editor is selected. http://git.eclipse.org/c/gef/org.eclipse.gef4.git/commit/?id=1e92f86657d78 I would prefer not to remove the resource based sync, as one of the main use cases I hope people will find the view useful for is to easily visualize debugging output with DOT. Any program can write a simple DOT file anywhere in the workspace and it will be visualized when in sync mode. If we agree to keep this resource based sync, I wonder if it's a good choice to also update the view while editing. A resource-only approach seems clearer and more consistent to me. In particular in interaction with the sync to PDF mode, which renders the file with Graphviz. I think this new behavior is reasonably close to the outline (and covers the scenario from comment #0), but maintains its own specific focus: to visualize a DOT file. For a live view, we have the custom outline (see bug 452650). I took a look at the new behavior, and in linked mode it now behaves as I expect (i.e. uses the input of the dot editor). There is only a small issue, namely that the link mode tooltip still speaks of "Sync with .doc files in the workspace", whereas it should probably say something like "Sync with.dot editor input". If we leave the resource-based mode in place (which seems to be reasonable), I think the editor-sync mode should rely on the current edit state of the editor as it currently does. If a user does not want this behavior, he may use the resource-based mode instead. Nevertheless, I still see the following points for improvement: 1) If being in resource-based mode, there is no indication to which resource the viewer is currently bound (i.e. which one was loaded). Also, if a resource was initially loaded, only the toggle state of the link toggle button indicates whether the editor input is currently used or the loaded resource. IMHO there should be some label to indicate which resource is currently used (either directly, because it was loaded, or indirectly, because we are in linked mode; we could add a suffix like "<editor-synched>" to indicate the latter). - If we are not in editor-sync mode and no resource was loaded, its not directly clear what to do. We could use the viewer area to display some help information: "Please load a resource or enable editor-sync mode" or something similar. - The load resource dialog is not intuitive, as one does not see the workspace resources directly but has to type in some search term. IMHO, we should replace it with a more appropriate dialog that allows to browse the workspace and file system in addition. (In reply to Alexander Nyßen from comment #5) Good point about the label to show info on the current resource. Pushed: http://git.eclipse.org/c/gef/org.eclipse.gef4.git/commit/?id=a9de3107867b3 I also agree that we should use a more accessible file selection dialog that also allows selection of non-workspace resources. The previous functionality of searching by name in the dialog can be achieved by enabling sync mode and using the Open Resource dialog. Pushed: http://git.eclipse.org/c/gef/org.eclipse.gef4.git/commit/?id=7ca2de143c503 > If we leave the resource-based mode in place (which seems to be reasonable), > I think the editor-sync mode should rely on the current edit state of the > editor as it currently does. By edit state you mean the current, unsaved editor state, right? That is not implemented yet, and as I described in comment #4 I don't think we should do it that way. I therefore didn't change the tooltip for sync mode. (In reply to Fabian Steeg from comment #6) > (In reply to Alexander Nyßen from comment #5) > > Good point about the label to show info on the current resource. Pushed: > > http://git.eclipse.org/c/gef/org.eclipse.gef4.git/commit/?id=a9de3107867b3 > > I also agree that we should use a more accessible file selection dialog that > also allows selection of non-workspace resources. The previous functionality > of searching by name in the dialog can be achieved by enabling sync mode and > using the Open Resource dialog. Pushed: > > http://git.eclipse.org/c/gef/org.eclipse.gef4.git/commit/?id=7ca2de143c503 That looks a lot nicer. Thanks! > > > If we leave the resource-based mode in place (which seems to be reasonable), > > I think the editor-sync mode should rely on the current edit state of the > > editor as it currently does. > > By edit state you mean the current, unsaved editor state, right? That is not > implemented yet, and as I described in comment #4 I don't think we should do > it that way. I therefore didn't change the tooltip for sync mode. Yes, that's what I meant. I admit I didn't get your point of duality with respect to the PDF export, until having looked into the DOT component a bit closer in the last days. I actually thought about the PDF/image export and came to doubt about the overall validity of the approach. Wouldn't it be clearer, if we use Zest (i.e. the Dot Graph view) for rendering a dot graph, to also export into image/PDF what is actually rendered by Zest? Also, as Zest is now based on JavaFX it will allow to render much nicer visualizations, so why rely on GraphViz here? Where a GraphViz integration IMHO makes sense is in the context of GEF4 Layout, because to properly mimic the GraphViz layout algorithms seems to be unachievable. If that would be available, one could use the benefits of both worlds (much more capable Layouting of GraphViz combined with much nicer rendering options of JavaFX). To summarize, I would propose to replace the GraphViz-based image/PDF export with a MVC/Zest-based one (we will need to offer such a functionality anyway), and to rather aim at a GraphViz integration for our GEF4 Layouting component. What do you think? (In reply to Alexander Nyßen from comment #7) I understand your thoughts about Graphviz here. I'll try to explain my own idea and experience with the DOT component. For me it actually consists of two parts: 1) a pure Java DOT rendering engine based on Zest, and 2) authoring tools for editing and rendering DOT graphs. And I agree, ideally 2) could be fully implemented by 1). Realistically though, I think it will take a very long time until Zest will actually be functionally on par with Graphviz, which is incredibly rich in rendering options. So in practice, if I create DOT graphs (for documentation etc.), I want to render them with Graphviz. And not only the final version for publishing, but also while editing (to get feedback on twiddling with the rendering options). In this most common use case for myself, I always have the PDF sync mode enabled. So I agree with your vision. When we have Zest rendering and image export on par with Graphviz, we can switch the default to that. However I think we should always retain the option to render with Graphviz. I think of it like a compiler config option. (In reply to Fabian Steeg from comment #8) > (In reply to Alexander Nyßen from comment #7) > > I understand your thoughts about Graphviz here. I'll try to explain my own > idea and experience with the DOT component. For me it actually consists of > two parts: > > 1) a pure Java DOT rendering engine based on Zest, and > 2) authoring tools for editing and rendering DOT graphs. > > And I agree, ideally 2) could be fully implemented by 1). Realistically > though, I think it will take a very long time until Zest will actually be > functionally on par with Graphviz, which is incredibly rich in rendering > options. > > So in practice, if I create DOT graphs (for documentation etc.), I want to > render them with Graphviz. And not only the final version for publishing, > but also while editing (to get feedback on twiddling with the rendering > options). In this most common use case for myself, I always have the PDF > sync mode enabled. > > So I agree with your vision. When we have Zest rendering and image export on > par with Graphviz, we can switch the default to that. However I think we > should always retain the option to render with Graphviz. I think of it like > a compiler config option. I agree on that. An image/pdf export for GEF4 MVC (and thereby also for GEF4 Zest and the Dot Graph Viewer) would be a vision for the long term. And a GraphViz-based PDF/image export option could still make sense even if we realize this vision. Having reconsidered this, I think my major concern for now (and the reason that the current PDF-sync looks inconsistent to me) is that the PDF-sync functionality is related to the Dot Graph view (which is bound to Zest-rendering and actually has nothing to do with GraphViz-based visualization). Wouldn't it make thinks much clearer, if we turn this into a global toogle action (i.e. in the main toolbar) that is either bound to the dot-editor (i.e. create a PDF output as kind of a save action) or even realized as a builder? This way, the PDF-sync and the Zest-based rendering would both be bound to .dot files, and would furthermore be clearly be separated. Based on my last comment, I would propose to implement the following changes: - Remove the "Toogle PDF-sync" action from the DOT Graph view and turn it into a DOT editor action, i.e. one that gets integrated into the global toolbar. The DOT editor should evaluate the state when saving, i.e. produce a PDF on every editor save - Ensure that the DOT Graph view shows the current edit state of the DOT editor if being in linked mode (if the PDF-sync is no longer bound to the Graph view, that seems to be more natural to me, as the view - in linked mode - serves as sort of a graphical outline). (In reply to Alexander Nyßen from comment #10) > Based on my last comment, I would propose to implement the following changes: > > - Remove the "Toogle PDF-sync" action from the DOT Graph view and turn it > into a DOT editor action, i.e. one that gets integrated into the global > toolbar. The DOT editor should evaluate the state when saving, i.e. produce > a PDF on every editor save > - Ensure that the DOT Graph view shows the current edit state of the DOT > editor if being in linked mode (if the PDF-sync is no longer bound to the > Graph view, that seems to be more natural to me, as the view - in linked > mode - serves as sort of a graphical outline). Fabian, do you agree? (In reply to Alexander Nyßen from comment #11) > > - Remove the "Toogle PDF-sync" action from the DOT Graph view and turn it > > into a DOT editor action, i.e. one that gets integrated into the global > > toolbar. The DOT editor should evaluate the state when saving, i.e. produce > > a PDF on every editor save The builder you mentioned in comment #9 sounds like a better option to me than an editor action, as it would retain the possibility to generate PDFs for *.dot files that are not edited in an editor. However I'm not sure about the practical benefit of a builder over the current solution. It's true, the coupling of the Zest and the Graphviz rendering in the current view can seem arbitrary. On the other hand, it's both DOT rendering, and there's something about having our (very limited) DOT rendering features in one place. > > - Ensure that the DOT Graph view shows the current edit state of the DOT > > editor if being in linked mode (if the PDF-sync is no longer bound to the > > Graph view, that seems to be more natural to me, as the view - in linked > > mode - serves as sort of a graphical outline). As described in comment #4 I'd prefer the simple resource based approach. By the way I don't really see the view as an outline, in particular because of this very idea of a versatile resource based DOT rendering tool. I mentioned the debugging scenario, and I could imagine all kinds of (context aware) visual aids created dynamically using DOT, visualized in that view. > Fabian, do you agree? From dragging my feet on this it seems these options don't convince me enough to take the time to implement them myself. Nevertheless, if the current functionality is retained, I wouldn't object to a solution with a builder and current editor state support either. To improve the usability of the graph view, I've added links to the default info message shown when no graph is loaded (see comment #6), which trigger the corresponding toolbar actions, like this: <a>Load *.dot file</a> or <a>toggle sync mode</a> and edit it. http://git.eclipse.org/c/gef/org.eclipse.gef4.git/commit/?id=2369a94c2f4dc Fixed issues exposed by image export on editor selection (see comment #4): http://git.eclipse.org/c/gef/org.eclipse.gef4.git/commit/?id=d304de6a4cd67 Created attachment 257496 [details] [337644] DOT Graph view updating in linked mode I took a look at this issue and share the opinion of Alexander Nyßen. I attached a patch with the following improvements: [337644] DOT Graph view updating in linked mode - Move the "DOT Export" button from the DOT Graph View toolbar to the workbench(main) toolbar. Change the type of the button from "toggle" to "push". The button will be visible only if the active editor is a DOT Editor. - Update the DOT Graph view immediately if a DOT editor is open/active and the user turns the "link with editor mode" on. Till now the DOT Graph View was not updated immediately, but the user had to click into the Dot Editor first to get the DOT Graph View updated. - Change the tooltip of the "link with editor mode" button on the DOT Graph View from "Sync with *.dot files opened in DOT editor" to "Link with DOT Editor" - Use the path to the dot executable directly, instead of determining the directory from the file location and appending the dot/dot.exe to it again. Hereby I confirm that my contribution complies with https://www.eclipse.org/legal/CoO.php. Thanks Tamas. It seems your patch is not up to date with origin/master. It overwrites some of the changes that have been applied within bug #446639. Could you please rebase and re-create the patch? Created attachment 257526 [details]
Rebase to HEAD origin/master
Comment on attachment 257526 [details] Rebase to HEAD origin/master Thanks Tamas. Something still looks strange to me. The contribution comment within DotNativeDrawer and the changes within DotNativeDrawer and GraphvizPreferencePage do not seem to be related to this bug, but rather to bug #446639. Or am I getting something wrong here? You are right, these changes are related to: - Use the path to the dot executable directly, instead of determining the directory from the file location and appending the dot/dot.exe to it again. These changes belong to the ticket bug #446639 rather than to this ticket. Since that ticket already has been closed, I thought applying these changes here, as this issue is also related to the "Export" button, which triggers showing the Graphviz Preference page (in case of Graphviz misconfiguration) and/or starting the export process (in case of a proper Graphviz configuration). (In reply to Tamas Miklossy from comment #19) > You are right, these changes are related to: > > - Use the path to the dot executable directly, instead of determining > the directory from the file location and appending the dot/dot.exe to it > again. > > These changes belong to the ticket bug #446639 rather than to this ticket. > Since that ticket already has been closed, I thought applying these changes > here, as this issue is also related to the "Export" button, which triggers > showing the Graphviz Preference page (in case of Graphviz misconfiguration) > and/or starting the export process (in case of a proper Graphviz > configuration). That does not seem to be straight-forward. Can you please reopen bug #446639 and upload a separate patch with the respective changes there (and remove them from your patch for this bugzilla)? Created attachment 257557 [details] [337644] DOT Graph view updating in linked mode Removed changes belonging to bug #446639 (In reply to Tamas Miklossy from comment #21) > Created attachment 257557 [details] > [337644] DOT Graph view updating in linked mode > > Removed changes belonging to bug #446639 While the extraction of the dot action looks ok, I think turning it into a push button somehow compromises the original use-case behind. With the old push button it was possible to keep the exported pdf in-sync with the dot file content (it was automatically updated). IMHO we should preserve this functionality. Second, its not optimal to have the printer logo now twice in the toolbar (that was not nice before as well, but the Graph view at least had its own scope). I have contacted the Graphviz team to see whether there is a 16x16 logo we could adopt for it (Graphviz is EPL as well). I think that would be most appropriate (maybe in combination with a pdf overlay). Hello Alexander, Fabian I get the following exception while using the DOT "live export" functionality: Calling '[C:\Program Files (x86)\Graphviz2.38\bin\dot.exe, -Tpdf, -o, C:\Users\miklossy\AppData\Local\Temp\zest2401480869798072511.dot.pdf, C:\Users\miklossy\AppData\Local\Temp\zest2401480869798072511.dot]' resulted in exit status: 0. Calling '[C:\Program Files (x86)\Graphviz2.38\bin\dot.exe, -Tpdf, -o, C:\Users\miklossy\AppData\Local\Temp\zest5327564323750790465.dot.pdf, C:\Users\miklossy\AppData\Local\Temp\zest5327564323750790465.dot]' resulted in exit status: 0. java.io.FileNotFoundException: C:\eclipse-commiters-mars\runtime-GEF4\DotFiles\arrowshapes_deprecated.dot.pdf (Der Prozess kann nicht auf die Datei zugreifen, da sie von einem anderen Prozess verwendet wird) at java.io.FileOutputStream.open0(Native Method) at java.io.FileOutputStream.open(FileOutputStream.java:270) at java.io.FileOutputStream.<init>(FileOutputStream.java:213) at java.io.FileOutputStream.<init>(FileOutputStream.java:162) at org.eclipse.gef4.internal.dot.DotFileUtils.copySingleFile(DotFileUtils.java:171) at org.eclipse.gef4.internal.dot.ui.DotGraphView$ExportToggle.generateImageFromGraph(DotGraphView.java:254) at org.eclipse.gef4.internal.dot.ui.DotGraphView$ExportToggle.linkCorrespondingImage(DotGraphView.java:333) at org.eclipse.gef4.internal.dot.ui.DotGraphView$2.run(DotGraphView.java:181) 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:4155) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3772) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run(PartRenderingEngine.java:1127) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:337) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1018) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:156) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:654) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:337) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:598) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:139) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:380) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:235) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:497) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:669) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:608) at org.eclipse.equinox.launcher.Main.run(Main.java:1515) at org.eclipse.equinox.launcher.Main.main(Main.java:1488) On my Windows 7 computer is Adobe Acrobat Reader DC assigned as the standard application for opening pdf files. Generating the .pdf file for the first time works without any problem (see the first calling command in the attached log). Generating the .pdf file for the second time without closing the Adobe Acrobat Reader DC before results in a exception (see the second calling command in the attached log). It seems that the regenearted pdf file cannot be overwritten since the Adobe Acrobat Reader DC is still holding the file handler for that. @Alexander, @Fabian: Do you experience the same problem on your computers? @Fabian: From the attached log it is visible that the invoked dot.exe command is operating on some temporary files. The generated temporary pdf file is copied and renamed into the directory where the .dot file is located. Why are these temprory files/copy mechanisms necessary? (In reply to Tamas Miklossy from comment #23) > Do you experience the same problem on your computers? It works fine with master on my Linux machine (Ubuntu, Atril PDF viewer). I remember the error message from Windows. Since this always worked on my Mac and Linux machines, I assumed Acrobat does not support updating the open PDF. Back then I switched to a different PDF viewer on Windows. > From the attached log it is visible that the invoked dot.exe command is > operating on some temporary files. The generated temporary pdf file is > copied and renamed into the directory where the .dot file is located. > Why are these temprory files/copy mechanisms necessary? The DotGraphView tracks its current textual DOT content in a string field (currentDot). The export generates a temp file for that textual content. From looking at the code in DotGraphView, the scenario where we have a textual form but no file is embedded DOT, so that seems to be the reason for the field and this first temp file. The second temp file, the output PDF, seems unnecessary if we have an input file. So instead of copying it in DotGraphViewer.ExportToggle.generateImageFromGraph, we could pass the output file name to DotNativeDrawer.renderImage (instead of passing null). Created attachment 257951 [details]
separate "live export" from "live update" functionality
[337644] Refactoring of DOT Graph view live update/live export
1. Live Update:
- Rename tooltip: "Sync with *.dot files opened in DOT editor" to "DOT
Live Update".
- Update the DOT Graph view immediately if a DOT editor is open/active
and the user turns the "DOT Live Update" on. Till now the DOT
Graph View was not updated immediately, but the user had to click into
the Dot Editor first to get the DOT Graph View updated.
2. Live Export:
- Rename tooltip: "Sync with printable PDF using Graphviz" to "DOT Live
Export".
- Move the "DOT Live Export" button from the DOT Graph View toolbar to
the workbench toolbar.
- Bound the "DOT Live Export" button visibility to the active editor:
visible only if the open/active editor is a DOT Editor.
- Extract the implementation of the live export function from the
DotGraphView.ExportToggle class into a separate LiveExportHandler class.
- Use IPartListener2 instead of ISelectionListener for performance
improvement (ISelectionListener is notified on every keystroke).
(In reply to Alexander Nyßen from comment #22) > (In reply to Tamas Miklossy from comment #21) > > Created attachment 257557 [details] > > [337644] DOT Graph view updating in linked mode > > > > Removed changes belonging to bug #446639 > > While the extraction of the dot action looks ok, I think turning it into a > push button somehow compromises the original use-case behind. With the old > push button it was possible to keep the exported pdf in-sync with the dot > file content (it was automatically updated). IMHO we should preserve this > functionality. Patch added where the export toggle functionality ("live export") is preserved. > > Second, its not optimal to have the printer logo now twice in the toolbar > (that was not nice before as well, but the Graph view at least had its own > scope). I have contacted the Graphviz team to see whether there is a 16x16 > logo we could adopt for it (Graphviz is EPL as well). I think that would be > most appropriate (maybe in combination with a pdf overlay). Hereby I confirm that my contribution complies with https://www.eclipse.org/legal/CoO.php. Comment on attachment 257951 [details]
separate "live export" from "live update" functionality
Unfortunately I cannot apply the patch (problems within DotGraphView). Can you rebase it on master, please?
Created attachment 258176 [details]
separate "live export" from "live update" functionality
(In reply to Tamas Miklossy from comment #29) > Created attachment 258176 [details] > separate "live export" from "live update" functionality Thanks Tamas. The patch is now applicable. I have two findings: - We should 'inline' PartAdapter2, as its only used in a single place. - It seems the DotGraphView is not updated properly when saving the editor. If you changed the edge style in the editor, the PDF output gets properly updated, while the DOT Graph view (in linked mode) is not. (In reply to Alexander Nyßen from comment #30) > (In reply to Tamas Miklossy from comment #29) > > Created attachment 258176 [details] > > separate "live export" from "live update" functionality > > Thanks Tamas. The patch is now applicable. I have two findings: > > - We should 'inline' PartAdapter2, as its only used in a single place. > - It seems the DotGraphView is not updated properly when saving the editor. > If you changed the edge style in the editor, the PDF output gets properly > updated, while the DOT Graph view (in linked mode) is not. It seems that edge styles are no longer rendered at all (unrelated to the changes in your patch). Created attachment 258179 [details] separate "live export" from "live update" functionality class PartAdapter2 inlined. Hereby I confirm that my contribution complies with https://www.eclipse.org/legal/CoO.php. (In reply to Tamas Miklossy from comment #32) > Created attachment 258179 [details] > separate "live export" from "live update" functionality > > class PartAdapter2 inlined. > > Hereby I confirm that my contribution complies with > https://www.eclipse.org/legal/CoO.php. Looks fine now, thanks! I have two (last) comments (and apologize for not having mentioned that before): - I would not change the tooltip of the load button to "DOT Live Update" but rather to "Link with DOT Editor". - I would change the name of the export from "DOT Live Export" to "Sync Graphviz Export (PDF)" and adjust the naming of handles and commands accordingly (SyncGraphvizExport). Created attachment 258180 [details] separate "pdf export" from "view sync" functionality Renaming tooltips. Hereby I confirm that my contribution complies with https://www.eclipse.org/legal/CoO.php. [337644] Refactoring of DOT Graph view live update/live export - Rename tooltip: "Sync with *.dot files opened in DOT editor" to "Link with DOT Editor". - Update the DOT Graph view immediately if a DOT editor is open/active and the user turns the "Link with DOT Editor" on. Till now the DOT Graph View was not updated immediately, but the user had to click into the Dot Editor first to get the DOT Graph View updated. - Rename tooltip: "Sync with printable PDF using Graphviz" to "Sync Graphviz Export (PDF)". - Move the "Sync Graphviz Export (PDF)" button from the DOT Graph View toolbar to the workbench toolbar. - Bound the button visibility to the active editor: visible only if the open/active editor is a DOT Editor. - Extract the implementation of the live export function from the DotGraphView.ExportToggle class into a separate LiveExportHandler class. - Use IPartListener2 instead of ISelectionListener for performance improvement (ISelectionListener is notified on every keystroke). Thanks Tamas! I applied your patch and pushed it to origin/master. Concerning the PDF export,the following things would still need to be investigated: 1) We should replace the printer logo with a better one that indicates the PDF export functionality. 2) We should remove the not needed temp file for the PDF export. I renamed this bugzilla to indicate it is finally related to an extraction of the PDF export alone. Any further refactorings should IMHO be handled in separate bugzillas. And, of course, we need to update the user documentation. I took a look into the master HEAD and seems that the unnecessary copying mechanism during pdf export has already been removed with the previous patch. The current dot execution command is the following: Calling '[C:\Program Files (x86)\Graphviz2.38\bin\dot.exe, -Tpdf, -o, C:\eclipse-commiters-mars\runtime-GEF4\DotFiles\styled_graph.dot.pdf, C:\eclipse-commiters-mars\runtime-GEF4\DotFiles\styled_graph.dot]' input file: styled_graph.dot output file: styled_graph.dot.pdf (we may change it to styled_graph.pdf) I opened bug #484132 to keep track of the icon replacement. Resolving this as fixed in 3.11.0 M4. |