diff --git a/src/archive/tar.rs b/src/archive/tar.rs index a42108c..c3ae74c 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -38,7 +38,11 @@ pub fn unpack_archive( file.unpack_in(output_folder)?; - info!("{:?} extracted. ({})", output_folder.join(file.path()?), Bytes::new(file.size())); + // This is printed for every file in the archive and has little + // importance for most users, but would generate lots of + // spoken text for users using screen readers, braille displays + // and so on + info!(inaccessible, "{:?} extracted. ({})", output_folder.join(file.path()?), Bytes::new(file.size())); files_unpacked.push(file_path); } @@ -80,7 +84,11 @@ where let entry = entry?; let path = entry.path(); - info!("Compressing '{}'.", utils::to_utf(path)); + // This is printed for every file in `input_filenames` and has + // little importance for most users, but would generate lots of + // spoken text for users using screen readers, braille displays + // and so on + info!(inaccessible, "Compressing '{}'.", utils::to_utf(path)); if path.is_dir() { builder.append_dir(path, path)?; diff --git a/src/archive/zip.rs b/src/archive/zip.rs index 38924f0..027efbc 100644 --- a/src/archive/zip.rs +++ b/src/archive/zip.rs @@ -48,7 +48,11 @@ where match (&*file.name()).ends_with('/') { _is_dir @ true => { - info!("File {} extracted to \"{}\"", idx, file_path.display()); + // This is printed for every file in the archive and has little + // importance for most users, but would generate lots of + // spoken text for users using screen readers, braille displays + // and so on + info!(inaccessible, "File {} extracted to \"{}\"", idx, file_path.display()); fs::create_dir_all(&file_path)?; } _is_file @ false => { @@ -59,7 +63,8 @@ where } let file_path = strip_cur_dir(file_path.as_path()); - info!("{:?} extracted. ({})", file_path.display(), Bytes::new(file.size())); + // same reason is in _is_dir: long, often not needed text + info!(inaccessible, "{:?} extracted. ({})", file_path.display(), Bytes::new(file.size())); let mut output_file = fs::File::create(&file_path)?; io::copy(&mut file, &mut output_file)?; @@ -125,7 +130,11 @@ where let entry = entry?; let path = entry.path(); - info!("Compressing '{}'.", to_utf(path)); + // This is printed for every file in `input_filenames` and has + // little importance for most users, but would generate lots of + // spoken text for users using screen readers, braille displays + // and so on + info!(inaccessible, "Compressing '{}'.", to_utf(path)); if path.is_dir() { if dir_is_empty(path) { @@ -149,7 +158,8 @@ where fn check_for_comments(file: &ZipFile) { let comment = file.comment(); if !comment.is_empty() { - info!("Found comment in {}: {}", file.name(), comment); + // TODO: is this important? + info!(inaccessible, "Found comment in {}: {}", file.name(), comment); } } diff --git a/src/commands.rs b/src/commands.rs index ad3ed41..2da6d2b 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -139,7 +139,7 @@ pub fn run(args: Opts, question_policy: QuestionPolicy) -> crate::Result<()> { // Path::extension says: "if there is no file_name, then there is no extension". // Contrapositive statement: "if there is extension, then there is file_name". info!( - a11y_show, + accessible, // important information "Partial compression detected. Compressing {} into {}", to_utf(files[0].as_path().file_name().unwrap()), to_utf(&output_path) @@ -160,7 +160,11 @@ pub fn run(args: Opts, question_policy: QuestionPolicy) -> crate::Result<()> { eprintln!(" Error:{reset} {}{red}.{reset}\n", err, reset = *colors::RESET, red = *colors::RED); } } else { - info!(a11y_show, "Successfully compressed '{}'.", to_utf(output_path)); + // this is only printed once, so it doesn't result in much text. On the other hand, + // 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)); } compress_result?; @@ -350,7 +354,11 @@ fn decompress_file( utils::create_dir_if_non_existent(output_dir)?; let zip_archive = zip::ZipArchive::new(reader)?; let _files = crate::archive::zip::unpack_archive(zip_archive, output_dir, question_policy)?; - info!(a11y_show, "Successfully decompressed archive in {}.", nice_directory_display(output_dir)); + // this is only printed once, so it doesn't result in much text. On the other hand, + // 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 decompressed archive in {}.", nice_directory_display(output_dir)); return Ok(()); } @@ -416,8 +424,12 @@ fn decompress_file( } } - info!(a11y_show, "Successfully decompressed archive in {}.", nice_directory_display(output_dir)); - info!(a11y_show, "Files unpacked: {}", files_unpacked.len()); + // this is only printed once, so it doesn't result in much text. On the other hand, + // 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 decompressed archive in {}.", nice_directory_display(output_dir)); + info!(accessible, "Files unpacked: {}", files_unpacked.len()); Ok(()) } @@ -498,7 +510,9 @@ fn check_mime_type( // File with no extension // Try to detect it automatically and prompt the user about it if let Some(detected_format) = try_infer_extension(path) { - info!(a11y_show, "Detected file: `{}` extension as `{}`", path.display(), detected_format); + // Infering the file extension can have unpredicted consequences (e.g. the user just + // mistyped, ...) which we should always inform the user about. + info!(accessible, "Detected file: `{}` extension as `{}`", path.display(), detected_format); if user_wants_to_continue_decompressing(path, question_policy)? { format.push(detected_format); } else { @@ -522,7 +536,7 @@ fn check_mime_type( } else { // NOTE: If this actually produces no false positives, we can upgrade it in the future // to a warning and ask the user if he wants to continue decompressing. - info!(a11y_show, "Could not detect the extension of `{}`", path.display()); + info!(accessible, "Could not detect the extension of `{}`", path.display()); } } Ok(ControlFlow::Continue(())) diff --git a/src/error.rs b/src/error.rs index d68fdaf..550def2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -49,6 +49,8 @@ pub struct FinalError { impl Display for FinalError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { // Title + // + // When in ACCESSIBLE mode, the square brackets are suppressed if *crate::cli::ACCESSIBLE.get().unwrap_or(&false) { write!(f, "{}ERROR{}: {}", *RED, *RESET, self.title)?; } else { @@ -64,9 +66,18 @@ impl Display for FinalError { if !self.hints.is_empty() { // Separate by one blank line. writeln!(f)?; - for hint in &self.hints { - write!(f, "\n{}hint:{} {}", *GREEN, *RESET, hint)?; - } + // to reduce redundant output for text-to-speach systems, braille + // displays and so on, only print "hints" once in ACCESSIBLE mode + if *crate::cli::ACCESSIBLE.get().unwrap_or(&false) { + write!(f, "\n{}hints:{}", *GREEN, *RESET)?; + for hint in &self.hints { + write!(f, "\n{}", hint)?; + } + } else { + for hint in &self.hints { + write!(f, "\n{}hint:{} {}", *GREEN, *RESET, hint)?; + } + } } Ok(()) diff --git a/src/macros.rs b/src/macros.rs index 755987d..0826a64 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -2,20 +2,32 @@ /// Macro that prints \[INFO\] messages, wraps [`println`]. /// -/// Normally, this prints nothing if ACCESSIBLE mode is turned on, -/// except when called as `info!(a11y_show, "..", ..)` +/// There are essentially two different versions of the `info!()` macro: +/// - `info!(accessible, ...)` should only be used for short, important +/// information which is expected to be useful for e.g. blind users whose +/// text-to-speach systems read out every output line, which is why we +/// should reduce nonessential output to a minimum when running in +/// ACCESSIBLE mode +/// - `info!(inaccessible, ...)` can be used more carelessly / for less +/// important information. A seeing user can easily skim through more lines +/// of output, so e.g. reporting every single processed file can be helpful, +/// while it would generate long and hard to navigate text for blind people +/// who have to have each line of output read to them aloud, whithout to +/// ability to skip some lines deemed not important like a seeing person would. #[macro_export] macro_rules! info { + // Accessible (short/important) info message. // Show info message even in ACCESSIBLE mode - (a11y_show, $($arg:tt)*) => { + (accessible, $($arg:tt)*) => { // if in ACCESSIBLE mode, suppress the "[INFO]" and just print the message if (!$crate::cli::ACCESSIBLE.get().unwrap()) { $crate::macros::_info_helper(); } println!($($arg)*); }; + // Inccessible (long/no important) info message. // Print info message if ACCESSIBLE is not turned on - ($($arg:tt)*) => { + (inaccessible, $($arg:tt)*) => { if (!$crate::cli::ACCESSIBLE.get().unwrap()) { $crate::macros::_info_helper(); println!($($arg)*); diff --git a/src/utils/fs.rs b/src/utils/fs.rs index f1d14ab..7f39032 100644 --- a/src/utils/fs.rs +++ b/src/utils/fs.rs @@ -42,7 +42,9 @@ pub fn clear_path(path: &Path, question_policy: QuestionPolicy) -> crate::Result pub fn create_dir_if_non_existent(path: &Path) -> crate::Result<()> { if !path.exists() { fs::create_dir_all(path)?; - info!("directory {} created.", to_utf(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)); } Ok(()) }