Skip to content

Commit

Permalink
feat: don't allocate when possible in to_utf, `nice_directory_displ…
Browse files Browse the repository at this point in the history
…ay` (#249)
  • Loading branch information
vrmiguel authored Feb 5, 2022
1 parent 336fc97 commit 8b1cd18
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 20 deletions.
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

0 comments on commit 8b1cd18

Please sign in to comment.