From 0b07ecdad228af02808ab6548a743a7c886d169f Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Mon, 21 Feb 2022 19:52:04 -0500 Subject: [PATCH 1/3] df: use Options.columns field to control output Replace the `Options.show_fs_type` and `Options.show_inode_instead` fields with the more general `Options.columns`, a `Vec` of `Column` variants representing the exact sequence of columns to display in the output. This makes `Options` able to represent arbitrary output column display configurations. --- src/uu/df/src/columns.rs | 109 ++++++++++++++++++++++++++++ src/uu/df/src/df.rs | 32 +++++++-- src/uu/df/src/table.rs | 149 ++++++++++++++++++++------------------- 3 files changed, 210 insertions(+), 80 deletions(-) create mode 100644 src/uu/df/src/columns.rs diff --git a/src/uu/df/src/columns.rs b/src/uu/df/src/columns.rs new file mode 100644 index 000000000..f9b4c4f2b --- /dev/null +++ b/src/uu/df/src/columns.rs @@ -0,0 +1,109 @@ +// * This file is part of the uutils coreutils package. +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. +// spell-checker:ignore itotal iused iavail ipcent pcent squashfs +use crate::{OPT_INODES, OPT_PRINT_TYPE}; +use clap::ArgMatches; + +/// The columns in the output table produced by `df`. +/// +/// The [`Row`] struct has a field corresponding to each of the +/// variants of this enumeration. +/// +/// [`Row`]: crate::table::Row +#[derive(PartialEq, Copy, Clone)] +pub(crate) enum Column { + /// The source of the mount point, usually a device. + Source, + + /// Total number of blocks. + Size, + + /// Number of used blocks. + Used, + + /// Number of available blocks. + Avail, + + /// Percentage of blocks used out of total number of blocks. + Pcent, + + /// The mount point. + Target, + + /// Total number of inodes. + Itotal, + + /// Number of used inodes. + Iused, + + /// Number of available inodes. + Iavail, + + /// Percentage of inodes used out of total number of inodes. + Ipcent, + + /// The filename given as a command-line argument. + File, + + /// The filesystem type, like "ext4" or "squashfs". + Fstype, + + /// Percentage of bytes available to non-privileged processes. + #[cfg(target_os = "macos")] + Capacity, +} + +impl Column { + /// Convert from command-line arguments to sequence of columns. + /// + /// The set of columns that will appear in the output table can be + /// specified by command-line arguments. This function converts + /// those arguments to a [`Vec`] of [`Column`] variants. + pub(crate) fn from_matches(matches: &ArgMatches) -> Vec { + match ( + matches.is_present(OPT_PRINT_TYPE), + matches.is_present(OPT_INODES), + ) { + (false, false) => vec![ + Self::Source, + Self::Size, + Self::Used, + Self::Avail, + #[cfg(target_os = "macos")] + Self::Capacity, + Self::Pcent, + Self::Target, + ], + (false, true) => vec![ + Self::Source, + Self::Itotal, + Self::Iused, + Self::Iavail, + Self::Ipcent, + Self::Target, + ], + (true, false) => vec![ + Self::Source, + Self::Fstype, + Self::Size, + Self::Used, + Self::Avail, + #[cfg(target_os = "macos")] + Self::Capacity, + Self::Pcent, + Self::Target, + ], + (true, true) => vec![ + Self::Source, + Self::Fstype, + Self::Itotal, + Self::Iused, + Self::Iavail, + Self::Ipcent, + Self::Target, + ], + } + } +} diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 65091b4cc..62e669282 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -7,6 +7,7 @@ // that was distributed with this source code. // spell-checker:ignore itotal iused iavail ipcent pcent tmpfs squashfs mod blocks; +mod columns; mod table; use uucore::error::{UResult, USimpleError}; @@ -25,6 +26,7 @@ use std::iter::FromIterator; use std::path::Path; use crate::blocks::{block_size_from_matches, BlockSize}; +use crate::columns::Column; use crate::table::{DisplayRow, Header, Row}; static ABOUT: &str = "Show information about the file system on which each FILE resides,\n\ @@ -66,18 +68,37 @@ struct FsSelector { /// Most of these parameters control which rows and which columns are /// displayed. The `block_size` determines the units to use when /// displaying numbers of bytes or inodes. -#[derive(Default)] struct Options { show_local_fs: bool, show_all_fs: bool, show_listed_fs: bool, - show_fs_type: bool, - show_inode_instead: bool, block_size: BlockSize, fs_selector: FsSelector, - /// Whether to show a final row comprising the totals for each column. show_total: bool, + /// Sequence of columns to display in the output table. + columns: Vec, +} + +impl Default for Options { + fn default() -> Self { + Self { + show_local_fs: Default::default(), + show_all_fs: Default::default(), + show_listed_fs: Default::default(), + block_size: Default::default(), + fs_selector: Default::default(), + show_total: Default::default(), + columns: vec![ + Column::Source, + Column::Size, + Column::Used, + Column::Avail, + Column::Pcent, + Column::Target, + ], + } + } } enum OptionsError { @@ -103,12 +124,11 @@ impl Options { show_local_fs: matches.is_present(OPT_LOCAL), show_all_fs: matches.is_present(OPT_ALL), show_listed_fs: false, - show_fs_type: matches.is_present(OPT_PRINT_TYPE), - show_inode_instead: matches.is_present(OPT_INODES), block_size: block_size_from_matches(matches) .map_err(|_| OptionsError::InvalidBlockSize)?, fs_selector: FsSelector::from(matches), show_total: matches.is_present(OPT_TOTAL), + columns: Column::from_matches(matches), }) } } diff --git a/src/uu/df/src/table.rs b/src/uu/df/src/table.rs index 65d390dd2..a05b74d59 100644 --- a/src/uu/df/src/table.rs +++ b/src/uu/df/src/table.rs @@ -2,7 +2,7 @@ // * // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -// spell-checker:ignore tmpfs +// spell-checker:ignore tmpfs Pcent Itotal Iused Iavail Ipcent //! The filesystem usage data table. //! //! A table comprises a header row ([`Header`]) and a collection of @@ -11,6 +11,7 @@ //! [`DisplayRow`] implements [`std::fmt::Display`]. use number_prefix::NumberPrefix; +use crate::columns::Column; use crate::{BlockSize, Filesystem, Options}; use uucore::fsext::{FsUsage, MountInfo}; @@ -225,56 +226,37 @@ impl<'a> DisplayRow<'a> { Some(x) => format!("{:.0}%", (100.0 * x).ceil()), } } - - /// Write the bytes data for this row. - /// - /// # Errors - /// - /// If there is a problem writing to `f`. - /// - /// If the scaling factor is not 1000, 1024, or a negative number. - fn fmt_bytes(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{0: >12} ", self.scaled(self.row.bytes)?)?; - write!(f, "{0: >12} ", self.scaled(self.row.bytes_used)?)?; - write!(f, "{0: >12} ", self.scaled(self.row.bytes_free)?)?; - #[cfg(target_os = "macos")] - write!( - f, - "{0: >12} ", - DisplayRow::percentage(self.row.bytes_capacity) - )?; - write!(f, "{0: >5} ", DisplayRow::percentage(self.row.bytes_usage))?; - Ok(()) - } - - /// Write the inodes data for this row. - /// - /// # Errors - /// - /// If there is a problem writing to `f`. - /// - /// If the scaling factor is not 1000, 1024, or a negative number. - fn fmt_inodes(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{0: >12} ", self.scaled(self.row.inodes)?)?; - write!(f, "{0: >12} ", self.scaled(self.row.inodes_used)?)?; - write!(f, "{0: >12} ", self.scaled(self.row.inodes_free)?)?; - write!(f, "{0: >5} ", DisplayRow::percentage(self.row.inodes_usage))?; - Ok(()) - } } impl fmt::Display for DisplayRow<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{0: <16} ", self.row.fs_device)?; - if self.options.show_fs_type { - write!(f, "{0: <5} ", self.row.fs_type)?; + for column in &self.options.columns { + match column { + Column::Source => write!(f, "{0: <16} ", self.row.fs_device)?, + Column::Size => write!(f, "{0: >12} ", self.scaled(self.row.bytes)?)?, + Column::Used => write!(f, "{0: >12} ", self.scaled(self.row.bytes_used)?)?, + Column::Avail => write!(f, "{0: >12} ", self.scaled(self.row.bytes_free)?)?, + Column::Pcent => { + write!(f, "{0: >5} ", DisplayRow::percentage(self.row.bytes_usage))?; + } + Column::Target => write!(f, "{0: <16}", self.row.fs_mount)?, + Column::Itotal => write!(f, "{0: >12} ", self.scaled(self.row.inodes)?)?, + Column::Iused => write!(f, "{0: >12} ", self.scaled(self.row.inodes_used)?)?, + Column::Iavail => write!(f, "{0: >12} ", self.scaled(self.row.inodes_free)?)?, + Column::Ipcent => { + write!(f, "{0: >5} ", DisplayRow::percentage(self.row.inodes_usage))?; + } + // TODO Implement this. + Column::File => {} + Column::Fstype => write!(f, "{0: <5} ", self.row.fs_type)?, + #[cfg(target_os = "macos")] + Column::Capacity => write!( + f, + "{0: >12} ", + DisplayRow::percentage(self.row.bytes_capacity) + )?, + } } - if self.options.show_inode_instead { - self.fmt_inodes(f)?; - } else { - self.fmt_bytes(f)?; - } - write!(f, "{0: <16}", self.row.fs_mount)?; Ok(()) } } @@ -296,29 +278,29 @@ impl<'a> Header<'a> { impl fmt::Display for Header<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{0: <16} ", "Filesystem")?; - if self.options.show_fs_type { - write!(f, "{0: <5} ", "Type")?; + for column in &self.options.columns { + match column { + Column::Source => write!(f, "{0: <16} ", "Filesystem")?, + // `Display` is implemented for `BlockSize`, but + // `Display` only works when formatting an object into + // an empty format, `{}`. So we use `format!()` first + // to create the string, then use `write!()` to align + // the string and pad with spaces. + Column::Size => write!(f, "{0: >12} ", format!("{}", self.options.block_size))?, + Column::Used => write!(f, "{0: >12} ", "Used")?, + Column::Avail => write!(f, "{0: >12} ", "Available")?, + Column::Pcent => write!(f, "{0: >5} ", "Use%")?, + Column::Target => write!(f, "{0: <16} ", "Mounted on")?, + Column::Itotal => write!(f, "{0: >12} ", "Inodes")?, + Column::Iused => write!(f, "{0: >12} ", "IUsed")?, + Column::Iavail => write!(f, "{0: >12} ", "IFree")?, + Column::Ipcent => write!(f, "{0: >5} ", "IUse%")?, + Column::File => write!(f, "{0: <16}", "File")?, + Column::Fstype => write!(f, "{0: <5} ", "Type")?, + #[cfg(target_os = "macos")] + Column::Capacity => write!(f, "{0: >12} ", "Capacity")?, + } } - if self.options.show_inode_instead { - write!(f, "{0: >12} ", "Inodes")?; - write!(f, "{0: >12} ", "IUsed")?; - write!(f, "{0: >12} ", "IFree")?; - write!(f, "{0: >5} ", "IUse%")?; - } else { - // `Display` is implemented for `BlockSize`, but `Display` - // only works when formatting an object into an empty - // format, `{}`. So we use `format!()` first to create the - // string, then use `write!()` to align the string and pad - // with spaces. - write!(f, "{0: >12} ", format!("{}", self.options.block_size))?; - write!(f, "{0: >12} ", "Used")?; - write!(f, "{0: >12} ", "Available")?; - #[cfg(target_os = "macos")] - write!(f, "{0: >12} ", "Capacity")?; - write!(f, "{0: >5} ", "Use%")?; - } - write!(f, "{0: <16} ", "Mounted on")?; Ok(()) } } @@ -326,9 +308,28 @@ impl fmt::Display for Header<'_> { #[cfg(test)] mod tests { + use crate::columns::Column; use crate::table::{DisplayRow, Header, Row}; use crate::{BlockSize, Options}; + const COLUMNS_WITH_FS_TYPE: [Column; 7] = [ + Column::Source, + Column::Fstype, + Column::Size, + Column::Used, + Column::Avail, + Column::Pcent, + Column::Target, + ]; + const COLUMNS_WITH_INODES: [Column; 6] = [ + Column::Source, + Column::Itotal, + Column::Iused, + Column::Iavail, + Column::Ipcent, + Column::Target, + ]; + #[test] fn test_header_display() { let options = Default::default(); @@ -341,7 +342,7 @@ mod tests { #[test] fn test_header_display_fs_type() { let options = Options { - show_fs_type: true, + columns: COLUMNS_WITH_FS_TYPE.to_vec(), ..Default::default() }; assert_eq!( @@ -353,7 +354,7 @@ mod tests { #[test] fn test_header_display_inode() { let options = Options { - show_inode_instead: true, + columns: COLUMNS_WITH_INODES.to_vec(), ..Default::default() }; assert_eq!( @@ -431,8 +432,8 @@ mod tests { #[test] fn test_row_display_fs_type() { let options = Options { + columns: COLUMNS_WITH_FS_TYPE.to_vec(), block_size: BlockSize::Bytes(1), - show_fs_type: true, ..Default::default() }; let row = Row { @@ -462,8 +463,8 @@ mod tests { #[test] fn test_row_display_inodes() { let options = Options { + columns: COLUMNS_WITH_INODES.to_vec(), block_size: BlockSize::Bytes(1), - show_inode_instead: true, ..Default::default() }; let row = Row { @@ -494,7 +495,7 @@ mod tests { fn test_row_display_human_readable_si() { let options = Options { block_size: BlockSize::HumanReadableDecimal, - show_fs_type: true, + columns: COLUMNS_WITH_FS_TYPE.to_vec(), ..Default::default() }; let row = Row { @@ -525,7 +526,7 @@ mod tests { fn test_row_display_human_readable_binary() { let options = Options { block_size: BlockSize::HumanReadableBinary, - show_fs_type: true, + columns: COLUMNS_WITH_FS_TYPE.to_vec(), ..Default::default() }; let row = Row { From ec048b3857bddeca0e14c8ddb5e763191f8e1ac0 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Mon, 21 Feb 2022 19:54:12 -0500 Subject: [PATCH 2/3] df: implement the --output command-line argument Implement the `--output` command-line argument, which allows specifying an exact sequence of columns to display in the output table. For example, $ df --output=source,fstype | head -n3 Filesystem Type udev devtmpfs tmpfs tmpfs (The spacing does not exactly match the spacing of GNU `df` yet.) Fixes #3057. --- src/uu/df/src/columns.rs | 67 +++++++++++++++++++++++++++++++++++++--- tests/by-util/test_df.rs | 29 +++++++++++++++++ 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/src/uu/df/src/columns.rs b/src/uu/df/src/columns.rs index f9b4c4f2b..89dd35220 100644 --- a/src/uu/df/src/columns.rs +++ b/src/uu/df/src/columns.rs @@ -3,7 +3,7 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. // spell-checker:ignore itotal iused iavail ipcent pcent squashfs -use crate::{OPT_INODES, OPT_PRINT_TYPE}; +use crate::{OPT_INODES, OPT_OUTPUT, OPT_PRINT_TYPE}; use clap::ArgMatches; /// The columns in the output table produced by `df`. @@ -65,8 +65,9 @@ impl Column { match ( matches.is_present(OPT_PRINT_TYPE), matches.is_present(OPT_INODES), + matches.occurrences_of(OPT_OUTPUT) > 0, ) { - (false, false) => vec![ + (false, false, false) => vec![ Self::Source, Self::Size, Self::Used, @@ -76,7 +77,20 @@ impl Column { Self::Pcent, Self::Target, ], - (false, true) => vec![ + (false, false, true) => { + matches + .values_of(OPT_OUTPUT) + .unwrap() + .map(|s| { + // Unwrapping here should not panic because the + // command-line argument parsing library should be + // responsible for ensuring each comma-separated + // string is a valid column label. + Self::parse(s).unwrap() + }) + .collect() + } + (false, true, false) => vec![ Self::Source, Self::Itotal, Self::Iused, @@ -84,7 +98,7 @@ impl Column { Self::Ipcent, Self::Target, ], - (true, false) => vec![ + (true, false, false) => vec![ Self::Source, Self::Fstype, Self::Size, @@ -95,7 +109,7 @@ impl Column { Self::Pcent, Self::Target, ], - (true, true) => vec![ + (true, true, false) => vec![ Self::Source, Self::Fstype, Self::Itotal, @@ -104,6 +118,49 @@ impl Column { Self::Ipcent, Self::Target, ], + // The command-line arguments -T and -i are each mutually + // exclusive with --output, so the command-line argument + // parser should reject those combinations before we get + // to this point in the code. + _ => unreachable!(), + } + } + + /// Convert a column name to the corresponding enumeration variant. + /// + /// There are twelve valid column names, one for each variant: + /// + /// - "source" + /// - "fstype" + /// - "itotal" + /// - "iused" + /// - "iavail" + /// - "ipcent" + /// - "size" + /// - "used" + /// - "avail" + /// - "pcent" + /// - "file" + /// - "target" + /// + /// # Errors + /// + /// If the string `s` is not one of the valid column names. + fn parse(s: &str) -> Result { + match s { + "source" => Ok(Self::Source), + "fstype" => Ok(Self::Fstype), + "itotal" => Ok(Self::Itotal), + "iused" => Ok(Self::Iused), + "iavail" => Ok(Self::Iavail), + "ipcent" => Ok(Self::Ipcent), + "size" => Ok(Self::Size), + "used" => Ok(Self::Used), + "avail" => Ok(Self::Avail), + "pcent" => Ok(Self::Pcent), + "file" => Ok(Self::File), + "target" => Ok(Self::Target), + _ => Err(()), } } } diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 8f8474901..5d99b648b 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -183,4 +183,33 @@ fn test_block_size_1024() { assert_eq!(get_header(34 * 1024 * 1024 * 1024), "34G-blocks"); } +// TODO The spacing does not match GNU df. Also we need to remove +// trailing spaces from the heading row. +#[test] +fn test_output_selects_columns() { + let output = new_ucmd!() + .args(&["--output=source"]) + .succeeds() + .stdout_move_str(); + assert_eq!(output.lines().next().unwrap(), "Filesystem "); + + let output = new_ucmd!() + .args(&["--output=source,target"]) + .succeeds() + .stdout_move_str(); + assert_eq!( + output.lines().next().unwrap(), + "Filesystem Mounted on " + ); + + let output = new_ucmd!() + .args(&["--output=source,target,used"]) + .succeeds() + .stdout_move_str(); + assert_eq!( + output.lines().next().unwrap(), + "Filesystem Mounted on Used " + ); +} + // ToDO: more tests... From c0cd0177064695697ba24b29689057618c8faa43 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Mon, 21 Feb 2022 20:50:55 -0500 Subject: [PATCH 3/3] tests: update test_df.rs to use --output argument --- tests/by-util/test_df.rs | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 5d99b648b..896a55931 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -43,21 +43,14 @@ fn test_df_output() { /// Test that the order of rows in the table does not change across executions. #[test] fn test_order_same() { - // TODO When #3057 is resolved, we should just use - // - // new_ucmd!().arg("--output=source").succeeds().stdout_move_str(); - // - // instead of parsing the entire `df` table as a string. - let output1 = new_ucmd!().succeeds().stdout_move_str(); - let output2 = new_ucmd!().succeeds().stdout_move_str(); - let output1: Vec = output1 - .lines() - .map(|l| String::from(l.split_once(' ').unwrap().0)) - .collect(); - let output2: Vec = output2 - .lines() - .map(|l| String::from(l.split_once(' ').unwrap().0)) - .collect(); + let output1 = new_ucmd!() + .arg("--output=source") + .succeeds() + .stdout_move_str(); + let output2 = new_ucmd!() + .arg("--output=source") + .succeeds() + .stdout_move_str(); assert_eq!(output1, output2); }