RFR: JDK-8324950: IGV: save the state to a file [v18]

[Tobias Holenstein]

On Wed, 10 Apr 2024 08:51:44 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

> Hi Toby, this feature seems very useful, thanks for developing it! I very much prefer the current model that saves the state into the graph XML file compared to the original proposal. A few comments (this is not a full review yet):
Thanks for the comments!
> * The export/import icons I see differ from those shown in the PR description:
> 
> ![outline](https://private-user-images.githubusercontent.com/8792647/321167807-946952d4-4ddc-495f-b5cd-22adf9721839.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTI3NDQ3MjYsIm5iZiI6MTcxMjc0NDQyNiwicGF0aCI6Ii84NzkyNjQ3LzMyMTE2NzgwNy05NDY5NTJkNC00ZGRjLTQ5NWYtYjVjZC0yMmFkZjk3MjE4MzkucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDQxMCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA0MTBUMTAyMDI2WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9ZTUyMzA4NWQ3NzljNGM3NDJhY2I0OGQzYTJmOGY1Zjg5MmVlODcwN2I5YmI3ODNmYWE5NzI2NzZkYzY0NDE0MCZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.x5VxRqOAxE-ZWBjP9GlmvJacfQBMgrY4tAMTvvUtSuE)

Strange. How did you checkout the PR? Seems like the newly added files `export.png` and `open.png` can not be found. (E.g. when applying as a patch like `git apply --index 8324950.diff` binary (png) files are not added) - Can you try:

_Checkout this PR locally:_
`$ git fetch https://git.openjdk.org/jdk.git pull/17630/head:pull/17630`
`$ git checkout pull/17630`

> * I find the distinction between 'Save' and 'Export' a bit unclear from a user perspective. My suggestion would be to either remove 'Export' (one can achieve the same result by removing the unnecessary graphs and then saving all remaining ones) or rename 'Save', 'Save as...', and 'Export' with 'Save all', 'Save all as...', and 'Save selected as...' (and using similar icons for the three), if that is the only difference between saving and exporting.

Right, I understand that it can be a bit unclear. I would suggest to drop the `Export` option and move `Import` next after `Open`:
<img width="200" alt="bar" src="https://github.com/openjdk/jdk/assets/71546117/3721ec57-cdad-4051-aefc-ec2cb41bebcc">

> * It would be nice to make more tab-specific state persistent, for example which view is displayed (sea of nodes, CFG, etc.) or whether neighboring nodes of extracted nodes are shown. This could be done here or in a future RFE.

Yes, this would be nice. Also IGV could reopen the last opened XML at startup. But I suggest a future RFE for this.

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

PR Comment: https://git.openjdk.org/jdk/pull/17630#issuecomment-2047180083