From 26301d05f66cfa8f0a508973bb18a914eadc3d27 Mon Sep 17 00:00:00 2001 From: snapdgn Date: Sun, 11 Sep 2022 15:36:18 +0530 Subject: [PATCH 1/6] refactor: declarative macros to rust --- Cargo.lock | 2 +- src/uu/stat/src/stat.rs | 108 +++++++++++++++++++++------------------- 2 files changed, 57 insertions(+), 53 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7d9995659..3b42eb9dc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2624,7 +2624,7 @@ dependencies = [ "itertools", "quick-error", "regex", - "time 0.3.14", + "time", "uucore", ] diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index 2daa36dae..e4658253d 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -9,7 +9,7 @@ extern crate uucore; use clap::builder::ValueParser; use uucore::display::Quotable; -use uucore::error::{FromIo, UResult, USimpleError}; +use uucore::error::{FromIo, UError, UResult, USimpleError}; use uucore::fs::display_permissions; use uucore::fsext::{ pretty_filetype, pretty_fstype, pretty_time, read_fs_list, statfs, BirthTime, FsMeta, @@ -26,57 +26,61 @@ use std::os::unix::prelude::OsStrExt; use std::path::Path; use std::{cmp, fs, iter}; -macro_rules! check_bound { - ($str: ident, $bound:expr, $beg: expr, $end: expr) => { - if $end >= $bound { - return Err(USimpleError::new( - 1, - format!("{}: invalid directive", $str[$beg..$end].quote()), - )); - } - }; +fn check_bound(slice: &str, bound: usize, beg: usize, end: usize) -> Result<(), Box> { + if end >= bound { + return Err(USimpleError::new( + 1, + format!("{}: invalid directive", slice[beg..end].quote()), + )); + } + Ok(()) } -macro_rules! fill_string { - ($str: ident, $c: expr, $cnt: expr) => { - iter::repeat($c) - .take($cnt) - .map(|c| $str.push(c)) - .all(|_| true) - }; + +fn fill_string(strs: &mut String, c: char, cnt: usize) { + iter::repeat(c) + .take(cnt) + .map(|c| strs.push(c)) + .all(|_| true); } -macro_rules! extend_digits { - ($str: expr, $min: expr) => { - if $min > $str.len() { - let mut pad = String::with_capacity($min); - fill_string!(pad, '0', $min - $str.len()); - pad.push_str($str); - pad.into() + +fn extend_digits(strs: &str, min: usize) -> Cow<'_, str> { + if min > strs.len() { + let mut pad = String::with_capacity(min); + fill_string(&mut pad, '0', min - strs.len()); + pad.push_str(strs); + return pad.into(); + } else { + return strs.into(); + } +} + +fn pad_and_print>( + result: &mut String, + strs: S, + left: bool, + width: usize, + padding: char, +) { + let strs_ref = strs.as_ref(); + if strs_ref.len() < width { + if left { + result.push_str(strs.as_ref()); + fill_string(result, padding, width - strs_ref.len()); } else { - $str.into() + fill_string(result, padding, width - strs_ref.len()); + result.push_str(strs.as_ref()); } - }; -} -macro_rules! pad_and_print { - ($result: ident, $str: ident, $left: expr, $width: expr, $padding: expr) => { - if $str.len() < $width { - if $left { - $result.push_str($str.as_ref()); - fill_string!($result, $padding, $width - $str.len()); - } else { - fill_string!($result, $padding, $width - $str.len()); - $result.push_str($str.as_ref()); - } - } else { - $result.push_str($str.as_ref()); - } - print!("{}", $result); - }; + } else { + result.push_str(strs.as_ref()); + } + print!("{}", result); } + macro_rules! print_adjusted { ($str: ident, $left: expr, $width: expr, $padding: expr) => { let field_width = cmp::max($width, $str.len()); let mut result = String::with_capacity(field_width); - pad_and_print!(result, $str, $left, field_width, $padding); + pad_and_print(&mut result, $str, $left, field_width, $padding); }; ($str: ident, $left: expr, $need_prefix: expr, $prefix: expr, $width: expr, $padding: expr) => { let mut field_width = cmp::max($width, $str.len()); @@ -85,7 +89,7 @@ macro_rules! print_adjusted { result.push_str($prefix); field_width -= $prefix.len(); } - pad_and_print!(result, $str, $left, field_width, $padding); + pad_and_print(&mut result, $str, $left, field_width, $padding); }; } @@ -306,7 +310,7 @@ fn print_it(arg: &str, output_type: &OutputType, flag: u8, width: usize, precisi Cow::Borrowed(arg) }; let min_digits = cmp::max(precision, arg.len() as i32) as usize; - let extended: Cow = extend_digits!(arg.as_ref(), min_digits); + let extended: Cow = extend_digits(arg.as_ref(), min_digits); print_adjusted!(extended, left_align, has_sign, prefix, width, padding_char); } OutputType::Unsigned => { @@ -316,12 +320,12 @@ fn print_it(arg: &str, output_type: &OutputType, flag: u8, width: usize, precisi Cow::Borrowed(arg) }; let min_digits = cmp::max(precision, arg.len() as i32) as usize; - let extended: Cow = extend_digits!(arg.as_ref(), min_digits); + let extended: Cow = extend_digits(arg.as_ref(), min_digits); print_adjusted!(extended, left_align, width, padding_char); } OutputType::UnsignedOct => { let min_digits = cmp::max(precision, arg.len() as i32) as usize; - let extended: Cow = extend_digits!(arg, min_digits); + let extended: Cow = extend_digits(arg, min_digits); print_adjusted!( extended, left_align, @@ -333,7 +337,7 @@ fn print_it(arg: &str, output_type: &OutputType, flag: u8, width: usize, precisi } OutputType::UnsignedHex => { let min_digits = cmp::max(precision, arg.len() as i32) as usize; - let extended: Cow = extend_digits!(arg, min_digits); + let extended: Cow = extend_digits(arg, min_digits); print_adjusted!( extended, left_align, @@ -384,7 +388,7 @@ impl Stater { } i += 1; } - check_bound!(format_str, bound, old, i); + check_bound(format_str, bound, old, i)?; let mut width = 0_usize; let mut precision = -1_i32; @@ -394,11 +398,11 @@ impl Stater { width = field_width; j += offset; } - check_bound!(format_str, bound, old, j); + check_bound(format_str, bound, old, j)?; if chars[j] == '.' { j += 1; - check_bound!(format_str, bound, old, j); + check_bound(format_str, bound, old, j)?; match format_str[j..].scan_num::() { Some((value, offset)) => { @@ -409,7 +413,7 @@ impl Stater { } None => precision = 0, } - check_bound!(format_str, bound, old, j); + check_bound(format_str, bound, old, j)?; } i = j; From 39e352e2af915cac8ef79c097447ed8356751411 Mon Sep 17 00:00:00 2001 From: snapdgn Date: Sun, 11 Sep 2022 20:55:37 +0530 Subject: [PATCH 2/6] fix: failing test cases & some refactoring --- src/uu/stat/src/stat.rs | 130 +++++++++++++++++++++------------------- 1 file changed, 68 insertions(+), 62 deletions(-) diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index e4658253d..b92ad764d 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -9,7 +9,7 @@ extern crate uucore; use clap::builder::ValueParser; use uucore::display::Quotable; -use uucore::error::{FromIo, UError, UResult, USimpleError}; +use uucore::error::{FromIo, UResult, USimpleError}; use uucore::fs::display_permissions; use uucore::fsext::{ pretty_filetype, pretty_fstype, pretty_time, read_fs_list, statfs, BirthTime, FsMeta, @@ -26,7 +26,7 @@ use std::os::unix::prelude::OsStrExt; use std::path::Path; use std::{cmp, fs, iter}; -fn check_bound(slice: &str, bound: usize, beg: usize, end: usize) -> Result<(), Box> { +fn check_bound(slice: &str, bound: usize, beg: usize, end: usize) -> UResult<()> { if end >= bound { return Err(USimpleError::new( 1, @@ -36,61 +36,53 @@ fn check_bound(slice: &str, bound: usize, beg: usize, end: usize) -> Result<(), Ok(()) } -fn fill_string(strs: &mut String, c: char, cnt: usize) { - iter::repeat(c) - .take(cnt) - .map(|c| strs.push(c)) - .all(|_| true); -} - -fn extend_digits(strs: &str, min: usize) -> Cow<'_, str> { - if min > strs.len() { +fn extend_digits(string: &str, min: usize) -> Cow<'_, str> { + if min > string.len() { let mut pad = String::with_capacity(min); - fill_string(&mut pad, '0', min - strs.len()); - pad.push_str(strs); - return pad.into(); + iter::repeat('0') + .take(min - string.len()) + .map(|_| pad.push('0')) + .all(|_| true); + pad.push_str(string); + pad.into() } else { - return strs.into(); + string.into() } } -fn pad_and_print>( - result: &mut String, - strs: S, +enum Padding { + Zero, + Space, +} + +fn pad_and_print(result: &str, left: bool, width: usize, padding: Padding) { + match (left, padding) { + (false, Padding::Zero) => print!("{result:0>width$}"), + (false, Padding::Space) => print!("{result:>width$}"), + (true, Padding::Zero) => print!("{result:0 print!("{result:, + prefix: Option<&str>, width: usize, - padding: char, + padding: Padding, ) { - let strs_ref = strs.as_ref(); - if strs_ref.len() < width { - if left { - result.push_str(strs.as_ref()); - fill_string(result, padding, width - strs_ref.len()); - } else { - fill_string(result, padding, width - strs_ref.len()); - result.push_str(strs.as_ref()); + let mut field_width = cmp::max(width, s.len()); + if let Some(p) = prefix { + if let Some(needprefix) = need_prefix { + if needprefix { + field_width -= p.len(); + } } + pad_and_print(s, left, field_width, padding); } else { - result.push_str(strs.as_ref()); + pad_and_print(s, left, field_width, padding); } - print!("{}", result); -} - -macro_rules! print_adjusted { - ($str: ident, $left: expr, $width: expr, $padding: expr) => { - let field_width = cmp::max($width, $str.len()); - let mut result = String::with_capacity(field_width); - pad_and_print(&mut result, $str, $left, field_width, $padding); - }; - ($str: ident, $left: expr, $need_prefix: expr, $prefix: expr, $width: expr, $padding: expr) => { - let mut field_width = cmp::max($width, $str.len()); - let mut result = String::with_capacity(field_width + $prefix.len()); - if $need_prefix { - result.push_str($prefix); - field_width -= $prefix.len(); - } - pad_and_print(&mut result, $str, $left, field_width, $padding); - }; } static ABOUT: &str = "Display file or file system status."; @@ -271,10 +263,10 @@ fn print_it(arg: &str, output_type: &OutputType, flag: u8, width: usize, precisi } let left_align = has!(flag, F_LEFT); - let padding_char = if has!(flag, F_ZERO) && !left_align && precision == -1 { - '0' + let padding_char: Padding = if has!(flag, F_ZERO) && !left_align && precision == -1 { + Padding::Zero } else { - ' ' + Padding::Space }; let has_sign = has!(flag, F_SIGN) || has!(flag, F_SPACE); @@ -301,7 +293,7 @@ fn print_it(arg: &str, output_type: &OutputType, flag: u8, width: usize, precisi } else { arg }; - print_adjusted!(s, left_align, width, ' '); + print_adjusted(s, left_align, None, None, width, Padding::Space); } OutputType::Integer => { let arg = if has!(flag, F_GROUP) { @@ -311,7 +303,14 @@ fn print_it(arg: &str, output_type: &OutputType, flag: u8, width: usize, precisi }; let min_digits = cmp::max(precision, arg.len() as i32) as usize; let extended: Cow = extend_digits(arg.as_ref(), min_digits); - print_adjusted!(extended, left_align, has_sign, prefix, width, padding_char); + print_adjusted( + extended.as_ref(), + left_align, + Some(has_sign), + Some(prefix), + width, + padding_char, + ); } OutputType::Unsigned => { let arg = if has!(flag, F_GROUP) { @@ -321,30 +320,37 @@ fn print_it(arg: &str, output_type: &OutputType, flag: u8, width: usize, precisi }; let min_digits = cmp::max(precision, arg.len() as i32) as usize; let extended: Cow = extend_digits(arg.as_ref(), min_digits); - print_adjusted!(extended, left_align, width, padding_char); + print_adjusted( + extended.as_ref(), + left_align, + None, + None, + width, + padding_char, + ); } OutputType::UnsignedOct => { let min_digits = cmp::max(precision, arg.len() as i32) as usize; let extended: Cow = extend_digits(arg, min_digits); - print_adjusted!( - extended, + print_adjusted( + extended.as_ref(), left_align, - should_alter, - prefix, + Some(should_alter), + Some(prefix), width, - padding_char + padding_char, ); } OutputType::UnsignedHex => { let min_digits = cmp::max(precision, arg.len() as i32) as usize; let extended: Cow = extend_digits(arg, min_digits); - print_adjusted!( - extended, + print_adjusted( + extended.as_ref(), left_align, - should_alter, - prefix, + Some(should_alter), + Some(prefix), width, - padding_char + padding_char, ); } _ => unreachable!(), From 243546ce967af5aaeeed99880a71d62809eafe97 Mon Sep 17 00:00:00 2001 From: snapdgn Date: Mon, 12 Sep 2022 12:19:56 +0530 Subject: [PATCH 3/6] fix: style/spelling --- src/uu/stat/src/stat.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index b92ad764d..883bf66d2 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -74,8 +74,8 @@ fn print_adjusted( ) { let mut field_width = cmp::max(width, s.len()); if let Some(p) = prefix { - if let Some(needprefix) = need_prefix { - if needprefix { + if let Some(prefix_flag) = need_prefix { + if prefix_flag { field_width -= p.len(); } } From d8226bf658e84fba7a3bd150dd21d221819a5ae3 Mon Sep 17 00:00:00 2001 From: snapdgn Date: Mon, 12 Sep 2022 18:35:30 +0530 Subject: [PATCH 4/6] add: documentation to refactored macros->functions --- src/uu/stat/src/stat.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index 883bf66d2..ed1cba694 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -26,6 +26,8 @@ use std::os::unix::prelude::OsStrExt; use std::path::Path; use std::{cmp, fs, iter}; +/// checks if the string is within the specified bound +/// fn check_bound(slice: &str, bound: usize, beg: usize, end: usize) -> UResult<()> { if end >= bound { return Err(USimpleError::new( @@ -36,6 +38,9 @@ fn check_bound(slice: &str, bound: usize, beg: usize, end: usize) -> UResult<()> Ok(()) } +/// pads the string with zeroes if supplied min is greater +/// then the length of the string, else returns the original string +/// fn extend_digits(string: &str, min: usize) -> Cow<'_, str> { if min > string.len() { let mut pad = String::with_capacity(min); @@ -55,6 +60,16 @@ enum Padding { Space, } +/// pads the string with zeroes or spaces and prints it +/// +/// # Example +/// ```ignore +/// uu_stat::pad_and_print("1", false, 5, Padding::Zero) == "00001"; +/// ``` +/// currently only supports '0' & ' ' as the padding character +/// because the format specification of print! does not support general +/// fill characters +/// fn pad_and_print(result: &str, left: bool, width: usize, padding: Padding) { match (left, padding) { (false, Padding::Zero) => print!("{result:0>width$}"), @@ -64,6 +79,8 @@ fn pad_and_print(result: &str, left: bool, width: usize, padding: Padding) { }; } +/// prints the adjusted string after padding +/// fn print_adjusted( s: &str, left: bool, From b723be4fe24784eff16da7d4e81976503a121b52 Mon Sep 17 00:00:00 2001 From: snapdgn Date: Mon, 12 Sep 2022 19:56:02 +0530 Subject: [PATCH 5/6] reposition functions --- src/uu/stat/src/stat.rs | 43 ++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index ed1cba694..23e66eddd 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -26,6 +26,27 @@ use std::os::unix::prelude::OsStrExt; use std::path::Path; use std::{cmp, fs, iter}; +static ABOUT: &str = "Display file or file system status."; +const USAGE: &str = "{} [OPTION]... FILE..."; + +pub mod options { + pub static DEREFERENCE: &str = "dereference"; + pub static FILE_SYSTEM: &str = "file-system"; + pub static FORMAT: &str = "format"; + pub static PRINTF: &str = "printf"; + pub static TERSE: &str = "terse"; +} + +static ARG_FILES: &str = "files"; + +pub const F_ALTER: u8 = 1; +pub const F_ZERO: u8 = 1 << 1; +pub const F_LEFT: u8 = 1 << 2; +pub const F_SPACE: u8 = 1 << 3; +pub const F_SIGN: u8 = 1 << 4; +// unused at present +pub const F_GROUP: u8 = 1 << 5; + /// checks if the string is within the specified bound /// fn check_bound(slice: &str, bound: usize, beg: usize, end: usize) -> UResult<()> { @@ -101,28 +122,6 @@ fn print_adjusted( pad_and_print(s, left, field_width, padding); } } - -static ABOUT: &str = "Display file or file system status."; -const USAGE: &str = "{} [OPTION]... FILE..."; - -pub mod options { - pub static DEREFERENCE: &str = "dereference"; - pub static FILE_SYSTEM: &str = "file-system"; - pub static FORMAT: &str = "format"; - pub static PRINTF: &str = "printf"; - pub static TERSE: &str = "terse"; -} - -static ARG_FILES: &str = "files"; - -pub const F_ALTER: u8 = 1; -pub const F_ZERO: u8 = 1 << 1; -pub const F_LEFT: u8 = 1 << 2; -pub const F_SPACE: u8 = 1 << 3; -pub const F_SIGN: u8 = 1 << 4; -// unused at present -pub const F_GROUP: u8 = 1 << 5; - #[derive(Debug, PartialEq, Eq)] pub enum OutputType { Str, From f7601c022e7935f1348080982b47a70349a8ac7a Mon Sep 17 00:00:00 2001 From: snapdgn Date: Wed, 14 Sep 2022 10:51:03 +0530 Subject: [PATCH 6/6] updated documentation --- src/uu/stat/src/stat.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index 23e66eddd..658c47f17 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -47,7 +47,9 @@ pub const F_SIGN: u8 = 1 << 4; // unused at present pub const F_GROUP: u8 = 1 << 5; -/// checks if the string is within the specified bound +/// checks if the string is within the specified bound, +/// if it gets out of bound, error out by printing sub-string from index `beg` to`end`, +/// where `beg` & `end` is the beginning and end index of sub-string, respectively /// fn check_bound(slice: &str, bound: usize, beg: usize, end: usize) -> UResult<()> { if end >= bound { @@ -101,7 +103,11 @@ fn pad_and_print(result: &str, left: bool, width: usize, padding: Padding) { } /// prints the adjusted string after padding -/// +/// `left` flag specifies the type of alignment of the string +/// `width` is the supplied padding width of the string needed +/// `prefix` & `need_prefix` are Optional, which adjusts the `field_width` accordingly, where +/// `field_width` is the max of supplied `width` and size of string +/// `padding`, specifies type of padding, which is '0' or ' ' in this case. fn print_adjusted( s: &str, left: bool,