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

feat: don't allocate when possible in to_utf, nice_directory_display #249

Merged
merged 1 commit into from
Feb 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 7 additions & 7 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,16 @@ pub fn run(
// To file.tar.bz.xz
let extensions_text: String = formats.iter().map(|format| format.to_string()).collect();

let output_path = to_utf(output_path);
let output_path = to_utf(&output_path).to_string();

// Breaks if Lzma is .lz or .lzma and not .xz
// Or if Bzip is .bz2 and not .bz
let extensions_start_position = output_path.rfind(&extensions_text).unwrap();
let pos = extensions_start_position - 1;
let mut suggested_output_path = output_path.clone();
let mut suggested_output_path = output_path.to_string();
suggested_output_path.insert_str(pos, ".tar");

let error = FinalError::with_title(format!("Cannot compress to '{}'.", to_utf(&output_path)))
let error = FinalError::with_title(format!("Cannot compress to '{}'.", output_path))
.detail("You are trying to compress multiple files.")
.detail(format!("The compression format '{}' cannot receive multiple files.", &formats[0]))
.detail("The only supported formats that archive files into an archive are .tar and .zip.")
Expand Down Expand Up @@ -154,7 +154,7 @@ pub fn run(
info!(
accessible, // important information
"Partial compression detected. Compressing {} into {}",
to_utf(files[0].as_path().file_name().unwrap()),
to_utf(files[0].as_path().file_name().unwrap().as_ref()),
to_utf(&output_path)
);
formats = new_formats;
Expand All @@ -178,7 +178,7 @@ pub fn run(
// having a final status message is important especially in an accessibility context
// as screen readers may not read a commands exit code, making it hard to reason
// about whether the command succeeded without such a message
info!(accessible, "Successfully compressed '{}'.", to_utf(output_path));
info!(accessible, "Successfully compressed '{}'.", to_utf(&output_path));
}

compress_result?;
Expand Down Expand Up @@ -698,8 +698,8 @@ fn smart_unpack(
info!(
accessible,
"Successfully moved {} to {}.",
nice_directory_display(&temp_dir_path),
nice_directory_display(&output_file_path)
nice_directory_display(temp_dir_path),
nice_directory_display(output_file_path)
);
}
Ok(ControlFlow::Continue(files))
Expand Down
31 changes: 20 additions & 11 deletions src/utils/formatting.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
use std::{borrow::Cow, cmp, ffi::OsStr, path::Path};
use std::{borrow::Cow, cmp, path::Path};

// The lifetimes here could elided but I think they help
// comprehension in this case
#[allow(clippy::needless_lifetimes)]
/// Converts an OsStr to utf8 with custom formatting.
///
/// This is different from [`Path::display`].
///
/// See <https://gist.github.com/marcospb19/ebce5572be26397cf08bbd0fd3b65ac1> for a comparison.
pub fn to_utf(os_str: impl AsRef<OsStr>) -> String {
let text = format!("{:?}", os_str.as_ref());
text.trim_matches('"').to_string()
pub fn to_utf<'a>(os_str: &'a Path) -> Cow<'a, str> {
let format = || {
let text = format!("{:?}", os_str);
Cow::Owned(text.trim_matches('"').to_string())
};

os_str.to_str().map_or_else(format, Cow::Borrowed)
}

/// Removes the current dir from the beginning of a path
Expand All @@ -21,25 +28,27 @@ pub fn strip_cur_dir(source_path: &Path) -> &Path {
/// Converts a slice of AsRef<OsStr> to comma separated String
///
/// Panics if the slice is empty.
pub fn concatenate_os_str_list(os_strs: &[impl AsRef<OsStr>]) -> String {
pub fn concatenate_os_str_list(os_strs: &[impl AsRef<Path>]) -> String {
let mut iter = os_strs.iter().map(AsRef::as_ref);

let mut string = to_utf(iter.next().unwrap()); // May panic
let mut string = to_utf(iter.next().unwrap()).to_string(); // May panic

for os_str in iter {
string += ", ";
string += &to_utf(os_str);
string += &*to_utf(os_str);
}
string
}

// The lifetimes here could elided but I think they help
// comprehension in this case
#[allow(clippy::needless_lifetimes)]
/// Display the directory name, but change to "current directory" when necessary.
pub fn nice_directory_display(os_str: impl AsRef<OsStr>) -> Cow<'static, str> {
if os_str.as_ref() == "." {
pub fn nice_directory_display<'a>(path: &'a Path) -> Cow<'a, str> {
if path == Path::new(".") {
Cow::Borrowed("current directory")
} else {
let text = to_utf(os_str);
Cow::Owned(format!("'{}'", text))
to_utf(path)
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/utils/question.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub fn user_wants_to_overwrite(path: &Path, question_policy: QuestionPolicy) ->
QuestionPolicy::AlwaysNo => Ok(false),
QuestionPolicy::Ask => {
let path = to_utf(strip_cur_dir(path));
let path = Some(path.as_str());
let path = Some(&*path);
let placeholder = Some("FILE");
Confirmation::new("Do you want to overwrite 'FILE'?", placeholder).ask(path)
}
Expand Down Expand Up @@ -87,7 +87,7 @@ pub fn user_wants_to_continue(
QuestionAction::Decompression => "decompressing",
};
let path = to_utf(strip_cur_dir(path));
let path = Some(path.as_str());
let path = Some(&*path);
let placeholder = Some("FILE");
Confirmation::new(&format!("Do you want to continue {} 'FILE'?", action), placeholder).ask(path)
}
Expand Down