Improve accessibility code and comment decisions why some info is (in)accessible

This commit is contained in:
Anton Hermann 2021-11-23 11:53:17 +01:00
parent 34cbe5746d
commit 1030eb0de9
6 changed files with 78 additions and 21 deletions

View File

@ -38,7 +38,11 @@ pub fn unpack_archive(
file.unpack_in(output_folder)?; 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); files_unpacked.push(file_path);
} }
@ -80,7 +84,11 @@ where
let entry = entry?; let entry = entry?;
let path = entry.path(); 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() { if path.is_dir() {
builder.append_dir(path, path)?; builder.append_dir(path, path)?;

View File

@ -48,7 +48,11 @@ where
match (&*file.name()).ends_with('/') { match (&*file.name()).ends_with('/') {
_is_dir @ true => { _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)?; fs::create_dir_all(&file_path)?;
} }
_is_file @ false => { _is_file @ false => {
@ -59,7 +63,8 @@ where
} }
let file_path = strip_cur_dir(file_path.as_path()); 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)?; let mut output_file = fs::File::create(&file_path)?;
io::copy(&mut file, &mut output_file)?; io::copy(&mut file, &mut output_file)?;
@ -125,7 +130,11 @@ where
let entry = entry?; let entry = entry?;
let path = entry.path(); 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 path.is_dir() {
if dir_is_empty(path) { if dir_is_empty(path) {
@ -149,7 +158,8 @@ where
fn check_for_comments(file: &ZipFile) { fn check_for_comments(file: &ZipFile) {
let comment = file.comment(); let comment = file.comment();
if !comment.is_empty() { if !comment.is_empty() {
info!("Found comment in {}: {}", file.name(), comment); // TODO: is this important?
info!(inaccessible, "Found comment in {}: {}", file.name(), comment);
} }
} }

View File

@ -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". // 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". // Contrapositive statement: "if there is extension, then there is file_name".
info!( info!(
a11y_show, accessible, // important information
"Partial compression detected. Compressing {} into {}", "Partial compression detected. Compressing {} into {}",
to_utf(files[0].as_path().file_name().unwrap()), to_utf(files[0].as_path().file_name().unwrap()),
to_utf(&output_path) 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); eprintln!(" Error:{reset} {}{red}.{reset}\n", err, reset = *colors::RESET, red = *colors::RED);
} }
} else { } 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?; compress_result?;
@ -350,7 +354,11 @@ fn decompress_file(
utils::create_dir_if_non_existent(output_dir)?; utils::create_dir_if_non_existent(output_dir)?;
let zip_archive = zip::ZipArchive::new(reader)?; let zip_archive = zip::ZipArchive::new(reader)?;
let _files = crate::archive::zip::unpack_archive(zip_archive, output_dir, question_policy)?; 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(()); return Ok(());
} }
@ -416,8 +424,12 @@ fn decompress_file(
} }
} }
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,
info!(a11y_show, "Files unpacked: {}", files_unpacked.len()); // 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(()) Ok(())
} }
@ -498,7 +510,9 @@ fn check_mime_type(
// File with no extension // File with no extension
// Try to detect it automatically and prompt the user about it // Try to detect it automatically and prompt the user about it
if let Some(detected_format) = try_infer_extension(path) { 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)? { if user_wants_to_continue_decompressing(path, question_policy)? {
format.push(detected_format); format.push(detected_format);
} else { } else {
@ -522,7 +536,7 @@ fn check_mime_type(
} else { } else {
// NOTE: If this actually produces no false positives, we can upgrade it in the future // 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. // 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(())) Ok(ControlFlow::Continue(()))

View File

@ -49,6 +49,8 @@ pub struct FinalError {
impl Display for FinalError { impl Display for FinalError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// Title // Title
//
// When in ACCESSIBLE mode, the square brackets are suppressed
if *crate::cli::ACCESSIBLE.get().unwrap_or(&false) { if *crate::cli::ACCESSIBLE.get().unwrap_or(&false) {
write!(f, "{}ERROR{}: {}", *RED, *RESET, self.title)?; write!(f, "{}ERROR{}: {}", *RED, *RESET, self.title)?;
} else { } else {
@ -64,9 +66,18 @@ impl Display for FinalError {
if !self.hints.is_empty() { if !self.hints.is_empty() {
// Separate by one blank line. // Separate by one blank line.
writeln!(f)?; writeln!(f)?;
for hint in &self.hints { // to reduce redundant output for text-to-speach systems, braille
write!(f, "\n{}hint:{} {}", *GREEN, *RESET, hint)?; // 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(()) Ok(())

View File

@ -2,20 +2,32 @@
/// Macro that prints \[INFO\] messages, wraps [`println`]. /// Macro that prints \[INFO\] messages, wraps [`println`].
/// ///
/// Normally, this prints nothing if ACCESSIBLE mode is turned on, /// There are essentially two different versions of the `info!()` macro:
/// except when called as `info!(a11y_show, "..", ..)` /// - `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_export]
macro_rules! info { macro_rules! info {
// Accessible (short/important) info message.
// Show info message even in ACCESSIBLE mode // 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 in ACCESSIBLE mode, suppress the "[INFO]" and just print the message
if (!$crate::cli::ACCESSIBLE.get().unwrap()) { if (!$crate::cli::ACCESSIBLE.get().unwrap()) {
$crate::macros::_info_helper(); $crate::macros::_info_helper();
} }
println!($($arg)*); println!($($arg)*);
}; };
// Inccessible (long/no important) info message.
// Print info message if ACCESSIBLE is not turned on // Print info message if ACCESSIBLE is not turned on
($($arg:tt)*) => { (inaccessible, $($arg:tt)*) => {
if (!$crate::cli::ACCESSIBLE.get().unwrap()) { if (!$crate::cli::ACCESSIBLE.get().unwrap()) {
$crate::macros::_info_helper(); $crate::macros::_info_helper();
println!($($arg)*); println!($($arg)*);

View File

@ -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<()> { pub fn create_dir_if_non_existent(path: &Path) -> crate::Result<()> {
if !path.exists() { if !path.exists() {
fs::create_dir_all(path)?; 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(()) Ok(())
} }