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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Mar 26, 2024
Merged

Fix Issue #106: Clipboard flag ignored if path is qualified to file #108

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ca315d8
feat(clipboard): fix issue #106 clipboard flag ignored if path is qua…
CheerfulPianissimo Mar 23, 2024
70976dc
feat(clipboard): clarify flag description of --clipboard
CheerfulPianissimo Mar 23, 2024
588e2d4
feat(clipboard): enable multiline comment description for --clipboard…
CheerfulPianissimo Mar 23, 2024
fae2bc5
Merge branch 'freeze-feat-andreas' into fix-clipboard-independence
Shinyzenith Mar 24, 2024
fede62e
feat(clipboard): reduce buffer allocations for encoding image
CheerfulPianissimo Mar 24, 2024
7dcfaab
feat(clipboard): improve code quality
CheerfulPianissimo Mar 24, 2024
ff8f394
feat(clipboard): code style change: perform match inside function call
CheerfulPianissimo Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions wayshot/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ pub struct Cli {
#[arg(value_name = "OUTPUT")]
pub file: Option<PathBuf>,

/// Copy image to clipboard along with [OUTPUT] or stdout. Wayshot persists in the background to offer the image till the clipboard is overwritten.
#[arg(long)]
/// Copy image to clipboard. Can be used simultaneously with [OUTPUT] or stdout.
/// Wayshot persists in the background offering the image till the clipboard is overwritten.
#[arg(long, verbatim_doc_comment)]
pub clipboard: bool,

/// Log level to be used for printing to stderr
Expand Down
57 changes: 30 additions & 27 deletions wayshot/src/wayshot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::{
fs::File,
io::{stdout, BufWriter, Cursor, Write},
process::Command,
};
Expand Down Expand Up @@ -123,40 +124,42 @@ fn main() -> Result<()> {
}
};

let mut buffer = Cursor::new(Vec::new());
Copy link
Member

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.

Copy link
Author

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?

Copy link
Member

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

image_buffer.write_to(&mut buffer, requested_encoding)?;

if let Some(file) = file {
image_buffer.save(file)?;
let mut file = File::create(file)?;
file.write_all(buffer.get_ref())?;
} else {
let mut buffer = Cursor::new(Vec::new());
image_buffer.write_to(&mut buffer, requested_encoding)?;

if stdout_print {
let stdout = stdout();
let mut writer = BufWriter::new(stdout.lock());
writer.write_all(buffer.get_ref())?;
}
if cli.clipboard {
let mut opts = Options::new();
match unsafe { fork() } {
// Having the image persistently available on the clipboard requires a wayshot process to be alive.
// Fork the process with a child detached from the main process and have the parent exit
Ok(ForkResult::Parent { .. }) => {
return Ok(());
}
Ok(ForkResult::Child) => {
opts.foreground(true); // Offer the image till something else is available on the clipboard
opts.copy(
Source::Bytes(buffer.into_inner().into()),
MimeType::Autodetect,
)?;
}
Err(e) => {
tracing::warn!("Fork failed with error: {e}, couldn't offer image on the clipboard persistently.
Use a clipboard manager to record screenshot.");
opts.copy(
Source::Bytes(buffer.into_inner().into()),
MimeType::Autodetect,
)?;
}
}

if cli.clipboard {
let mut opts = Options::new();
match unsafe { fork() } {
// Having the image persistently available on the clipboard requires a wayshot process to be alive.
// Fork the process with a child detached from the main process and have the parent exit
Ok(ForkResult::Parent { .. }) => {
return Ok(());
}
Ok(ForkResult::Child) => {
opts.foreground(true); // Offer the image till something else is available on the clipboard
opts.copy(
Source::Bytes(buffer.into_inner().into()),
MimeType::Autodetect,
)?;
}
Err(e) => {
tracing::warn!("Fork failed with error: {e}, couldn't offer image on the clipboard persistently.
Use a clipboard manager to record screenshot.");
opts.copy(
Source::Bytes(buffer.into_inner().into()),
MimeType::Autodetect,
)?;
}
}
}
Expand Down