-
-
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
Fix Issue #106: Clipboard flag ignored if path is qualified to file #108
Fix Issue #106: Clipboard flag ignored if path is qualified to file #108
Conversation
Hi, please rebase your changes onto freeze-feat-andreas as there's a merge conflict. |
Hi it seems you included a commit which already exists in freeze branch. Why? |
… feature flag Signed-off-by: Shinyzenith <[email protected]>
70a8b85
to
588e2d4
Compare
wayshot/src/wayshot.rs
Outdated
@@ -123,40 +124,42 @@ fn main() -> Result<()> { | |||
} | |||
}; | |||
|
|||
let mut buffer = Cursor::new(Vec::new()); |
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.
Lets only create the buffer if clipboard is specified. Else this leads to an extra allocation which we can skip. Lets just call image.save here and create the buffer in the clipboard conditional block.
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.
I considered that but the stdout block also uses the buffer and I didn't want it to do the buffer write twice in the case of --clipboard + stdout. Should I create the buffer there too?
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.
Maybe a function can help
wayshot/src/wayshot.rs
Outdated
} | ||
} | ||
|
||
if !stdout_print && cli.clipboard { | ||
let mut buffer = Cursor::new(Vec::new()); |
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.
Do we need the cli.clipboard conditional in the stdout conditional? Is this block outside of the conditional not enough?
let image_buf: Option<Cursor<Vec<...> = None;
if Some(path) = file {
/// save
} else {
/// to stdout
image_buf = ...;
}
if cli.clipboard && image_buf = Some(image_buffer) {
/// Daemonize / create buf if needed
}
image_buffer.write_to(&mut buffer, requested_encoding)?; | ||
if cli.clipboard { | ||
let buffer = match image_buf { | ||
Some(buf) => buf, |
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.
Sorry for the last and final nitpick, just do the match inside of the clipboard daemonize call
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.
Done!
fix: Clipboard flag ignored if path is qualified to file (waycrate#108)
No description provided.