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

refac: use BStr to display possibly non-UTF8 byte sequences #332

Merged
merged 2 commits into from
Jan 5, 2023
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
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));
figsoda marked this conversation as resolved.
Show resolved Hide resolved

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