-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat(clipboard): fix interaction with freeze and select feature: Issue #109 #110
feat(clipboard): fix interaction with freeze and select feature: Issue #109 #110
Conversation
Andreas is fairly busy this week, do not expect a fast response. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix! Sorry it took me a while to review, I was sick for the entire week. Looks good to land except for the destroying regardless of whetehr the callback succeeds.
libwayshot/src/lib.rs
Outdated
&self, | ||
frames: &[(FrameCopy, FrameGuard, OutputInfo)], | ||
callback: Box<dyn Fn() -> Result<LogicalRegion, Error>>, | ||
) -> Result<(LogicalRegion)> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
) -> Result<(LogicalRegion)> { | |
) -> Result<LogicalRegion> { |
@Shinyzenith it seems like we don't have clippy enabled in GitHub actions?
Compiling wayshot v1.3.2-dev (/home/andreas/dev/wayshot/wayshot)
warning: unused import: `Region`
--> libwayshot/src/lib.rs:55:29
|
55 | region::{LogicalRegion, Region, Size},
| ^^^^^^
|
= note: `#[warn(unused_imports)]` on by default
warning: unnecessary parentheses around type
--> libwayshot/src/lib.rs:422:17
|
422 | ) -> Result<(LogicalRegion)> {
| ^ ^
|
= note: `#[warn(unused_parens)]` on by default
help: remove these parentheses
|
422 - ) -> Result<(LogicalRegion)> {
422 + ) -> Result<LogicalRegion> {
|
warning: unused import: `GenericImageView`
--> libwayshot/src/image_util.rs:1:27
|
1 | use image::{DynamicImage, GenericImageView};
| ^^^^^^^^^^^^^^^^
warning: unused variable: `logical_height`
--> libwayshot/src/image_util.rs:15:25
|
15 | let (logical_width, logical_height) = match transform {
| ^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_logical_height`
|
= note: `#[warn(unused_variables)]` on by default
warning: `libwayshot` (lib) generated 4 warnings (run `cargo fix --lib -p libwayshot` to apply 3 suggestions)
warning: unused import: `fs::File`
--> wayshot/src/wayshot.rs:2:5
|
2 | fs::File,
| ^^^^^^^^
|
= note: `#[warn(unused_imports)]` on by default
warning: `wayshot` (bin "wayshot") generated 1 warning (run `cargo fix --bin "wayshot"` to apply 1 suggestion)
Finished release [optimized] target(s) in 0.92s
Running `target/release/wayshot --clipboard --slurp ''`
Error: error occurred in freeze callback
Location:
wayshot/src/wayshot.rs:72:9
Shouldn't impact the PR, but wanted to mention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will address this.
Modifies the overlay creation function to destroy the shell surfaces after use. Not much familiar with layer shell so input from @AndreasBackx would be appreciated on if this is all that is required. I have moved the slurp callback activation into that function too because it needs to be called before the surface is destroyed and destroying the surface needs context only present within this function.