Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a history step when importing SVG files #1656

Merged
merged 2 commits into from
Mar 8, 2024

Conversation

milan-sedivy
Copy link
Contributor

Generally the issue is that the actual import and creation of a new SVG is handled through GraphOperationMessageHandler which doesn't have a "syncish" way of backuping data (the only way how you could backup the data from there is by rewriting the DocumentMessage::Backup message and fixing code around and after that you would call it, but that leads to an async backup of data meaning the "step before the import" will likely not get saved in time).

So my approach was to add a DocumentMessage::ImportSvg ... which pretty much tries to create a usvg::Tree the same way as GraphOperationMessage::NewSvg does... with the exception that it doesn't care about the result... only cares about the succesful unwraping Ok(_) of the Option<Tree> and then proceeds to send the original message (and calls self.backup(responses) before)

Now ofcourse the idea of just deserializing the tree and sending that crossed my mind but after investigating serde I realized it would just be a reimplementation of their Tree::to_string method which seemed pointless.
I'll look into what is your branching strategy and I'll submit a PR so that you can take a look at it
Maybe just to add... of course an option would be to just self.backup(...) on every GraphOperationMessage received by DocumentMessageHandler... but that would lead to undesired behaviour with some of the other tools that work with vector graphics

Copy link
Member

@0HyperCube 0HyperCube left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - thanks for the code improvement.

@milan-sedivy
Copy link
Contributor Author

Looks good - thanks for the code improvement.

Thanks for the suggestion/feedback, really appreciate it!

@Keavon
Copy link
Member

Keavon commented Mar 8, 2024

!build

Copy link

github-actions bot commented Mar 8, 2024

📦 Build Complete for 411a61e
https://395ff523.graphite.pages.dev

@Keavon Keavon merged commit ea3f834 into GraphiteEditor:master Mar 8, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants