RFR: JDK-8293364: IGV: Refactor Action in EditorTopComponent and fix minor bugs [v14]

[Christian Hagedorn]

On Tue, 20 Sep 2022 13:06:26 GMT, Tobias Holenstein <tholenstein at openjdk.org> wrote:

>> Refactor the Actions in EditorTopComponent (com.sun.hotspot.igv.view.actions). Move Action specific code from EditorTopComponent to the corresponding Action.
>> 
>> # Refactoring of com.sun.hotspot.igv.view.actions and EditorTopComponent
>> - Created a new `ExportGraph` Action and moved corresponding functions `exportToSVG(..)` and `exportToPDF(..)` to new `ExportGraph.java`
>> - Moved key bindings for satellite-view (pressing S) from `EditorTopComponent` to `OverviewAction.java` 
>> - Moved Action specific code from `EditorTopComponent` to the corresponding `XXXAction.java`
>> - Changed `PrevDiagramAction`, `ExpandDiffAction`, `ExtractAction`, `HideAction`, `NextDiagramAction`, `ReduceDiffAction` and `ShowAllAction` to be context aware `ContextAction<DiagramViewModel>` actions and use more modern `@ActionRegistration` to move away from manually defining actions in `layer.xml`   
>> - new `addContextListener` / `removeContextListener` function  in `ContextAction<T>` enables context aware actions to define to which `ChangedEvent` they want to react to
>> 
>> # Fixing minor Bugs
>> - "Show empty blocks in control-flow graph view" is selected by default but only enabled in CFG view. 
>> This is distracting for the eye when we are not in CFG:
>> <img width="192" alt="cfg_before" src="https://user-images.githubusercontent.com/71546117/188463472-b928075f-a687-46d6-a871-35f287e943c7.png">
>> Now "Show empty blocks in control-flow graph view" is not selected anymore when disabled (greyed out)
>> <img width="192" alt="cfg_node_disable" src="https://user-images.githubusercontent.com/71546117/188463809-c998b03e-cc2f-48c1-8f5f-135edffa9c8c.png">
>> But still gets selected by default when enabled
>> <img width="197" alt="cfg_now" src="https://user-images.githubusercontent.com/71546117/188463853-33e6e7cb-b8ff-4c75-a522-63fd858b42ef.png">
>> 
>> - "Extract current set of selected nodes", "Hide selected nodes" and "show all nodes" were always enabled, even when they didn't effect anything. 
>> <img width="99" alt="selection_before" src="https://user-images.githubusercontent.com/71546117/188464555-45ef06af-b247-46b0-aaa2-4c2397566f8c.png">
>> Now "Extract current set of selected nodes", "Hide selected nodes" are disabled (greyed out) when no nodes are selected. And "show all nodes" is disabled (greyed out) when all nodes are already visible.
>> <img width="91" alt="selection_now" src="https://user-images.githubusercontent.com/71546117/188464589-61310b13-ba58-45a9-b2d8-d3be0da582ab.png">
>> 
>> - "Reduce the difference selection" got stuck when at the last graphs in the group because it got greyed out. 
>> <img width="130" alt="reduce_stuck" src="https://user-images.githubusercontent.com/71546117/188465260-6fca7ca6-3e8e-4bd9-9b70-4ac19d79eff5.png">
>> duce the difference selection" 
>> Now "Reduce the difference selection" works as expected:
>> <img width="152" alt="reduce_now" src="https://user-images.githubusercontent.com/71546117/188465314-bce441ba-20d6-4e3d-ba4e-ff1f4a79cd9d.png">
>
> Tobias Holenstein has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove empty componentOpened()

Thanks for doing the suggested updates. Apart from two small things, it looks good to me!

src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/EnableBlockLayoutAction.java line 35:

> 33: public class EnableBlockLayoutAction extends EnableLayoutAction {
> 34: 
> 35:     private final EditorTopComponent editor;

The field can also be moved to `EnableLayoutAction` as it is used in all subclasses.

src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/ExpandAdjacentAction.java line 41:

> 39:     }
> 40: 
> 41:     protected void expandFigures(AdjacentOperator adjacentOp) {

You could also directly use the predefined `Function` interface for one argument functions with return value:
Suggestion:

    protected void expandFigures(Function<Figure, List<Figure>> adjacentOp) {

-------------

Marked as reviewed by chagedorn (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10170