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

src/commands.rs

@@ -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.")
@@ -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;
@@ -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?;
@@ -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))

src/utils/formatting.rs

@@ -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
@@ -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)
     }
 }
 

src/utils/question.rs

@@ -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)
         }
@@ -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)
         }