Skip to content

Commit

Permalink
Merge pull request #332 from ouch-org/bstr-path-display
Browse files Browse the repository at this point in the history
refac: use BStr to display possibly non-UTF8 byte sequences
  • Loading branch information
figsoda authored Jan 5, 2023
2 parents 9854285 + 2caeb10 commit 9fdc7ed
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 30 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ description = "A command-line utility for easily compressing and decompressing f

[dependencies]
atty = "0.2.14"
bstr = "1.1.0"
bstr = { version = "1.1.0", default-features = false, features = ["std"] }
bzip2 = "0.4.3"
clap = { version = "4.0.32", features = ["derive", "env"] }
filetime = "0.2.19"
Expand Down
4 changes: 2 additions & 2 deletions src/archive/tar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::{
error::FinalError,
info,
list::FileInArchive,
utils::{self, FileVisibilityPolicy},
utils::{self, EscapedPathDisplay, FileVisibilityPolicy},
warning,
};

Expand Down Expand Up @@ -121,7 +121,7 @@ where
// spoken text for users using screen readers, braille displays
// and so on
if !quiet {
info!(inaccessible, "Compressing '{}'.", utils::to_utf(path));
info!(inaccessible, "Compressing '{}'.", EscapedPathDisplay::new(path));
}

if path.is_dir() {
Expand Down
6 changes: 3 additions & 3 deletions src/archive/zip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ use crate::{
info,
list::FileInArchive,
utils::{
self, cd_into_same_dir_as, get_invalid_utf8_paths, pretty_format_list_of_paths, strip_cur_dir, to_utf,
FileVisibilityPolicy,
self, cd_into_same_dir_as, get_invalid_utf8_paths, pretty_format_list_of_paths, strip_cur_dir,
EscapedPathDisplay, FileVisibilityPolicy,
},
warning,
};
Expand Down Expand Up @@ -191,7 +191,7 @@ where
// spoken text for users using screen readers, braille displays
// and so on
if !quiet {
info!(inaccessible, "Compressing '{}'.", to_utf(path));
info!(inaccessible, "Compressing '{}'.", EscapedPathDisplay::new(path));
}

let metadata = match path.metadata() {
Expand Down
45 changes: 26 additions & 19 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ use crate::{
info,
list::ListOptions,
utils::{
self, pretty_format_list_of_paths, to_utf, try_infer_extension, user_wants_to_continue, FileVisibilityPolicy,
self, pretty_format_list_of_paths, to_utf, try_infer_extension, user_wants_to_continue, EscapedPathDisplay,
FileVisibilityPolicy,
},
warning, Opts, QuestionAction, QuestionPolicy, Subcommand,
};
Expand Down Expand Up @@ -115,7 +116,7 @@ pub fn run(
let formats = extension::extensions_from_path(&output_path);

let first_format = formats.first().ok_or_else(|| {
let output_path = to_utf(&output_path);
let output_path = EscapedPathDisplay::new(&output_path);
FinalError::with_title(format!("Cannot compress to '{output_path}'."))
.detail("You shall supply the compression format")
.hint("Try adding supported extensions (see --help):")
Expand All @@ -138,7 +139,7 @@ pub fn run(
// To file.tar.bz.xz
let suggested_output_path = build_archive_file_suggestion(&output_path, ".tar")
.expect("output path should contain a compression format");
let output_path = to_utf(&output_path);
let output_path = EscapedPathDisplay::new(&output_path);
let first_detail_message = if is_multiple_inputs {
"You are trying to compress multiple files."
} else {
Expand All @@ -160,21 +161,24 @@ pub fn run(
}

if let Some(format) = formats.iter().skip(1).find(|format| format.is_archive()) {
let error = FinalError::with_title(format!("Cannot compress to '{}'.", to_utf(&output_path)))
.detail(format!("Found the format '{}' in an incorrect position.", format))
.detail(format!(
"'{}' can only be used at the start of the file extension.",
format
))
.hint(format!(
"If you wish to compress multiple files, start the extension with '{}'.",
format
))
.hint(format!(
"Otherwise, remove the last '{}' from '{}'.",
format,
to_utf(&output_path)
));
let error = FinalError::with_title(format!(
"Cannot compress to '{}'.",
EscapedPathDisplay::new(&output_path)
))
.detail(format!("Found the format '{}' in an incorrect position.", format))
.detail(format!(
"'{}' can only be used at the start of the file extension.",
format
))
.hint(format!(
"If you wish to compress multiple files, start the extension with '{}'.",
format
))
.hint(format!(
"Otherwise, remove the last '{}' from '{}'.",
format,
EscapedPathDisplay::new(&output_path)
));

return Err(error.into());
}
Expand Down Expand Up @@ -207,7 +211,10 @@ pub fn run(
// out that we left a possibly CORRUPTED file at `output_path`
if utils::remove_file_or_dir(&output_path).is_err() {
eprintln!("{red}FATAL ERROR:\n", red = *colors::RED);
eprintln!(" Ouch failed to delete the file '{}'.", to_utf(&output_path));
eprintln!(
" Ouch failed to delete the file '{}'.",
EscapedPathDisplay::new(&output_path)
);
eprintln!(" Please delete it manually.");
eprintln!(" This file is corrupted if compression didn't finished.");

Expand Down
40 changes: 39 additions & 1 deletion src/utils/formatting.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,45 @@
use std::{borrow::Cow, path::Path};
use std::{borrow::Cow, fmt::Display, path::Path};

use crate::CURRENT_DIRECTORY;

/// Converts invalid UTF-8 bytes to the Unicode replacement codepoint (�) in its Display implementation.
pub struct EscapedPathDisplay<'a> {
path: &'a Path,
}

impl<'a> EscapedPathDisplay<'a> {
pub fn new(path: &'a Path) -> Self {
Self { path }
}
}

#[cfg(unix)]
impl Display for EscapedPathDisplay<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
use std::os::unix::prelude::OsStrExt;

let bstr = bstr::BStr::new(self.path.as_os_str().as_bytes());

write!(f, "{}", bstr)
}
}

#[cfg(windows)]
impl Display for EscapedPathDisplay<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
use std::{char, fmt::Write, os::windows::prelude::OsStrExt};

let utf16 = self.path.as_os_str().encode_wide();
let chars = char::decode_utf16(utf16).map(|decoded| decoded.unwrap_or(char::REPLACEMENT_CHARACTER));

for char in chars {
f.write_char(char)?;
}

Ok(())
}
}

/// Converts an OsStr to utf8 with custom formatting.
///
/// This is different from [`Path::display`].
Expand Down
6 changes: 3 additions & 3 deletions src/utils/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use std::{

use fs_err as fs;

use super::{to_utf, user_wants_to_overwrite};
use crate::{extension::Extension, info, QuestionPolicy};
use super::user_wants_to_overwrite;
use crate::{extension::Extension, info, utils::EscapedPathDisplay, QuestionPolicy};

/// Remove `path` asking the user to overwrite if necessary.
///
Expand Down Expand Up @@ -41,7 +41,7 @@ pub fn create_dir_if_non_existent(path: &Path) -> crate::Result<()> {
fs::create_dir_all(path)?;
// creating a directory is an important change to the file system we
// should always inform the user about
info!(accessible, "directory {} created.", to_utf(path));
info!(accessible, "directory {} created.", EscapedPathDisplay::new(path));
}
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ mod fs;
mod question;

pub use file_visibility::FileVisibilityPolicy;
pub use formatting::{nice_directory_display, pretty_format_list_of_paths, strip_cur_dir, to_utf};
pub use formatting::{nice_directory_display, pretty_format_list_of_paths, strip_cur_dir, to_utf, EscapedPathDisplay};
pub use fs::{
cd_into_same_dir_as, clear_path, create_dir_if_non_existent, is_symlink, remove_file_or_dir, try_infer_extension,
};
Expand Down

0 comments on commit 9fdc7ed

Please sign in to comment.