Skip to content

Add destroy to Drawing to close window programatically - #245

Merged
sunjay merged 4 commits into
sunjay:masterfrom
sathwikmatsa:master
Jul 16, 2021
Merged

Add destroy to Drawing to close window programatically#245
sunjay merged 4 commits into
sunjay:masterfrom
sathwikmatsa:master

Conversation

@sathwikmatsa

Copy link
Copy Markdown
Contributor

Resolves #181
Modified examples/runtest.rs to use Drawing::destroy() and tested it on Mac.

@sunjay sunjay left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for taking the time to implement this feature! Unfortunately, this repo doesn't currently support rustfmt (a lot of the formatting decisions made by that tool aren't super desirable). Could you update your PR to only contain changes relevant to the feature you're implementing? You can usually do this by using a tool like git gui or sourcetree to only select the changes you want to submit.

As far as I can see you've done a great job implementing this. No major feedback other than a few small comments. I'd be happy to take another look and merge once we've narrowed this down to only the changes needed for #181. Thanks again! 🎉 😁

Comment thread src/drawing.rs
Self { event_loop }
}

pub fn exit(&self) -> Result<(), EventLoopClosed> {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

nit: could you move this method down to the bottom so it matches the order in the enum? (makes it a bit easier to see that each variant of the enum has a corresponding method here)

Self {}
}

pub fn exit(&self) -> Result<(), EventLoopClosed> {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please also move this method down per my previous comment.

@sunjay sunjay left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Great work @sathwikmatsa! Appreciate you writing docs and an example for the method and knowing to add the compile_error! call to the runtest example.

@sunjay
sunjay merged commit c9918c4 into sunjay:master Jul 16, 2021
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.

Add a way to close the window

2 participants