From 5ec1bba5e8095f49f904cdd74f3db3c32bd0e058 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sat, 13 Mar 2021 12:42:52 +0100 Subject: [PATCH 01/21] touch: use arggroup for sources --- src/uu/touch/src/touch.rs | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index 1cd3b2a70..1cb551fd2 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -13,7 +13,7 @@ pub extern crate filetime; #[macro_use] extern crate uucore; -use clap::{App, Arg}; +use clap::{App, Arg, ArgGroup}; use filetime::*; use std::fs::{self, File}; use std::io::Error; @@ -129,6 +129,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .takes_value(true) .min_values(1), ) + .group(ArgGroup::with_name("sources").args(&[ + options::sources::CURRENT, + options::sources::DATE, + options::sources::REFERENCE, + ])) .get_matches_from(args); let files: Vec = matches @@ -136,19 +141,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .map(|v| v.map(ToString::to_string).collect()) .unwrap_or_default(); - if matches.is_present(options::sources::DATE) - && (matches.is_present(options::sources::REFERENCE) - || matches.is_present(options::sources::CURRENT)) - || matches.is_present(options::sources::REFERENCE) - && (matches.is_present(options::sources::DATE) - || matches.is_present(options::sources::CURRENT)) - || matches.is_present(options::sources::CURRENT) - && (matches.is_present(options::sources::DATE) - || matches.is_present(options::sources::REFERENCE)) - { - panic!("Invalid options: cannot specify reference time from more than one source"); - } - let (mut atime, mut mtime) = if matches.is_present(options::sources::REFERENCE) { stat( &matches.value_of(options::sources::REFERENCE).unwrap()[..], @@ -188,10 +180,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { }; // Minor optimization: if no reference time was specified, we're done. - if !(matches.is_present(options::sources::DATE) - || matches.is_present(options::sources::REFERENCE) - || matches.is_present(options::sources::CURRENT)) - { + if !matches.is_present("sources") { continue; } } From dfc7a9505424539ee02aa0c051f16ddb98539ef4 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sat, 13 Mar 2021 12:46:54 +0100 Subject: [PATCH 02/21] tests/touch: add tests for multiple sources --- tests/by-util/test_touch.rs | 38 +++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/by-util/test_touch.rs b/tests/by-util/test_touch.rs index 7e04beced..52b0c1f51 100644 --- a/tests/by-util/test_touch.rs +++ b/tests/by-util/test_touch.rs @@ -203,6 +203,44 @@ fn test_touch_set_only_mtime_failed() { ucmd.args(&["-t", "2015010112342", "-m", file]).fails(); } +#[test] +fn test_touch_set_both_time_and_reference() { + let (at, mut ucmd) = at_and_ucmd!(); + let ref_file = "test_touch_reference"; + let file = "test_touch_set_both_time_and_reference"; + + let start_of_year = str_to_filetime("%Y%m%d%H%M", "201501010000"); + + at.touch(ref_file); + set_file_times(&at, ref_file, start_of_year, start_of_year); + assert!(at.file_exists(ref_file)); + + ucmd.args(&["-t", "2015010112342", "-r", ref_file]).fails(); +} + +#[test] +fn test_touch_set_both_date_and_reference() { + let (at, mut ucmd) = at_and_ucmd!(); + let ref_file = "test_touch_reference"; + let file = "test_touch_set_both_date_and_reference"; + + let start_of_year = str_to_filetime("%Y%m%d%H%M", "201501010000"); + + at.touch(ref_file); + set_file_times(&at, ref_file, start_of_year, start_of_year); + assert!(at.file_exists(ref_file)); + + ucmd.args(&["-d", "Thu Jan 01 12:34:00 2015", "-r", ref_file]).fails(); +} + +#[test] +fn test_touch_set_both_time_and_date() { + let (at, mut ucmd) = at_and_ucmd!(); + let file = "test_touch_set_both_time_and_date"; + + ucmd.args(&["-t", "2015010112342", "-d", "Thu Jan 01 12:34:00 2015", file]).fails(); +} + #[test] fn test_touch_set_only_mtime() { let (at, mut ucmd) = at_and_ucmd!(); From 86422a70d2b1c3b737175ddec4189d5568b81536 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sat, 13 Mar 2021 12:47:20 +0100 Subject: [PATCH 03/21] touch: turn macros into functions --- src/uu/touch/src/touch.rs | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index 1cb551fd2..e362329b3 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -36,23 +36,15 @@ pub mod options { static ARG_FILES: &str = "files"; -// Since touch's date/timestamp parsing doesn't account for timezone, the -// returned value from time::strptime() is UTC. We get system's timezone to -// localize the time. -macro_rules! to_local( - ($exp:expr) => ({ - let mut tm = $exp; - tm.tm_utcoff = time::now().tm_utcoff; - tm - }) -); +fn to_local(mut tm: time::Tm) -> time::Tm { + tm.tm_utcoff = time::now().tm_utcoff; + tm +} -macro_rules! local_tm_to_filetime( - ($exp:expr) => ({ - let ts = $exp.to_timespec(); - FileTime::from_unix_time(ts.sec as i64, ts.nsec as u32) - }) -); +fn local_tm_to_filetime(tm: time::Tm) -> FileTime { + let ts = tm.to_timespec(); + FileTime::from_unix_time(ts.sec as i64, ts.nsec as u32) +} fn get_usage() -> String { format!("{0} [OPTION]... [USER]", executable!()) @@ -161,7 +153,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { }; (timestamp, timestamp) } else { - let now = local_tm_to_filetime!(time::now()); + let now = local_tm_to_filetime(time::now()); (now, now) }; @@ -249,7 +241,7 @@ fn parse_date(str: &str) -> FileTime { // not about to implement GNU parse_datetime. // http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob_plain;f=lib/parse-datetime.y match time::strptime(str, "%c") { - Ok(tm) => local_tm_to_filetime!(to_local!(tm)), + Ok(tm) => local_tm_to_filetime(to_local(tm)), Err(e) => panic!("Unable to parse date\n{}", e), } } @@ -267,7 +259,7 @@ fn parse_timestamp(s: &str) -> FileTime { }; match time::strptime(&ts, format) { - Ok(tm) => local_tm_to_filetime!(to_local!(tm)), + Ok(tm) => local_tm_to_filetime(to_local(tm)), Err(e) => panic!("Unable to parse timestamp\n{}", e), } } From ed2787a6dfee224a2e9f8df03f6d8101f0afece7 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sat, 13 Mar 2021 13:25:36 +0100 Subject: [PATCH 04/21] test/touch: fmt --- tests/by-util/test_touch.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/by-util/test_touch.rs b/tests/by-util/test_touch.rs index 52b0c1f51..9921c16b5 100644 --- a/tests/by-util/test_touch.rs +++ b/tests/by-util/test_touch.rs @@ -215,7 +215,8 @@ fn test_touch_set_both_time_and_reference() { set_file_times(&at, ref_file, start_of_year, start_of_year); assert!(at.file_exists(ref_file)); - ucmd.args(&["-t", "2015010112342", "-r", ref_file]).fails(); + ucmd.args(&["-t", "2015010112342", "-r", ref_file, file]) + .fails(); } #[test] @@ -230,15 +231,23 @@ fn test_touch_set_both_date_and_reference() { set_file_times(&at, ref_file, start_of_year, start_of_year); assert!(at.file_exists(ref_file)); - ucmd.args(&["-d", "Thu Jan 01 12:34:00 2015", "-r", ref_file]).fails(); + ucmd.args(&["-d", "Thu Jan 01 12:34:00 2015", "-r", ref_file, file]) + .fails(); } #[test] fn test_touch_set_both_time_and_date() { - let (at, mut ucmd) = at_and_ucmd!(); + let (_at, mut ucmd) = at_and_ucmd!(); let file = "test_touch_set_both_time_and_date"; - ucmd.args(&["-t", "2015010112342", "-d", "Thu Jan 01 12:34:00 2015", file]).fails(); + ucmd.args(&[ + "-t", + "2015010112342", + "-d", + "Thu Jan 01 12:34:00 2015", + file, + ]) + .fails(); } #[test] From 44c390c29095b6f1498402877608556e5c7d1e13 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sat, 13 Mar 2021 13:52:08 +0100 Subject: [PATCH 05/21] touch: constant for the sources ArgGroup --- src/uu/touch/src/touch.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index e362329b3..39405900e 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -22,6 +22,8 @@ use std::path::Path; static VERSION: &str = env!("CARGO_PKG_VERSION"); static ABOUT: &str = "Update the access and modification times of each FILE to the current time."; pub mod options { + // Both SOURCES and sources are needed as we need to be able to refer to the ArgGroup. + pub static SOURCES: &str = "sources"; pub mod sources { pub static DATE: &str = "date"; pub static REFERENCE: &str = "reference"; @@ -121,7 +123,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .takes_value(true) .min_values(1), ) - .group(ArgGroup::with_name("sources").args(&[ + .group(ArgGroup::with_name(options::SOURCES).args(&[ options::sources::CURRENT, options::sources::DATE, options::sources::REFERENCE, @@ -172,7 +174,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { }; // Minor optimization: if no reference time was specified, we're done. - if !matches.is_present("sources") { + if !matches.is_present(options::SOURCES) { continue; } } From 9e98d24f5f370d28ad83e243e221edeb650e14bb Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sat, 13 Mar 2021 23:43:36 +0100 Subject: [PATCH 06/21] ls: move from getopts to clap --- src/uu/ls/Cargo.toml | 2 +- src/uu/ls/src/ls.rs | 376 ++++++++++++++++++++++++++----------------- 2 files changed, 228 insertions(+), 150 deletions(-) diff --git a/src/uu/ls/Cargo.toml b/src/uu/ls/Cargo.toml index 10d7cc2da..59901f807 100644 --- a/src/uu/ls/Cargo.toml +++ b/src/uu/ls/Cargo.toml @@ -15,7 +15,7 @@ edition = "2018" path = "src/ls.rs" [dependencies] -getopts = "0.2.18" +clap = "2.33" lazy_static = "1.0.1" number_prefix = "0.4" term_grid = "0.1.5" diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index b8defa397..ec17ec2b3 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -13,6 +13,7 @@ extern crate lazy_static; #[macro_use] extern crate uucore; +use clap::{App, Arg}; use number_prefix::NumberPrefix; use std::cmp::Reverse; #[cfg(unix)] @@ -33,14 +34,20 @@ use unicode_width::UnicodeWidthStr; #[cfg(unix)] use uucore::libc::{mode_t, S_ISGID, S_ISUID, S_ISVTX, S_IWOTH, S_IXGRP, S_IXOTH, S_IXUSR}; -static NAME: &str = "ls"; -static SUMMARY: &str = ""; -static LONG_HELP: &str = " +static VERSION: &str = env!("CARGO_PKG_VERSION"); +static ABOUT: &str = " By default, ls will list the files and contents of any directories on the command line, expect that it will ignore files and directories whose names start with '.' "; +fn get_usage() -> String { + format!( + "{0} [OPTION]... [FILE]...", + executable!() + ) +} + #[cfg(unix)] static DEFAULT_COLORS: &str = "rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=00:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arc=01;31:*.arj=01;31:*.taz=01;31:*.lha=01;31:*.lz4=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.tzo=01;31:*.t7z=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.dz=01;31:*.gz=01;31:*.lrz=01;31:*.lz=01;31:*.lzo=01;31:*.xz=01;31:*.bz2=01;31:*.bz=01;31:*.tbz=01;31:*.tbz2=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.war=01;31:*.ear=01;31:*.sar=01;31:*.rar=01;31:*.alz=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.cab=01;31:*.jpg=01;35:*.jpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.webm=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=00;36:*.au=00;36:*.flac=00;36:*.m4a=00;36:*.mid=00;36:*.midi=00;36:*.mka=00;36:*.mp3=00;36:*.mpc=00;36:*.ogg=00;36:*.ra=00;36:*.wav=00;36:*.oga=00;36:*.opus=00;36:*.spx=00;36:*.xspf=00;36:"; @@ -52,7 +59,7 @@ lazy_static! { let codes = LS_COLORS.split(':'); let mut map = HashMap::new(); for c in codes { - let p: Vec<_> = c.split('=').collect(); + let p: Vec<_> = c.splitn(1, '=').collect(); if p.len() == 2 { map.insert(p[0], p[1]); } @@ -65,107 +72,175 @@ lazy_static! { static ref END_CODE: &'static str = COLOR_MAP.get("ec").unwrap_or(&""); } +pub mod options { + pub static ONE: &str = "1"; + pub static ALL: &str = "all"; + pub static ALMOST_ALL: &str = "almost-all"; + pub static IGNORE_BACKUPS: &str = "ignore-backups"; + pub static COLUMNS: &str = "c"; + pub static DIRECTORY: &str = "directory"; + pub static CLASSIFY: &str = "classify"; + pub static HUMAN_READABLE: &str = "human-readable"; + pub static INODE: &str = "inode"; + pub static DEREFERENCE: &str = "dereference"; + pub static LONG: &str = "long"; + pub static NUMERIC_UID_GID: &str = "numeric-uid-gid"; + pub static REVERSE: &str = "reverse"; + pub static RECURSIVE: &str = "recursive"; + pub static SORT_SIZE: &str = "S"; + pub static SORT_TIME: &str = "t"; + pub static SORT_NONE: &str = "U"; + pub static COLOR: &str = "color"; + pub static PATHS: &str = "paths"; +} + pub fn uumain(args: impl uucore::Args) -> i32 { let args = args.collect_str(); - let syntax = format!( - "[OPTION]... DIRECTORY - {0} [OPTION]... [FILE]...", - NAME - ); - let matches = app!(&syntax, SUMMARY, LONG_HELP) - .optflag("1", "", "list one file per line.") - .optflag( - "a", - "all", - "Do not ignore hidden files (files with names that start with '.').", + let usage = get_usage(); + + let matches = App::new(executable!()) + .version(VERSION) + .about(ABOUT) + .usage(&usage[..]) + .arg( + Arg::with_name(options::ONE) + .short(options::ONE) + .help("list one file per line."), ) - .optflag( - "A", - "almost-all", - "In a directory, do not ignore all file names that start with '.', only ignore \ - '.' and '..'.", + .arg( + Arg::with_name(options::ALL) + .short("a") + .long(options::ALL) + .help("Do not ignore hidden files (files with names that start with '.')."), ) - .optflag("B", "ignore-backups", "Ignore entries which end with ~.") - .optflag( - "c", - "", - "If the long listing format (e.g., -l, -o) is being used, print the status \ - change time (the ‘ctime’ in the inode) instead of the modification time. When \ - explicitly sorting by time (--sort=time or -t) or when not using a long listing \ - format, sort according to the status change time.", + .arg( + Arg::with_name(options::ALMOST_ALL) + .short("A") + .long(options::ALMOST_ALL) + .help( + "In a directory, do not ignore all file names that start with '.', only ignore \ + '.' and '..'.", + ), ) - .optflag( - "d", - "directory", - "Only list the names of directories, rather than listing directory contents. \ - This will not follow symbolic links unless one of `--dereference-command-line \ - (-H)`, `--dereference (-L)`, or `--dereference-command-line-symlink-to-dir` is \ - specified.", + .arg( + Arg::with_name(options::IGNORE_BACKUPS) + .short("B") + .long(options::IGNORE_BACKUPS) + .help("Ignore entries which end with ~."), ) - .optflag( - "F", - "classify", - "Append a character to each file name indicating the file type. Also, for \ - regular files that are executable, append '*'. The file type indicators are \ - '/' for directories, '@' for symbolic links, '|' for FIFOs, '=' for sockets, \ - '>' for doors, and nothing for regular files.", + .arg( + Arg::with_name(options::COLUMNS) + .short(options::COLUMNS) + .help("If the long listing format (e.g., -l, -o) is being used, print the status \ + change time (the ‘ctime’ in the inode) instead of the modification time. When \ + explicitly sorting by time (--sort=time or -t) or when not using a long listing \ + format, sort according to the status change time.", + )) + .arg( + Arg::with_name(options::DIRECTORY) + .short("d") + .long(options::DIRECTORY) + .help( + "Only list the names of directories, rather than listing directory contents. \ + This will not follow symbolic links unless one of `--dereference-command-line \ + (-H)`, `--dereference (-L)`, or `--dereference-command-line-symlink-to-dir` is \ + specified.", + ), ) - .optflag( - "h", - "human-readable", - "Print human readable file sizes (e.g. 1K 234M 56G).", + .arg( + Arg::with_name(options::CLASSIFY) + .short("F") + .long(options::CLASSIFY) + .help("Append a character to each file name indicating the file type. Also, for \ + regular files that are executable, append '*'. The file type indicators are \ + '/' for directories, '@' for symbolic links, '|' for FIFOs, '=' for sockets, \ + '>' for doors, and nothing for regular files.", + )) + .arg( + Arg::with_name(options::HUMAN_READABLE) + .short("h") + .long(options::HUMAN_READABLE) + .help("Print human readable file sizes (e.g. 1K 234M 56G)."), ) - .optflag("i", "inode", "print the index number of each file") - .optflag( - "L", - "dereference", - "When showing file information for a symbolic link, show information for the \ - file the link references rather than the link itself.", + .arg( + Arg::with_name(options::INODE) + .short("i") + .long(options::INODE) + .help("print the index number of each file"), ) - .optflag("l", "long", "Display detailed information.") - .optflag("n", "numeric-uid-gid", "-l with numeric UIDs and GIDs.") - .optflag( - "r", - "reverse", - "Reverse whatever the sorting method is--e.g., list files in reverse \ - alphabetical order, youngest first, smallest first, or whatever.", + .arg( + Arg::with_name(options::DEREFERENCE) + .short("L") + .long(options::DEREFERENCE) + .help( + "When showing file information for a symbolic link, show information for the \ + file the link references rather than the link itself.", + ), ) - .optflag( - "R", - "recursive", - "List the contents of all directories recursively.", + .arg( + Arg::with_name(options::LONG) + .short("l") + .long(options::LONG) + .help("Display detailed information."), ) - .optflag("S", "", "Sort by file size, largest first.") - .optflag( - "t", - "", - "Sort by modification time (the 'mtime' in the inode), newest first.", + .arg( + Arg::with_name(options::NUMERIC_UID_GID) + .short("n") + .long(options::NUMERIC_UID_GID) + .help("-l with numeric UIDs and GIDs."), ) - .optflag( - "U", - "", - "Do not sort; list the files in whatever order they are stored in the \ - directory. This is especially useful when listing very large directories, \ - since not doing any sorting can be noticeably faster.", + .arg( + Arg::with_name(options::REVERSE) + .short("r") + .long(options::REVERSE) + .help("Reverse whatever the sorting method is--e.g., list files in reverse \ + alphabetical order, youngest first, smallest first, or whatever.", + )) + .arg( + Arg::with_name(options::RECURSIVE) + .short("R") + .long(options::RECURSIVE) + .help("List the contents of all directories recursively."), ) - .optflagopt( - "", - "color", - "Color output based on file type.", - "always|auto|never", + .arg( + Arg::with_name(options::SORT_SIZE) + .short(options::SORT_SIZE) + .help("Sort by file size, largest first."), ) - .parse(args); + .arg( + Arg::with_name(options::SORT_TIME) + .short(options::SORT_TIME) + .help("Sort by modification time (the 'mtime' in the inode), newest first."), + ) + .arg( + Arg::with_name(options::SORT_NONE) + .short(options::SORT_NONE) + .help("Do not sort; list the files in whatever order they are stored in the \ + directory. This is especially useful when listing very large directories, \ + since not doing any sorting can be noticeably faster.", + )) + .arg( + Arg::with_name(options::COLOR) + .long(options::COLOR) + .help("Color output based on file type.") + .possible_values(&["always", "yes", "force", "tty", "if-tty", "auto", "never", "no", "none"]) + .require_equals(true) + .empty_values(true), + ) + .arg(Arg::with_name(options::PATHS).multiple(true).takes_value(true)) + .get_matches_from(args); list(matches) } -fn list(options: getopts::Matches) -> i32 { - let locs: Vec = if options.free.is_empty() { - vec![String::from(".")] - } else { - options.free.to_vec() - }; +fn list(options: clap::ArgMatches) -> i32 { + let locs: Vec = options + .values_of(options::PATHS) + .map(|v| v.map(ToString::to_string).collect()) + .unwrap_or_else(|| vec![String::from(".")]); + + let number_of_locs = locs.len(); let mut files = Vec::::new(); let mut dirs = Vec::::new(); @@ -181,9 +256,9 @@ fn list(options: getopts::Matches) -> i32 { } let mut dir = false; - if p.is_dir() && !options.opt_present("d") { + if p.is_dir() && !options.is_present(options::DIRECTORY) { dir = true; - if options.opt_present("l") && !(options.opt_present("L")) { + if options.is_present(options::LONG) && !options.is_present(options::DEREFERENCE) { if let Ok(md) = p.symlink_metadata() { if md.file_type().is_symlink() && !p.ends_with("/") { dir = false; @@ -202,7 +277,7 @@ fn list(options: getopts::Matches) -> i32 { sort_entries(&mut dirs, &options); for dir in dirs { - if options.free.len() > 1 { + if number_of_locs > 1 { println!("\n{}:", dir.to_string_lossy()); } enter_directory(&dir, &options); @@ -215,10 +290,10 @@ fn list(options: getopts::Matches) -> i32 { } #[cfg(any(unix, target_os = "redox"))] -fn sort_entries(entries: &mut Vec, options: &getopts::Matches) { - let mut reverse = options.opt_present("r"); - if options.opt_present("t") { - if options.opt_present("c") { +fn sort_entries(entries: &mut Vec, options: &clap::ArgMatches) { + let mut reverse = options.is_present(options::REVERSE); + if options.is_present(options::SORT_TIME) { + if options.is_present(options::COLUMNS) { entries.sort_by_key(|k| { Reverse(get_metadata(k, options).map(|md| md.ctime()).unwrap_or(0)) }); @@ -232,10 +307,10 @@ fn sort_entries(entries: &mut Vec, options: &getopts::Matches) { ) }); } - } else if options.opt_present("S") { + } else if options.is_present(options::SORT_SIZE) { entries.sort_by_key(|k| get_metadata(k, options).map(|md| md.size()).unwrap_or(0)); reverse = !reverse; - } else if !options.opt_present("U") { + } else if !options.is_present(options::SORT_NONE) { entries.sort(); } @@ -257,9 +332,9 @@ fn is_hidden(file_path: &DirEntry) -> std::io::Result { } #[cfg(windows)] -fn sort_entries(entries: &mut Vec, options: &getopts::Matches) { - let mut reverse = options.opt_present("r"); - if options.opt_present("t") { +fn sort_entries(entries: &mut Vec, options: &clap::ArgMatches) { + let mut reverse = options.is_present(options::REVERSE); + if options.is_present(options::SORT_TIME) { entries.sort_by_key(|k| { // Newest first Reverse( @@ -268,14 +343,14 @@ fn sort_entries(entries: &mut Vec, options: &getopts::Matches) { .unwrap_or(std::time::UNIX_EPOCH), ) }); - } else if options.opt_present("S") { + } else if options.is_present(options::SORT_SIZE) { entries.sort_by_key(|k| { get_metadata(k, options) .map(|md| md.file_size()) .unwrap_or(0) }); reverse = !reverse; - } else if !options.opt_present("U") { + } else if !options.is_present(options::SORT_NONE) { entries.sort(); } @@ -284,19 +359,22 @@ fn sort_entries(entries: &mut Vec, options: &getopts::Matches) { } } -fn should_display(entry: &DirEntry, options: &getopts::Matches) -> bool { +fn should_display(entry: &DirEntry, options: &clap::ArgMatches) -> bool { let ffi_name = entry.file_name(); let name = ffi_name.to_string_lossy(); - if !options.opt_present("a") && !options.opt_present("A") && is_hidden(entry).unwrap() { + if !options.is_present(options::ALL) + && !options.is_present(options::ALMOST_ALL) + && is_hidden(entry).unwrap() + { return false; } - if options.opt_present("B") && name.ends_with('~') { + if options.is_present(options::IGNORE_BACKUPS) && name.ends_with('~') { return false; } true } -fn enter_directory(dir: &PathBuf, options: &getopts::Matches) { +fn enter_directory(dir: &PathBuf, options: &clap::ArgMatches) { let mut entries: Vec<_> = safe_unwrap!(fs::read_dir(dir).and_then(Iterator::collect)); entries.retain(|e| should_display(e, options)); @@ -304,7 +382,7 @@ fn enter_directory(dir: &PathBuf, options: &getopts::Matches) { let mut entries: Vec<_> = entries.iter().map(DirEntry::path).collect(); sort_entries(&mut entries, options); - if options.opt_present("a") { + if options.is_present(options::ALL) { let mut display_entries = entries.clone(); display_entries.insert(0, dir.join("..")); display_entries.insert(0, dir.join(".")); @@ -313,7 +391,7 @@ fn enter_directory(dir: &PathBuf, options: &getopts::Matches) { display_items(&entries, Some(dir), options); } - if options.opt_present("R") { + if options.is_present(options::RECURSIVE) { for e in entries.iter().filter(|p| p.is_dir()) { println!("\n{}:", e.to_string_lossy()); enter_directory(&e, options); @@ -321,15 +399,15 @@ fn enter_directory(dir: &PathBuf, options: &getopts::Matches) { } } -fn get_metadata(entry: &PathBuf, options: &getopts::Matches) -> std::io::Result { - if options.opt_present("L") { +fn get_metadata(entry: &PathBuf, options: &clap::ArgMatches) -> std::io::Result { + if options.is_present(options::DEREFERENCE) { entry.metadata().or_else(|_| entry.symlink_metadata()) } else { entry.symlink_metadata() } } -fn display_dir_entry_size(entry: &PathBuf, options: &getopts::Matches) -> (usize, usize) { +fn display_dir_entry_size(entry: &PathBuf, options: &clap::ArgMatches) -> (usize, usize) { if let Ok(md) = get_metadata(entry, options) { ( display_symlink_count(&md).len(), @@ -341,17 +419,11 @@ fn display_dir_entry_size(entry: &PathBuf, options: &getopts::Matches) -> (usize } fn pad_left(string: String, count: usize) -> String { - if count > string.len() { - let pad = count - string.len(); - let pad = String::from_utf8(vec![b' '; pad]).unwrap(); - format!("{}{}", pad, string) - } else { - string - } + format!("{:>width$}", string, width = count) } -fn display_items(items: &[PathBuf], strip: Option<&Path>, options: &getopts::Matches) { - if options.opt_present("long") || options.opt_present("numeric-uid-gid") { +fn display_items(items: &[PathBuf], strip: Option<&Path>, options: &clap::ArgMatches) { + if options.is_present(options::LONG) || options.is_present(options::NUMERIC_UID_GID) { let (mut max_links, mut max_size) = (1, 1); for item in items { let (links, size) = display_dir_entry_size(item, options); @@ -362,7 +434,7 @@ fn display_items(items: &[PathBuf], strip: Option<&Path>, options: &getopts::Mat display_item_long(item, strip, max_links, max_size, options); } } else { - if !options.opt_present("1") { + if !options.is_present(options::ONE) { let names = items.iter().filter_map(|i| { let md = get_metadata(i, options); match md { @@ -410,7 +482,7 @@ fn display_item_long( strip: Option<&Path>, max_links: usize, max_size: usize, - options: &getopts::Matches, + options: &clap::ArgMatches, ) { let md = match get_metadata(item, options) { Err(e) => { @@ -436,8 +508,8 @@ fn display_item_long( } #[cfg(unix)] -fn get_inode(metadata: &Metadata, options: &getopts::Matches) -> String { - if options.opt_present("inode") { +fn get_inode(metadata: &Metadata, options: &clap::ArgMatches) -> String { + if options.is_present(options::INODE) { format!("{:8} ", metadata.ino()) } else { "".to_string() @@ -445,7 +517,7 @@ fn get_inode(metadata: &Metadata, options: &getopts::Matches) -> String { } #[cfg(not(unix))] -fn get_inode(_metadata: &Metadata, _options: &getopts::Matches) -> String { +fn get_inode(_metadata: &Metadata, _options: &clap::ArgMatches) -> String { "".to_string() } @@ -455,8 +527,8 @@ fn get_inode(_metadata: &Metadata, _options: &getopts::Matches) -> String { use uucore::entries; #[cfg(unix)] -fn display_uname(metadata: &Metadata, options: &getopts::Matches) -> String { - if options.opt_present("numeric-uid-gid") { +fn display_uname(metadata: &Metadata, options: &clap::ArgMatches) -> String { + if options.is_present(options::NUMERIC_UID_GID) { metadata.uid().to_string() } else { entries::uid2usr(metadata.uid()).unwrap_or_else(|_| metadata.uid().to_string()) @@ -464,8 +536,8 @@ fn display_uname(metadata: &Metadata, options: &getopts::Matches) -> String { } #[cfg(unix)] -fn display_group(metadata: &Metadata, options: &getopts::Matches) -> String { - if options.opt_present("numeric-uid-gid") { +fn display_group(metadata: &Metadata, options: &clap::ArgMatches) -> String { + if options.is_present(options::NUMERIC_UID_GID) { metadata.gid().to_string() } else { entries::gid2grp(metadata.gid()).unwrap_or_else(|_| metadata.gid().to_string()) @@ -474,19 +546,19 @@ fn display_group(metadata: &Metadata, options: &getopts::Matches) -> String { #[cfg(not(unix))] #[allow(unused_variables)] -fn display_uname(metadata: &Metadata, _options: &getopts::Matches) -> String { +fn display_uname(metadata: &Metadata, _options: &clap::ArgMatches) -> String { "somebody".to_string() } #[cfg(not(unix))] #[allow(unused_variables)] -fn display_group(metadata: &Metadata, _options: &getopts::Matches) -> String { +fn display_group(metadata: &Metadata, _options: &clap::ArgMatches) -> String { "somegroup".to_string() } #[cfg(unix)] -fn display_date(metadata: &Metadata, options: &getopts::Matches) -> String { - let secs = if options.opt_present("c") { +fn display_date(metadata: &Metadata, options: &clap::ArgMatches) -> String { + let secs = if options.is_present(options::COLUMNS) { metadata.ctime() } else { metadata.mtime() @@ -497,7 +569,7 @@ fn display_date(metadata: &Metadata, options: &getopts::Matches) -> String { #[cfg(not(unix))] #[allow(unused_variables)] -fn display_date(metadata: &Metadata, options: &getopts::Matches) -> String { +fn display_date(metadata: &Metadata, options: &clap::ArgMatches) -> String { if let Ok(mtime) = metadata.modified() { let time = time::at(Timespec::new( mtime @@ -512,8 +584,10 @@ fn display_date(metadata: &Metadata, options: &getopts::Matches) -> String { } } -fn display_file_size(metadata: &Metadata, options: &getopts::Matches) -> String { - if options.opt_present("human-readable") { +fn display_file_size(metadata: &Metadata, options: &clap::ArgMatches) -> String { + // NOTE: The human-readable behaviour deviates from the GNU ls. + // The GNU ls uses binary prefixes by default. + if options.is_present(options::HUMAN_READABLE) { match NumberPrefix::decimal(metadata.len() as f64) { NumberPrefix::Standalone(bytes) => bytes.to_string(), NumberPrefix::Prefixed(prefix, bytes) => { @@ -551,15 +625,15 @@ fn display_file_name( path: &Path, strip: Option<&Path>, metadata: &Metadata, - options: &getopts::Matches, + options: &clap::ArgMatches, ) -> Cell { let mut name = get_file_name(path, strip); - if !options.opt_present("long") { + if !options.is_present(options::LONG) { name = get_inode(metadata, options) + &name; } - if options.opt_present("classify") { + if options.is_present(options::CLASSIFY) { let file_type = metadata.file_type(); if file_type.is_dir() { name.push('/'); @@ -568,7 +642,7 @@ fn display_file_name( } } - if options.opt_present("long") && metadata.file_type().is_symlink() { + if options.is_present(options::LONG) && metadata.file_type().is_symlink() { if let Ok(target) = path.read_link() { // We don't bother updating width here because it's not used for long listings let target_name = target.to_string_lossy().to_string(); @@ -613,23 +687,27 @@ fn display_file_name( path: &Path, strip: Option<&Path>, metadata: &Metadata, - options: &getopts::Matches, + options: &clap::ArgMatches, ) -> Cell { let mut name = get_file_name(path, strip); - if !options.opt_present("long") { + if !options.is_present(options::LONG) { name = get_inode(metadata, options) + &name; } let mut width = UnicodeWidthStr::width(&*name); - let color = match options.opt_str("color") { - None => atty::is(atty::Stream::Stdout), - Some(val) => match val.as_ref() { - "always" | "yes" | "force" => true, - "auto" | "tty" | "if-tty" => atty::is(atty::Stream::Stdout), - /* "never" | "no" | "none" | */ _ => false, - }, + let color = if options.is_present(options::COLOR) { + match options.value_of(options::COLOR) { + None => atty::is(atty::Stream::Stdout), + Some(val) => match val { + "" | "always" | "yes" | "force" => true, + "auto" | "tty" | "if-tty" => atty::is(atty::Stream::Stdout), + /* "never" | "no" | "none" | */ _ => false, + }, + } + } else { + false }; - let classify = options.opt_present("classify"); + let classify = options.is_present(options::CLASSIFY); let ext; if color || classify { @@ -693,7 +771,7 @@ fn display_file_name( } } - if options.opt_present("long") && metadata.file_type().is_symlink() { + if options.is_present(options::LONG) && metadata.file_type().is_symlink() { if let Ok(target) = path.read_link() { // We don't bother updating width here because it's not used for long listings let code = if target.exists() { "fi" } else { "mi" }; From 7c8e8b2d4cce76b7c3bdddd11a1dcc4b683d6e4e Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sun, 14 Mar 2021 12:22:32 +0100 Subject: [PATCH 07/21] ls: refactor arguments into a config struct --- src/uu/ls/src/ls.rs | 419 ++++++++++++++++++++++++++------------------ 1 file changed, 253 insertions(+), 166 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ec17ec2b3..d61a45702 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -42,10 +42,7 @@ static ABOUT: &str = " "; fn get_usage() -> String { - format!( - "{0} [OPTION]... [FILE]...", - executable!() - ) + format!("{0} [OPTION]... [FILE]...", executable!()) } #[cfg(unix)] @@ -73,11 +70,10 @@ lazy_static! { } pub mod options { - pub static ONE: &str = "1"; + pub static ONELINE: &str = "1"; pub static ALL: &str = "all"; pub static ALMOST_ALL: &str = "almost-all"; pub static IGNORE_BACKUPS: &str = "ignore-backups"; - pub static COLUMNS: &str = "c"; pub static DIRECTORY: &str = "directory"; pub static CLASSIFY: &str = "classify"; pub static HUMAN_READABLE: &str = "human-readable"; @@ -90,10 +86,128 @@ pub mod options { pub static SORT_SIZE: &str = "S"; pub static SORT_TIME: &str = "t"; pub static SORT_NONE: &str = "U"; + pub static SORT_CTIME: &str = "c"; pub static COLOR: &str = "color"; pub static PATHS: &str = "paths"; } +#[derive(PartialEq, Eq)] +enum DisplayOptions { + Columns, + Long, + OneLine, +} + +enum Sort { + None, + Name, + Size, + Time, + CTime, +} + +enum SizeFormats { + Bytes, + Binary, // Powers of 1024, --human-readable +} + +#[derive(PartialEq, Eq)] +enum Files { + All, + AlmostAll, + Normal, +} + +struct Config { + display: DisplayOptions, + files: Files, + sort: Sort, + + recursive: bool, + reverse: bool, + dereference: bool, + classify: bool, + ignore_backups: bool, + size_format: SizeFormats, + numeric_uid_gid: bool, + directory: bool, + #[cfg(unix)] + inode: bool, + #[cfg(unix)] + color: bool, +} + +impl Config { + fn from(options: clap::ArgMatches) -> Config { + let display = if options.is_present(options::LONG) { + DisplayOptions::Long + } else if options.is_present(options::ONELINE) { + DisplayOptions::OneLine + } else { + DisplayOptions::Columns + }; + + let files = if options.is_present(options::ALL) { + Files::All + } else if options.is_present(options::ALMOST_ALL) { + Files::AlmostAll + } else { + Files::Normal + }; + + let sort = if options.is_present(options::SORT_TIME) { + Sort::Time + } else if options.is_present(options::SORT_CTIME) { + Sort::CTime + } else if options.is_present(options::SORT_SIZE) { + Sort::Size + } else if options.is_present(options::SORT_NONE) { + Sort::None + } else { + Sort::Name + }; + + #[cfg(unix)] + let color = if options.is_present(options::COLOR) { + match options.value_of(options::COLOR) { + None => atty::is(atty::Stream::Stdout), + Some(val) => match val { + "" | "always" | "yes" | "force" => true, + "auto" | "tty" | "if-tty" => atty::is(atty::Stream::Stdout), + /* "never" | "no" | "none" | */ _ => false, + }, + } + } else { + false + }; + + let size_format = if options.is_present(options::HUMAN_READABLE) { + SizeFormats::Binary + } else { + SizeFormats::Bytes + }; + + Config { + display, + files, + sort, + + recursive: options.is_present(options::RECURSIVE), + reverse: options.is_present(options::REVERSE), + dereference: options.is_present(options::DEREFERENCE), + classify: options.is_present(options::CLASSIFY), + ignore_backups: options.is_present(options::IGNORE_BACKUPS), + size_format, + numeric_uid_gid: options.is_present(options::NUMERIC_UID_GID), + directory: options.is_present(options::DIRECTORY), + #[cfg(unix)] + color, + #[cfg(unix)] + inode: options.is_present(options::INODE), + } + } +} + pub fn uumain(args: impl uucore::Args) -> i32 { let args = args.collect_str(); @@ -104,8 +218,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .about(ABOUT) .usage(&usage[..]) .arg( - Arg::with_name(options::ONE) - .short(options::ONE) + Arg::with_name(options ::ONELINE) + .short(options ::ONELINE) .help("list one file per line."), ) .arg( @@ -130,8 +244,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("Ignore entries which end with ~."), ) .arg( - Arg::with_name(options::COLUMNS) - .short(options::COLUMNS) + Arg::with_name(options::SORT_CTIME) + .short(options::SORT_CTIME) .help("If the long listing format (e.g., -l, -o) is being used, print the status \ change time (the ‘ctime’ in the inode) instead of the modification time. When \ explicitly sorting by time (--sort=time or -t) or when not using a long listing \ @@ -231,15 +345,15 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .arg(Arg::with_name(options::PATHS).multiple(true).takes_value(true)) .get_matches_from(args); - list(matches) -} - -fn list(options: clap::ArgMatches) -> i32 { - let locs: Vec = options + let locs = matches .values_of(options::PATHS) .map(|v| v.map(ToString::to_string).collect()) .unwrap_or_else(|| vec![String::from(".")]); + list(locs, Config::from(matches)) +} + +fn list(locs: Vec, config: Config) -> i32 { let number_of_locs = locs.len(); let mut files = Vec::::new(); @@ -256,9 +370,9 @@ fn list(options: clap::ArgMatches) -> i32 { } let mut dir = false; - if p.is_dir() && !options.is_present(options::DIRECTORY) { + if p.is_dir() && !config.directory { dir = true; - if options.is_present(options::LONG) && !options.is_present(options::DEREFERENCE) { + if config.display == DisplayOptions::Long && !config.dereference { if let Ok(md) = p.symlink_metadata() { if md.file_type().is_symlink() && !p.ends_with("/") { dir = false; @@ -272,15 +386,15 @@ fn list(options: clap::ArgMatches) -> i32 { files.push(p); } } - sort_entries(&mut files, &options); - display_items(&files, None, &options); + sort_entries(&mut files, &config); + display_items(&files, None, &config); - sort_entries(&mut dirs, &options); + sort_entries(&mut dirs, &config); for dir in dirs { if number_of_locs > 1 { println!("\n{}:", dir.to_string_lossy()); } - enter_directory(&dir, &options); + enter_directory(&dir, &config); } if has_failed { 1 @@ -290,128 +404,118 @@ fn list(options: clap::ArgMatches) -> i32 { } #[cfg(any(unix, target_os = "redox"))] -fn sort_entries(entries: &mut Vec, options: &clap::ArgMatches) { - let mut reverse = options.is_present(options::REVERSE); - if options.is_present(options::SORT_TIME) { - if options.is_present(options::COLUMNS) { - entries.sort_by_key(|k| { - Reverse(get_metadata(k, options).map(|md| md.ctime()).unwrap_or(0)) - }); - } else { - entries.sort_by_key(|k| { - // Newest first - Reverse( - get_metadata(k, options) - .and_then(|md| md.modified()) - .unwrap_or(std::time::UNIX_EPOCH), - ) - }); - } - } else if options.is_present(options::SORT_SIZE) { - entries.sort_by_key(|k| get_metadata(k, options).map(|md| md.size()).unwrap_or(0)); - reverse = !reverse; - } else if !options.is_present(options::SORT_NONE) { - entries.sort(); - } - - if reverse { - entries.reverse(); - } -} - -#[cfg(windows)] -fn is_hidden(file_path: &DirEntry) -> std::io::Result { - let metadata = fs::metadata(file_path.path())?; - let attr = metadata.file_attributes(); - Ok(((attr & 0x2) > 0) || file_path.file_name().to_string_lossy().starts_with('.')) -} - -#[cfg(unix)] -fn is_hidden(file_path: &DirEntry) -> std::io::Result { - Ok(file_path.file_name().to_string_lossy().starts_with('.')) -} - -#[cfg(windows)] -fn sort_entries(entries: &mut Vec, options: &clap::ArgMatches) { - let mut reverse = options.is_present(options::REVERSE); - if options.is_present(options::SORT_TIME) { - entries.sort_by_key(|k| { - // Newest first +fn sort_entries(entries: &mut Vec, config: &Config) { + match config.sort { + Sort::CTime => entries + .sort_by_key(|k| Reverse(get_metadata(k, config).map(|md| md.ctime()).unwrap_or(0))), + Sort::Time => entries.sort_by_key(|k| { Reverse( - get_metadata(k, options) + get_metadata(k, config) .and_then(|md| md.modified()) .unwrap_or(std::time::UNIX_EPOCH), ) - }); - } else if options.is_present(options::SORT_SIZE) { - entries.sort_by_key(|k| { - get_metadata(k, options) - .map(|md| md.file_size()) - .unwrap_or(0) - }); - reverse = !reverse; - } else if !options.is_present(options::SORT_NONE) { - entries.sort(); + }), + Sort::Size => entries + .sort_by_key(|k| Reverse(get_metadata(k, config).map(|md| md.size()).unwrap_or(0))), + Sort::Name => entries.sort(), + Sort::None => {} } - if reverse { + if config.reverse { entries.reverse(); } } -fn should_display(entry: &DirEntry, options: &clap::ArgMatches) -> bool { +#[cfg(windows)] +fn is_hidden(file_path: &DirEntry) -> bool { + let metadata = fs::metadata(file_path.path()).unwrap(); + let attr = metadata.file_attributes(); + ((attr & 0x2) > 0) || file_path.file_name().to_string_lossy().starts_with('.') +} + +#[cfg(unix)] +fn is_hidden(file_path: &DirEntry) -> bool { + file_path.file_name().to_string_lossy().starts_with('.') +} + +#[cfg(windows)] +fn sort_entries(entries: &mut Vec, config: &Config) { + match config.sort { + Sort::CTime | Sort::Time => entries.sort_by_key(|k| { + // Newest first + Reverse( + get_metadata(k, config) + .and_then(|md| md.modified()) + .unwrap_or(std::time::UNIX_EPOCH), + ) + }), + Sort::Size => entries.sort_by_key(|k| { + // Largest first + Reverse( + get_metadata(k, config) + .map(|md| md.file_size()) + .unwrap_or(0), + ) + }), + Sort::Name => entries.sort(), + Sort::None => {}, + } + + if config.reverse { + entries.reverse(); + } +} + +fn should_display(entry: &DirEntry, config: &Config) -> bool { let ffi_name = entry.file_name(); let name = ffi_name.to_string_lossy(); - if !options.is_present(options::ALL) - && !options.is_present(options::ALMOST_ALL) - && is_hidden(entry).unwrap() - { + if config.files == Files::Normal && is_hidden(entry) { return false; } - if options.is_present(options::IGNORE_BACKUPS) && name.ends_with('~') { + if config.ignore_backups && name.ends_with('~') { return false; } true } -fn enter_directory(dir: &PathBuf, options: &clap::ArgMatches) { +fn enter_directory(dir: &PathBuf, config: &Config) { let mut entries: Vec<_> = safe_unwrap!(fs::read_dir(dir).and_then(Iterator::collect)); - entries.retain(|e| should_display(e, options)); + entries.retain(|e| should_display(e, config)); let mut entries: Vec<_> = entries.iter().map(DirEntry::path).collect(); - sort_entries(&mut entries, options); + sort_entries(&mut entries, config); - if options.is_present(options::ALL) { + if config.files == Files::All { let mut display_entries = entries.clone(); display_entries.insert(0, dir.join("..")); display_entries.insert(0, dir.join(".")); - display_items(&display_entries, Some(dir), options); + display_items(&display_entries, Some(dir), config); } else { - display_items(&entries, Some(dir), options); + display_items(&entries, Some(dir), config); } - if options.is_present(options::RECURSIVE) { + if config.recursive { for e in entries.iter().filter(|p| p.is_dir()) { println!("\n{}:", e.to_string_lossy()); - enter_directory(&e, options); + enter_directory(&e, config); } } } -fn get_metadata(entry: &PathBuf, options: &clap::ArgMatches) -> std::io::Result { - if options.is_present(options::DEREFERENCE) { +fn get_metadata(entry: &PathBuf, config: &Config) -> std::io::Result { + if config.dereference { entry.metadata().or_else(|_| entry.symlink_metadata()) } else { entry.symlink_metadata() } } -fn display_dir_entry_size(entry: &PathBuf, options: &clap::ArgMatches) -> (usize, usize) { - if let Ok(md) = get_metadata(entry, options) { +fn display_dir_entry_size(entry: &PathBuf, config: &Config) -> (usize, usize) { + if let Ok(md) = get_metadata(entry, config) { ( display_symlink_count(&md).len(), - display_file_size(&md, options).len(), + display_file_size(&md, config).len(), ) } else { (0, 0) @@ -422,28 +526,28 @@ fn pad_left(string: String, count: usize) -> String { format!("{:>width$}", string, width = count) } -fn display_items(items: &[PathBuf], strip: Option<&Path>, options: &clap::ArgMatches) { - if options.is_present(options::LONG) || options.is_present(options::NUMERIC_UID_GID) { +fn display_items(items: &[PathBuf], strip: Option<&Path>, config: &Config) { + if config.display == DisplayOptions::Long || config.numeric_uid_gid { let (mut max_links, mut max_size) = (1, 1); for item in items { - let (links, size) = display_dir_entry_size(item, options); + let (links, size) = display_dir_entry_size(item, config); max_links = links.max(max_links); max_size = size.max(max_size); } for item in items { - display_item_long(item, strip, max_links, max_size, options); + display_item_long(item, strip, max_links, max_size, config); } } else { - if !options.is_present(options::ONE) { + if config.display != DisplayOptions::OneLine { let names = items.iter().filter_map(|i| { - let md = get_metadata(i, options); + let md = get_metadata(i, config); match md { Err(e) => { let filename = get_file_name(i, strip); show_error!("'{}': {}", filename, e); None } - Ok(md) => Some(display_file_name(&i, strip, &md, options)), + Ok(md) => Some(display_file_name(&i, strip, &md, config)), } }); @@ -467,9 +571,9 @@ fn display_items(items: &[PathBuf], strip: Option<&Path>, options: &clap::ArgMat // Couldn't display a grid, either because we don't know // the terminal width or because fit_into_width failed for i in items { - let md = get_metadata(i, options); + let md = get_metadata(i, config); if let Ok(md) = md { - println!("{}", display_file_name(&i, strip, &md, options).contents); + println!("{}", display_file_name(&i, strip, &md, config).contents); } } } @@ -482,9 +586,9 @@ fn display_item_long( strip: Option<&Path>, max_links: usize, max_size: usize, - options: &clap::ArgMatches, + config: &Config, ) { - let md = match get_metadata(item, options) { + let md = match get_metadata(item, config) { Err(e) => { let filename = get_file_name(&item, strip); show_error!("{}: {}", filename, e); @@ -495,21 +599,21 @@ fn display_item_long( println!( "{}{}{} {} {} {} {} {} {}", - get_inode(&md, options), + get_inode(&md, config), display_file_type(md.file_type()), display_permissions(&md), pad_left(display_symlink_count(&md), max_links), - display_uname(&md, options), - display_group(&md, options), - pad_left(display_file_size(&md, options), max_size), - display_date(&md, options), - display_file_name(&item, strip, &md, options).contents + display_uname(&md, config), + display_group(&md, config), + pad_left(display_file_size(&md, config), max_size), + display_date(&md, config), + display_file_name(&item, strip, &md, config).contents ); } #[cfg(unix)] -fn get_inode(metadata: &Metadata, options: &clap::ArgMatches) -> String { - if options.is_present(options::INODE) { +fn get_inode(metadata: &Metadata, config: &Config) -> String { + if config.inode { format!("{:8} ", metadata.ino()) } else { "".to_string() @@ -517,7 +621,7 @@ fn get_inode(metadata: &Metadata, options: &clap::ArgMatches) -> String { } #[cfg(not(unix))] -fn get_inode(_metadata: &Metadata, _options: &clap::ArgMatches) -> String { +fn get_inode(_metadata: &Metadata, _config: &Config) -> String { "".to_string() } @@ -527,8 +631,8 @@ fn get_inode(_metadata: &Metadata, _options: &clap::ArgMatches) -> String { use uucore::entries; #[cfg(unix)] -fn display_uname(metadata: &Metadata, options: &clap::ArgMatches) -> String { - if options.is_present(options::NUMERIC_UID_GID) { +fn display_uname(metadata: &Metadata, config: &Config) -> String { + if config.numeric_uid_gid { metadata.uid().to_string() } else { entries::uid2usr(metadata.uid()).unwrap_or_else(|_| metadata.uid().to_string()) @@ -536,8 +640,8 @@ fn display_uname(metadata: &Metadata, options: &clap::ArgMatches) -> String { } #[cfg(unix)] -fn display_group(metadata: &Metadata, options: &clap::ArgMatches) -> String { - if options.is_present(options::NUMERIC_UID_GID) { +fn display_group(metadata: &Metadata, config: &Config) -> String { + if config.numeric_uid_gid { metadata.gid().to_string() } else { entries::gid2grp(metadata.gid()).unwrap_or_else(|_| metadata.gid().to_string()) @@ -545,31 +649,29 @@ fn display_group(metadata: &Metadata, options: &clap::ArgMatches) -> String { } #[cfg(not(unix))] -#[allow(unused_variables)] -fn display_uname(metadata: &Metadata, _options: &clap::ArgMatches) -> String { +fn display_uname(_metadata: &Metadata, _config: &Config) -> String { "somebody".to_string() } #[cfg(not(unix))] #[allow(unused_variables)] -fn display_group(metadata: &Metadata, _options: &clap::ArgMatches) -> String { +fn display_group(_metadata: &Metadata, _config: &Config) -> String { "somegroup".to_string() } #[cfg(unix)] -fn display_date(metadata: &Metadata, options: &clap::ArgMatches) -> String { - let secs = if options.is_present(options::COLUMNS) { - metadata.ctime() - } else { - metadata.mtime() +fn display_date(metadata: &Metadata, config: &Config) -> String { + let secs = match config.sort { + Sort::CTime => metadata.ctime(), + Sort::Time => metadata.mtime(), + _ => 0, }; let time = time::at(Timespec::new(secs, 0)); strftime("%F %R", &time).unwrap() } #[cfg(not(unix))] -#[allow(unused_variables)] -fn display_date(metadata: &Metadata, options: &clap::ArgMatches) -> String { +fn display_date(metadata: &Metadata, _config: &Config) -> String { if let Ok(mtime) = metadata.modified() { let time = time::at(Timespec::new( mtime @@ -584,18 +686,17 @@ fn display_date(metadata: &Metadata, options: &clap::ArgMatches) -> String { } } -fn display_file_size(metadata: &Metadata, options: &clap::ArgMatches) -> String { +fn display_file_size(metadata: &Metadata, config: &Config) -> String { // NOTE: The human-readable behaviour deviates from the GNU ls. // The GNU ls uses binary prefixes by default. - if options.is_present(options::HUMAN_READABLE) { - match NumberPrefix::decimal(metadata.len() as f64) { + match config.size_format { + SizeFormats::Binary => match NumberPrefix::decimal(metadata.len() as f64) { NumberPrefix::Standalone(bytes) => bytes.to_string(), NumberPrefix::Prefixed(prefix, bytes) => { format!("{:.2}{}", bytes, prefix).to_uppercase() } - } - } else { - metadata.len().to_string() + }, + SizeFormats::Bytes => metadata.len().to_string(), } } @@ -625,15 +726,15 @@ fn display_file_name( path: &Path, strip: Option<&Path>, metadata: &Metadata, - options: &clap::ArgMatches, + config: &Config, ) -> Cell { let mut name = get_file_name(path, strip); - if !options.is_present(options::LONG) { - name = get_inode(metadata, options) + &name; + if config.display == DisplayOptions::Long { + name = get_inode(metadata, config) + &name; } - if options.is_present(options::CLASSIFY) { + if config.classify { let file_type = metadata.file_type(); if file_type.is_dir() { name.push('/'); @@ -642,7 +743,7 @@ fn display_file_name( } } - if options.is_present(options::LONG) && metadata.file_type().is_symlink() { + if config.display == DisplayOptions::Long && metadata.file_type().is_symlink() { if let Ok(target) = path.read_link() { // We don't bother updating width here because it's not used for long listings let target_name = target.to_string_lossy().to_string(); @@ -687,30 +788,17 @@ fn display_file_name( path: &Path, strip: Option<&Path>, metadata: &Metadata, - options: &clap::ArgMatches, + config: &Config, ) -> Cell { let mut name = get_file_name(path, strip); - if !options.is_present(options::LONG) { - name = get_inode(metadata, options) + &name; + if config.display != DisplayOptions::Long { + name = get_inode(metadata, config) + &name; } let mut width = UnicodeWidthStr::width(&*name); - let color = if options.is_present(options::COLOR) { - match options.value_of(options::COLOR) { - None => atty::is(atty::Stream::Stdout), - Some(val) => match val { - "" | "always" | "yes" | "force" => true, - "auto" | "tty" | "if-tty" => atty::is(atty::Stream::Stdout), - /* "never" | "no" | "none" | */ _ => false, - }, - } - } else { - false - }; - let classify = options.is_present(options::CLASSIFY); let ext; - if color || classify { + if config.color || config.classify { let file_type = metadata.file_type(); let (code, sym) = if file_type.is_dir() { @@ -760,10 +848,10 @@ fn display_file_name( ("", None) }; - if color { + if config.color { name = color_name(name, code); } - if classify { + if config.classify { if let Some(s) = sym { name.push(s); width += 1; @@ -771,7 +859,7 @@ fn display_file_name( } } - if options.is_present(options::LONG) && metadata.file_type().is_symlink() { + if config.display == DisplayOptions::Long && metadata.file_type().is_symlink() { if let Ok(target) = path.read_link() { // We don't bother updating width here because it's not used for long listings let code = if target.exists() { "fi" } else { "mi" }; @@ -788,8 +876,7 @@ fn display_file_name( } #[cfg(not(unix))] -#[allow(unused_variables)] -fn display_symlink_count(metadata: &Metadata) -> String { +fn display_symlink_count(_metadata: &Metadata) -> String { // Currently not sure of how to get this on Windows, so I'm punting. // Git Bash looks like it may do the same thing. String::from("1") From 0717a5f3014b52b862c177222afcd257cbe633f3 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sun, 14 Mar 2021 13:32:15 +0100 Subject: [PATCH 08/21] ls: formatting --- src/uu/ls/src/ls.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index d61a45702..d9bbd0f88 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -122,7 +122,6 @@ struct Config { display: DisplayOptions, files: Files, sort: Sort, - recursive: bool, reverse: bool, dereference: bool, @@ -191,7 +190,7 @@ impl Config { display, files, sort, - + recursive: options.is_present(options::RECURSIVE), reverse: options.is_present(options::REVERSE), dereference: options.is_present(options::DEREFERENCE), @@ -458,7 +457,7 @@ fn sort_entries(entries: &mut Vec, config: &Config) { ) }), Sort::Name => entries.sort(), - Sort::None => {}, + Sort::None => {} } if config.reverse { From 5d7a8514715ecc815a777834c37c9a9e81a80b5c Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sun, 14 Mar 2021 21:30:21 +0100 Subject: [PATCH 09/21] ls: fix --color behaviour --- src/uu/ls/src/ls.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index d9bbd0f88..20fffdedc 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -56,7 +56,7 @@ lazy_static! { let codes = LS_COLORS.split(':'); let mut map = HashMap::new(); for c in codes { - let p: Vec<_> = c.splitn(1, '=').collect(); + let p: Vec<_> = c.splitn(2, '=').collect(); if p.len() == 2 { map.insert(p[0], p[1]); } @@ -337,9 +337,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Arg::with_name(options::COLOR) .long(options::COLOR) .help("Color output based on file type.") - .possible_values(&["always", "yes", "force", "tty", "if-tty", "auto", "never", "no", "none"]) + .takes_value(true) .require_equals(true) - .empty_values(true), + .min_values(0), ) .arg(Arg::with_name(options::PATHS).multiple(true).takes_value(true)) .get_matches_from(args); From c454d2640cfdee55337f0cb128184873e33ff2c3 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sun, 14 Mar 2021 21:32:21 +0100 Subject: [PATCH 10/21] ls: structure options some more --- src/uu/ls/src/ls.rs | 110 +++++++++++++++++++++++++++++++------------- 1 file changed, 77 insertions(+), 33 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 20fffdedc..0d8d9e3c3 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -7,6 +7,45 @@ // spell-checker:ignore (ToDO) cpio svgz webm somegroup nlink rmvb xspf +// Missing features from GNU Coreutils: +// --author +// -b, --escape +// --block-size=SIZE +// -c +// -D, --Dired +// -f +// --file-type +// --format=WORD +// --full-time +// -g +// --group-directories-first +// -G, --no-group +// --si +// -H, --dereference-command-line +// --dereference-command-line-symlink-to-dir +// --hide=PATTERN +// --hyperlink[=WHEN] +// --indicator-style=WORD +// -I, --ignore +// -k, --kibibytes +// -m +// -N, --literal +// -o +// -p, --indicator-style=slash +// -q, --hide-control-chars +// --show-control-chars +// -Q, --quote-name +// --quoting-style=WORD +// --time=WORD +// --time-style=TIME_STYLE +// -T, --tabsize=COLS +// -u +// -v +// -w, --width=COLS +// -x +// -X +// -Z, --context + #[cfg(unix)] #[macro_use] extern crate lazy_static; @@ -70,23 +109,29 @@ lazy_static! { } pub mod options { - pub static ONELINE: &str = "1"; - pub static ALL: &str = "all"; - pub static ALMOST_ALL: &str = "almost-all"; + pub mod display { + pub static ONELINE: &str = "1"; + pub static LONG: &str = "long"; + } + pub mod files { + pub static ALL: &str = "all"; + pub static ALMOST_ALL: &str = "almost-all"; + } + pub mod sort { + pub static SIZE: &str = "S"; + pub static TIME: &str = "t"; + pub static NONE: &str = "U"; + pub static CTIME: &str = "c"; + } pub static IGNORE_BACKUPS: &str = "ignore-backups"; pub static DIRECTORY: &str = "directory"; pub static CLASSIFY: &str = "classify"; pub static HUMAN_READABLE: &str = "human-readable"; pub static INODE: &str = "inode"; pub static DEREFERENCE: &str = "dereference"; - pub static LONG: &str = "long"; pub static NUMERIC_UID_GID: &str = "numeric-uid-gid"; pub static REVERSE: &str = "reverse"; pub static RECURSIVE: &str = "recursive"; - pub static SORT_SIZE: &str = "S"; - pub static SORT_TIME: &str = "t"; - pub static SORT_NONE: &str = "U"; - pub static SORT_CTIME: &str = "c"; pub static COLOR: &str = "color"; pub static PATHS: &str = "paths"; } @@ -138,29 +183,29 @@ struct Config { impl Config { fn from(options: clap::ArgMatches) -> Config { - let display = if options.is_present(options::LONG) { + let display = if options.is_present(options::display::LONG) { DisplayOptions::Long - } else if options.is_present(options::ONELINE) { + } else if options.is_present(options::display::ONELINE) { DisplayOptions::OneLine } else { DisplayOptions::Columns }; - let files = if options.is_present(options::ALL) { + let files = if options.is_present(options::files::ALL) { Files::All - } else if options.is_present(options::ALMOST_ALL) { + } else if options.is_present(options::files::ALMOST_ALL) { Files::AlmostAll } else { Files::Normal }; - let sort = if options.is_present(options::SORT_TIME) { + let sort = if options.is_present(options::sort::TIME) { Sort::Time - } else if options.is_present(options::SORT_CTIME) { + } else if options.is_present(options::sort::CTIME) { Sort::CTime - } else if options.is_present(options::SORT_SIZE) { + } else if options.is_present(options::sort::SIZE) { Sort::Size - } else if options.is_present(options::SORT_NONE) { + } else if options.is_present(options::sort::NONE) { Sort::None } else { Sort::Name @@ -190,7 +235,6 @@ impl Config { display, files, sort, - recursive: options.is_present(options::RECURSIVE), reverse: options.is_present(options::REVERSE), dereference: options.is_present(options::DEREFERENCE), @@ -217,20 +261,20 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .about(ABOUT) .usage(&usage[..]) .arg( - Arg::with_name(options ::ONELINE) - .short(options ::ONELINE) + Arg::with_name(options::display::ONELINE) + .short(options::display::ONELINE) .help("list one file per line."), ) .arg( - Arg::with_name(options::ALL) + Arg::with_name(options::files::ALL) .short("a") - .long(options::ALL) + .long(options::files::ALL) .help("Do not ignore hidden files (files with names that start with '.')."), ) .arg( - Arg::with_name(options::ALMOST_ALL) + Arg::with_name(options::files::ALMOST_ALL) .short("A") - .long(options::ALMOST_ALL) + .long(options::files::ALMOST_ALL) .help( "In a directory, do not ignore all file names that start with '.', only ignore \ '.' and '..'.", @@ -243,8 +287,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("Ignore entries which end with ~."), ) .arg( - Arg::with_name(options::SORT_CTIME) - .short(options::SORT_CTIME) + Arg::with_name(options::sort::CTIME) + .short(options::sort::CTIME) .help("If the long listing format (e.g., -l, -o) is being used, print the status \ change time (the ‘ctime’ in the inode) instead of the modification time. When \ explicitly sorting by time (--sort=time or -t) or when not using a long listing \ @@ -292,9 +336,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ), ) .arg( - Arg::with_name(options::LONG) + Arg::with_name(options::display::LONG) .short("l") - .long(options::LONG) + .long(options::display::LONG) .help("Display detailed information."), ) .arg( @@ -317,18 +361,18 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("List the contents of all directories recursively."), ) .arg( - Arg::with_name(options::SORT_SIZE) - .short(options::SORT_SIZE) + Arg::with_name(options::sort::SIZE) + .short(options::sort::SIZE) .help("Sort by file size, largest first."), ) .arg( - Arg::with_name(options::SORT_TIME) - .short(options::SORT_TIME) + Arg::with_name(options::sort::TIME) + .short(options::sort::TIME) .help("Sort by modification time (the 'mtime' in the inode), newest first."), ) .arg( - Arg::with_name(options::SORT_NONE) - .short(options::SORT_NONE) + Arg::with_name(options::sort::NONE) + .short(options::sort::NONE) .help("Do not sort; list the files in whatever order they are stored in the \ directory. This is especially useful when listing very large directories, \ since not doing any sorting can be noticeably faster.", From c86c18cbb54ba33b175729cd362584d9df3b82a9 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sun, 14 Mar 2021 23:11:11 +0100 Subject: [PATCH 11/21] ls: implement -c and -u --- src/uu/ls/src/ls.rs | 90 +++++++++++++++++++++++++++++++-------------- 1 file changed, 63 insertions(+), 27 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 0d8d9e3c3..1719579fc 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -54,7 +54,6 @@ extern crate uucore; use clap::{App, Arg}; use number_prefix::NumberPrefix; -use std::cmp::Reverse; #[cfg(unix)] use std::collections::HashMap; use std::fs; @@ -66,6 +65,10 @@ use std::os::unix::fs::MetadataExt; #[cfg(windows)] use std::os::windows::fs::MetadataExt; use std::path::{Path, PathBuf}; +use std::cmp::Reverse; +#[cfg(not(unix))] +use std::time::{SystemTime, UNIX_EPOCH}; + use term_grid::{Cell, Direction, Filling, Grid, GridOptions}; use time::{strftime, Timespec}; #[cfg(unix)] @@ -121,7 +124,10 @@ pub mod options { pub static SIZE: &str = "S"; pub static TIME: &str = "t"; pub static NONE: &str = "U"; - pub static CTIME: &str = "c"; + } + pub mod time { + pub static ACCESS: &str = "u"; + pub static CHANGE: &str = "c"; } pub static IGNORE_BACKUPS: &str = "ignore-backups"; pub static DIRECTORY: &str = "directory"; @@ -148,7 +154,6 @@ enum Sort { Name, Size, Time, - CTime, } enum SizeFormats { @@ -163,6 +168,12 @@ enum Files { Normal, } +enum Time { + Modification, + Access, + Change, +} + struct Config { display: DisplayOptions, files: Files, @@ -175,6 +186,7 @@ struct Config { size_format: SizeFormats, numeric_uid_gid: bool, directory: bool, + time: Time, #[cfg(unix)] inode: bool, #[cfg(unix)] @@ -201,8 +213,6 @@ impl Config { let sort = if options.is_present(options::sort::TIME) { Sort::Time - } else if options.is_present(options::sort::CTIME) { - Sort::CTime } else if options.is_present(options::sort::SIZE) { Sort::Size } else if options.is_present(options::sort::NONE) { @@ -211,6 +221,14 @@ impl Config { Sort::Name }; + let time = if options.is_present(options::time::ACCESS) { + Time::Access + } else if options.is_present(options::time::CHANGE) { + Time::Change + } else { + Time::Modification + }; + #[cfg(unix)] let color = if options.is_present(options::COLOR) { match options.value_of(options::COLOR) { @@ -243,6 +261,7 @@ impl Config { size_format, numeric_uid_gid: options.is_present(options::NUMERIC_UID_GID), directory: options.is_present(options::DIRECTORY), + time, #[cfg(unix)] color, #[cfg(unix)] @@ -287,13 +306,21 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("Ignore entries which end with ~."), ) .arg( - Arg::with_name(options::sort::CTIME) - .short(options::sort::CTIME) - .help("If the long listing format (e.g., -l, -o) is being used, print the status \ + Arg::with_name(options::time::CHANGE) + .short(options::time::CHANGE) + .help("If the long listing format (e.g., -l, -o) is being used, print the status \ change time (the ‘ctime’ in the inode) instead of the modification time. When \ explicitly sorting by time (--sort=time or -t) or when not using a long listing \ format, sort according to the status change time.", )) + .arg( + Arg::with_name(options::time::ACCESS) + .short(options::time::ACCESS) + .help("If the long listing format (e.g., -l, -o) is being used, print the status \ + access time instead of the modification time. When explicitly sorting by time \ + (--sort=time or -t) or when not using a long listing format, sort according to the \ + status change time.") + ) .arg( Arg::with_name(options::DIRECTORY) .short("d") @@ -449,13 +476,11 @@ fn list(locs: Vec, config: Config) -> i32 { #[cfg(any(unix, target_os = "redox"))] fn sort_entries(entries: &mut Vec, config: &Config) { match config.sort { - Sort::CTime => entries - .sort_by_key(|k| Reverse(get_metadata(k, config).map(|md| md.ctime()).unwrap_or(0))), Sort::Time => entries.sort_by_key(|k| { Reverse( get_metadata(k, config) - .and_then(|md| md.modified()) - .unwrap_or(std::time::UNIX_EPOCH), + .map(|md| get_time(&md, config)) + .unwrap_or(0), ) }), Sort::Size => entries @@ -484,13 +509,9 @@ fn is_hidden(file_path: &DirEntry) -> bool { #[cfg(windows)] fn sort_entries(entries: &mut Vec, config: &Config) { match config.sort { - Sort::CTime | Sort::Time => entries.sort_by_key(|k| { + Sort::Time => entries.sort_by_key(|k| { // Newest first - Reverse( - get_metadata(k, config) - .and_then(|md| md.modified()) - .unwrap_or(std::time::UNIX_EPOCH), - ) + Reverse(get_time(get_metadata(k, config), config).unwrap_or(std::time::UNIX_EPOCH)) }), Sort::Size => entries.sort_by_key(|k| { // Largest first @@ -702,23 +723,38 @@ fn display_group(_metadata: &Metadata, _config: &Config) -> String { "somegroup".to_string() } +// The implementations for get_time are separated because some options, such +// as ctime will not be available +#[cfg(unix)] +fn get_time(md: &Metadata, config: &Config) -> i64 { + match config.time { + Time::Change => md.ctime(), + Time::Modification => md.mtime(), + Time::Access => md.atime(), + } +} + +#[cfg(not(unix))] +fn get_time(md: &Metadata, config: &Config) -> Option { + match config.time { + Time::Modification => md.modification().ok(), + Time::Access => md.access().ok(), + _ => None, + } +} + #[cfg(unix)] fn display_date(metadata: &Metadata, config: &Config) -> String { - let secs = match config.sort { - Sort::CTime => metadata.ctime(), - Sort::Time => metadata.mtime(), - _ => 0, - }; + let secs = get_time(metadata, config); let time = time::at(Timespec::new(secs, 0)); strftime("%F %R", &time).unwrap() } #[cfg(not(unix))] -fn display_date(metadata: &Metadata, _config: &Config) -> String { - if let Ok(mtime) = metadata.modified() { +fn display_date(metadata: &Metadata, config: &Config) -> String { + if let Some(time) = get_time(metadata, config) { let time = time::at(Timespec::new( - mtime - .duration_since(std::time::UNIX_EPOCH) + time.duration_since(std::time::UNIX_EPOCH) .unwrap() .as_secs() as i64, 0, From 7bde2e78a97ddc8d390c8391457db8a015b92c08 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sun, 14 Mar 2021 23:34:52 +0100 Subject: [PATCH 12/21] ls: simplify --color and remove it on windows --- src/uu/ls/src/ls.rs | 46 +++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 1719579fc..6f74d459b 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -138,6 +138,7 @@ pub mod options { pub static NUMERIC_UID_GID: &str = "numeric-uid-gid"; pub static REVERSE: &str = "reverse"; pub static RECURSIVE: &str = "recursive"; + #[cfg(unix)] pub static COLOR: &str = "color"; pub static PATHS: &str = "paths"; } @@ -230,17 +231,13 @@ impl Config { }; #[cfg(unix)] - let color = if options.is_present(options::COLOR) { - match options.value_of(options::COLOR) { - None => atty::is(atty::Stream::Stdout), - Some(val) => match val { - "" | "always" | "yes" | "force" => true, - "auto" | "tty" | "if-tty" => atty::is(atty::Stream::Stdout), - /* "never" | "no" | "none" | */ _ => false, - }, - } - } else { - false + let color = match options.value_of(options::COLOR) { + None => options.is_present(options::COLOR), + Some(val) => match val { + "" | "always" | "yes" | "force" => true, + "auto" | "tty" | "if-tty" => atty::is(atty::Stream::Stdout), + /* "never" | "no" | "none" | */ _ => false, + }, }; let size_format = if options.is_present(options::HUMAN_READABLE) { @@ -275,7 +272,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let usage = get_usage(); - let matches = App::new(executable!()) + let mut app = App::new(executable!()) .version(VERSION) .about(ABOUT) .usage(&usage[..]) @@ -319,7 +316,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("If the long listing format (e.g., -l, -o) is being used, print the status \ access time instead of the modification time. When explicitly sorting by time \ (--sort=time or -t) or when not using a long listing format, sort according to the \ - status change time.") + access time.") ) .arg( Arg::with_name(options::DIRECTORY) @@ -404,16 +401,21 @@ pub fn uumain(args: impl uucore::Args) -> i32 { directory. This is especially useful when listing very large directories, \ since not doing any sorting can be noticeably faster.", )) - .arg( + .arg(Arg::with_name(options::PATHS).multiple(true).takes_value(true)); + + #[cfg(unix)] + { + app = app.arg( Arg::with_name(options::COLOR) - .long(options::COLOR) - .help("Color output based on file type.") - .takes_value(true) - .require_equals(true) - .min_values(0), - ) - .arg(Arg::with_name(options::PATHS).multiple(true).takes_value(true)) - .get_matches_from(args); + .long(options::COLOR) + .help("Color output based on file type.") + .takes_value(true) + .require_equals(true) + .min_values(0), + ); + } + + let matches = app.get_matches_from(args); let locs = matches .values_of(options::PATHS) From 61a95239cee641da0dbb9993378d458588bc4296 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Mon, 15 Mar 2021 09:30:50 +0100 Subject: [PATCH 13/21] ls: rename display to format, set arg overrides --- src/uu/ls/src/ls.rs | 212 +++++++++++++++++++++++++++++++------------- 1 file changed, 152 insertions(+), 60 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 6f74d459b..69d1e251d 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -112,9 +112,10 @@ lazy_static! { } pub mod options { - pub mod display { + pub mod format { pub static ONELINE: &str = "1"; pub static LONG: &str = "long"; + pub static COLUMNS: &str = "C"; } pub mod files { pub static ALL: &str = "all"; @@ -129,6 +130,9 @@ pub mod options { pub static ACCESS: &str = "u"; pub static CHANGE: &str = "c"; } + pub static FORMAT: &str = "format"; + pub static SORT: &str = "sort"; + pub static TIME: &str = "time"; pub static IGNORE_BACKUPS: &str = "ignore-backups"; pub static DIRECTORY: &str = "directory"; pub static CLASSIFY: &str = "classify"; @@ -144,7 +148,7 @@ pub mod options { } #[derive(PartialEq, Eq)] -enum DisplayOptions { +enum Format { Columns, Long, OneLine, @@ -176,7 +180,7 @@ enum Time { } struct Config { - display: DisplayOptions, + format: Format, files: Files, sort: Sort, recursive: bool, @@ -196,12 +200,22 @@ struct Config { impl Config { fn from(options: clap::ArgMatches) -> Config { - let display = if options.is_present(options::display::LONG) { - DisplayOptions::Long - } else if options.is_present(options::display::ONELINE) { - DisplayOptions::OneLine + let format = if let Some(format_) = options.value_of(options::FORMAT) { + match format_ { + "long" | "verbose" => Format::Long, + "single-column" => Format::OneLine, + "columns" => Format::Columns, + // below should never happen as clap already restricts the values. + _ => panic!("Invalid field for --format") + } + } else if options.is_present(options::format::LONG) { + Format::Long + } else if options.is_present(options::format::ONELINE) { + Format::OneLine + } else if options.is_present(options::format::COLUMNS) { + Format::Columns } else { - DisplayOptions::Columns + Format::Columns }; let files = if options.is_present(options::files::ALL) { @@ -212,7 +226,16 @@ impl Config { Files::Normal }; - let sort = if options.is_present(options::sort::TIME) { + let sort = if let Some(field) = options.value_of(options::SORT) { + match field { + "none" => Sort::None, + "name" => Sort::Name, + "time" => Sort::Time, + "size" => Sort::Size, + // below should never happen as clap already restricts the values. + _ => panic!("Invalid field for --sort") + } + } else if options.is_present(options::sort::TIME) { Sort::Time } else if options.is_present(options::sort::SIZE) { Sort::Size @@ -222,7 +245,14 @@ impl Config { Sort::Name }; - let time = if options.is_present(options::time::ACCESS) { + let time = if let Some(field) = options.value_of(options::TIME) { + match field { + "ctime" | "status" => Time::Change, + "access" | "atime" | "use" => Time::Access, + // below should never happen as clap already restricts the values. + _ => panic!("Invalid field for --time") + } + } else if options.is_present(options::time::ACCESS) { Time::Access } else if options.is_present(options::time::CHANGE) { Time::Change @@ -247,7 +277,7 @@ impl Config { }; Config { - display, + format, files, sort, recursive: options.is_present(options::RECURSIVE), @@ -276,11 +306,110 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .version(VERSION) .about(ABOUT) .usage(&usage[..]) + + // Format arguments .arg( - Arg::with_name(options::display::ONELINE) - .short(options::display::ONELINE) - .help("list one file per line."), + Arg::with_name(options::FORMAT) + .long(options::FORMAT) + .help("Set the display format.") + .takes_value(true) + .possible_values(&["long", "verbose", "single-column", "columns"]) + .hide_possible_values(true) + .require_equals(true) + .overrides_with_all(&[ + options::FORMAT, + options::format::COLUMNS, + options::format::ONELINE, + options::format::LONG, + ]), ) + .arg( + Arg::with_name(options::format::COLUMNS) + .short(options::format::COLUMNS) + .help("Display the files in columns.") + ) + .arg( + Arg::with_name(options::format::ONELINE) + .short(options::format::ONELINE) + .help("List one file per line.") + ) + .arg( + Arg::with_name(options::format::LONG) + .short("l") + .long(options::format::LONG) + .help("Display detailed information.") + ) + + // Time arguments + .arg( + Arg::with_name(options::TIME) + .long(options::TIME) + .help("Show time in :\n\ + \taccess time (-u): atime, access, use;\n\ + \tchange time (-t): ctime, status.") + .value_name("field") + .takes_value(true) + .possible_values(&["atime", "access", "use", "ctime", "status"]) + .hide_possible_values(true) + .require_equals(true) + .overrides_with_all(&[ + options::TIME, + options::time::ACCESS, + options::time::CHANGE, + ]) + ) + .arg( + Arg::with_name(options::time::CHANGE) + .short(options::time::CHANGE) + .help("If the long listing format (e.g., -l, -o) is being used, print the status \ + change time (the ‘ctime’ in the inode) instead of the modification time. When \ + explicitly sorting by time (--sort=time or -t) or when not using a long listing \ + format, sort according to the status change time.", + )) + .arg( + Arg::with_name(options::time::ACCESS) + .short(options::time::ACCESS) + .help("If the long listing format (e.g., -l, -o) is being used, print the status \ + access time instead of the modification time. When explicitly sorting by time \ + (--sort=time or -t) or when not using a long listing format, sort according to the \ + access time.") + ) + + // Sort arguments + .arg( + Arg::with_name(options::SORT) + .long(options::SORT) + .help("Sort by : name, none (-U), time (-t) or size (-S)") + .value_name("field") + .takes_value(true) + .possible_values(&["name", "none", "time", "size"]) + .require_equals(true) + .overrides_with_all(&[ + options::SORT, + options::sort::SIZE, + options::sort::TIME, + options::sort::NONE, + ]) + ) + .arg( + Arg::with_name(options::sort::SIZE) + .short(options::sort::SIZE) + .help("Sort by file size, largest first."), + ) + .arg( + Arg::with_name(options::sort::TIME) + .short(options::sort::TIME) + .help("Sort by modification time (the 'mtime' in the inode), newest first."), + ) + .arg( + Arg::with_name(options::sort::NONE) + .short(options::sort::NONE) + .help("Do not sort; list the files in whatever order they are stored in the \ + directory. This is especially useful when listing very large directories, \ + since not doing any sorting can be noticeably faster.") + ) + + // Other Flags .arg( Arg::with_name(options::files::ALL) .short("a") @@ -302,22 +431,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .long(options::IGNORE_BACKUPS) .help("Ignore entries which end with ~."), ) - .arg( - Arg::with_name(options::time::CHANGE) - .short(options::time::CHANGE) - .help("If the long listing format (e.g., -l, -o) is being used, print the status \ - change time (the ‘ctime’ in the inode) instead of the modification time. When \ - explicitly sorting by time (--sort=time or -t) or when not using a long listing \ - format, sort according to the status change time.", - )) - .arg( - Arg::with_name(options::time::ACCESS) - .short(options::time::ACCESS) - .help("If the long listing format (e.g., -l, -o) is being used, print the status \ - access time instead of the modification time. When explicitly sorting by time \ - (--sort=time or -t) or when not using a long listing format, sort according to the \ - access time.") - ) .arg( Arg::with_name(options::DIRECTORY) .short("d") @@ -359,12 +472,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { file the link references rather than the link itself.", ), ) - .arg( - Arg::with_name(options::display::LONG) - .short("l") - .long(options::display::LONG) - .help("Display detailed information."), - ) .arg( Arg::with_name(options::NUMERIC_UID_GID) .short("n") @@ -384,23 +491,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .long(options::RECURSIVE) .help("List the contents of all directories recursively."), ) - .arg( - Arg::with_name(options::sort::SIZE) - .short(options::sort::SIZE) - .help("Sort by file size, largest first."), - ) - .arg( - Arg::with_name(options::sort::TIME) - .short(options::sort::TIME) - .help("Sort by modification time (the 'mtime' in the inode), newest first."), - ) - .arg( - Arg::with_name(options::sort::NONE) - .short(options::sort::NONE) - .help("Do not sort; list the files in whatever order they are stored in the \ - directory. This is especially useful when listing very large directories, \ - since not doing any sorting can be noticeably faster.", - )) + + // Positional arguments .arg(Arg::with_name(options::PATHS).multiple(true).takes_value(true)); #[cfg(unix)] @@ -444,7 +536,7 @@ fn list(locs: Vec, config: Config) -> i32 { if p.is_dir() && !config.directory { dir = true; - if config.display == DisplayOptions::Long && !config.dereference { + if config.format == Format::Long && !config.dereference { if let Ok(md) = p.symlink_metadata() { if md.file_type().is_symlink() && !p.ends_with("/") { dir = false; @@ -593,7 +685,7 @@ fn pad_left(string: String, count: usize) -> String { } fn display_items(items: &[PathBuf], strip: Option<&Path>, config: &Config) { - if config.display == DisplayOptions::Long || config.numeric_uid_gid { + if config.format == Format::Long || config.numeric_uid_gid { let (mut max_links, mut max_size) = (1, 1); for item in items { let (links, size) = display_dir_entry_size(item, config); @@ -604,7 +696,7 @@ fn display_items(items: &[PathBuf], strip: Option<&Path>, config: &Config) { display_item_long(item, strip, max_links, max_size, config); } } else { - if config.display != DisplayOptions::OneLine { + if config.format != Format::OneLine { let names = items.iter().filter_map(|i| { let md = get_metadata(i, config); match md { @@ -811,7 +903,7 @@ fn display_file_name( ) -> Cell { let mut name = get_file_name(path, strip); - if config.display == DisplayOptions::Long { + if config.format == Format::Long { name = get_inode(metadata, config) + &name; } @@ -824,7 +916,7 @@ fn display_file_name( } } - if config.display == DisplayOptions::Long && metadata.file_type().is_symlink() { + if config.format == Format::Long && metadata.file_type().is_symlink() { if let Ok(target) = path.read_link() { // We don't bother updating width here because it's not used for long listings let target_name = target.to_string_lossy().to_string(); @@ -872,7 +964,7 @@ fn display_file_name( config: &Config, ) -> Cell { let mut name = get_file_name(path, strip); - if config.display != DisplayOptions::Long { + if config.format != Format::Long { name = get_inode(metadata, config) + &name; } let mut width = UnicodeWidthStr::width(&*name); @@ -940,7 +1032,7 @@ fn display_file_name( } } - if config.display == DisplayOptions::Long && metadata.file_type().is_symlink() { + if config.format == Format::Long && metadata.file_type().is_symlink() { if let Ok(target) = path.read_link() { // We don't bother updating width here because it's not used for long listings let code = if target.exists() { "fi" } else { "mi" }; From 5656a717c9f7445ea9b2695255e47e449aa2d183 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Mon, 15 Mar 2021 09:31:13 +0100 Subject: [PATCH 14/21] ls: make name sort case insensitive --- src/uu/ls/src/ls.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 69d1e251d..7e98a7fd3 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -579,7 +579,8 @@ fn sort_entries(entries: &mut Vec, config: &Config) { }), Sort::Size => entries .sort_by_key(|k| Reverse(get_metadata(k, config).map(|md| md.size()).unwrap_or(0))), - Sort::Name => entries.sort(), + // The default sort in GNU ls is case insensitive + Sort::Name => entries.sort_by_key(|k| k.to_string_lossy().to_lowercase()), Sort::None => {} } From 01fd207c81dc09b7efc7cff411b345c5441b98d4 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Mon, 15 Mar 2021 09:53:19 +0100 Subject: [PATCH 15/21] ls: remove list of missing features --- src/uu/ls/src/ls.rs | 39 --------------------------------------- 1 file changed, 39 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 7e98a7fd3..c2d544a2c 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -7,45 +7,6 @@ // spell-checker:ignore (ToDO) cpio svgz webm somegroup nlink rmvb xspf -// Missing features from GNU Coreutils: -// --author -// -b, --escape -// --block-size=SIZE -// -c -// -D, --Dired -// -f -// --file-type -// --format=WORD -// --full-time -// -g -// --group-directories-first -// -G, --no-group -// --si -// -H, --dereference-command-line -// --dereference-command-line-symlink-to-dir -// --hide=PATTERN -// --hyperlink[=WHEN] -// --indicator-style=WORD -// -I, --ignore -// -k, --kibibytes -// -m -// -N, --literal -// -o -// -p, --indicator-style=slash -// -q, --hide-control-chars -// --show-control-chars -// -Q, --quote-name -// --quoting-style=WORD -// --time=WORD -// --time-style=TIME_STYLE -// -T, --tabsize=COLS -// -u -// -v -// -w, --width=COLS -// -x -// -X -// -Z, --context - #[cfg(unix)] #[macro_use] extern crate lazy_static; From a4c79c92ae3172164a5adb8a1badaedf82a7f3de Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Mon, 15 Mar 2021 10:24:24 +0100 Subject: [PATCH 16/21] ls: fix windows issues --- src/uu/ls/src/ls.rs | 109 +++++++++++++++----------------------------- 1 file changed, 38 insertions(+), 71 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index c2d544a2c..187b39006 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -15,6 +15,7 @@ extern crate uucore; use clap::{App, Arg}; use number_prefix::NumberPrefix; +use std::cmp::Reverse; #[cfg(unix)] use std::collections::HashMap; use std::fs; @@ -26,9 +27,6 @@ use std::os::unix::fs::MetadataExt; #[cfg(windows)] use std::os::windows::fs::MetadataExt; use std::path::{Path, PathBuf}; -use std::cmp::Reverse; -#[cfg(not(unix))] -use std::time::{SystemTime, UNIX_EPOCH}; use term_grid::{Cell, Direction, Filling, Grid, GridOptions}; use time::{strftime, Timespec}; @@ -167,7 +165,7 @@ impl Config { "single-column" => Format::OneLine, "columns" => Format::Columns, // below should never happen as clap already restricts the values. - _ => panic!("Invalid field for --format") + _ => panic!("Invalid field for --format"), } } else if options.is_present(options::format::LONG) { Format::Long @@ -194,7 +192,7 @@ impl Config { "time" => Sort::Time, "size" => Sort::Size, // below should never happen as clap already restricts the values. - _ => panic!("Invalid field for --sort") + _ => panic!("Invalid field for --sort"), } } else if options.is_present(options::sort::TIME) { Sort::Time @@ -211,7 +209,7 @@ impl Config { "ctime" | "status" => Time::Change, "access" | "atime" | "use" => Time::Access, // below should never happen as clap already restricts the values. - _ => panic!("Invalid field for --time") + _ => panic!("Invalid field for --time"), } } else if options.is_present(options::time::ACCESS) { Time::Access @@ -263,7 +261,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let usage = get_usage(); - let mut app = App::new(executable!()) + let app = App::new(executable!()) .version(VERSION) .about(ABOUT) .usage(&usage[..]) @@ -455,18 +453,18 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // Positional arguments .arg(Arg::with_name(options::PATHS).multiple(true).takes_value(true)); - + #[cfg(unix)] - { - app = app.arg( + let app = { + app.arg( Arg::with_name(options::COLOR) - .long(options::COLOR) - .help("Color output based on file type.") - .takes_value(true) - .require_equals(true) - .min_values(0), - ); - } + .long(options::COLOR) + .help("Color output based on file type.") + .takes_value(true) + .require_equals(true) + .min_values(0), + ) + }; let matches = app.get_matches_from(args); @@ -528,18 +526,18 @@ fn list(locs: Vec, config: Config) -> i32 { } } -#[cfg(any(unix, target_os = "redox"))] fn sort_entries(entries: &mut Vec, config: &Config) { match config.sort { Sort::Time => entries.sort_by_key(|k| { Reverse( get_metadata(k, config) - .map(|md| get_time(&md, config)) + .ok() + .and_then(|md| get_time(&md, config)) .unwrap_or(0), ) }), Sort::Size => entries - .sort_by_key(|k| Reverse(get_metadata(k, config).map(|md| md.size()).unwrap_or(0))), + .sort_by_key(|k| Reverse(get_metadata(k, config).map(|md| md.len()).unwrap_or(0))), // The default sort in GNU ls is case insensitive Sort::Name => entries.sort_by_key(|k| k.to_string_lossy().to_lowercase()), Sort::None => {} @@ -562,30 +560,6 @@ fn is_hidden(file_path: &DirEntry) -> bool { file_path.file_name().to_string_lossy().starts_with('.') } -#[cfg(windows)] -fn sort_entries(entries: &mut Vec, config: &Config) { - match config.sort { - Sort::Time => entries.sort_by_key(|k| { - // Newest first - Reverse(get_time(get_metadata(k, config), config).unwrap_or(std::time::UNIX_EPOCH)) - }), - Sort::Size => entries.sort_by_key(|k| { - // Largest first - Reverse( - get_metadata(k, config) - .map(|md| md.file_size()) - .unwrap_or(0), - ) - }), - Sort::Name => entries.sort(), - Sort::None => {} - } - - if config.reverse { - entries.reverse(); - } -} - fn should_display(entry: &DirEntry, config: &Config) -> bool { let ffi_name = entry.file_name(); let name = ffi_name.to_string_lossy(); @@ -782,42 +756,35 @@ fn display_group(_metadata: &Metadata, _config: &Config) -> String { // The implementations for get_time are separated because some options, such // as ctime will not be available #[cfg(unix)] -fn get_time(md: &Metadata, config: &Config) -> i64 { - match config.time { +fn get_time(md: &Metadata, config: &Config) -> Option { + Some(match config.time { Time::Change => md.ctime(), Time::Modification => md.mtime(), Time::Access => md.atime(), - } + }) } #[cfg(not(unix))] -fn get_time(md: &Metadata, config: &Config) -> Option { - match config.time { - Time::Modification => md.modification().ok(), - Time::Access => md.access().ok(), - _ => None, - } +fn get_time(md: &Metadata, config: &Config) -> Option { + let time = match config.time { + Time::Modification => md.modified().ok()?, + Time::Access => md.accessed().ok()?, + _ => return None, + }; + Some( + time.duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() as i64, + ) } -#[cfg(unix)] fn display_date(metadata: &Metadata, config: &Config) -> String { - let secs = get_time(metadata, config); - let time = time::at(Timespec::new(secs, 0)); - strftime("%F %R", &time).unwrap() -} - -#[cfg(not(unix))] -fn display_date(metadata: &Metadata, config: &Config) -> String { - if let Some(time) = get_time(metadata, config) { - let time = time::at(Timespec::new( - time.duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() as i64, - 0, - )); - strftime("%F %R", &time).unwrap() - } else { - "???".to_string() + match get_time(metadata, config) { + Some(secs) => { + let time = time::at(Timespec::new(secs, 0)); + strftime("%F %R", &time).unwrap() + } + None => "???".into(), } } From f28d5f4a73c19775c3892d09c808aa4a98f473c7 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Mon, 15 Mar 2021 12:07:10 +0100 Subject: [PATCH 17/21] ls: attempt to fix windows sorting issues --- src/uu/ls/src/ls.rs | 48 +++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 187b39006..90e4e2b22 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -27,6 +27,7 @@ use std::os::unix::fs::MetadataExt; #[cfg(windows)] use std::os::windows::fs::MetadataExt; use std::path::{Path, PathBuf}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use term_grid::{Cell, Direction, Filling, Grid, GridOptions}; use time::{strftime, Timespec}; @@ -532,8 +533,8 @@ fn sort_entries(entries: &mut Vec, config: &Config) { Reverse( get_metadata(k, config) .ok() - .and_then(|md| get_time(&md, config)) - .unwrap_or(0), + .and_then(|md| get_system_time(&md, config)) + .unwrap_or(UNIX_EPOCH), ) }), Sort::Size => entries @@ -756,34 +757,35 @@ fn display_group(_metadata: &Metadata, _config: &Config) -> String { // The implementations for get_time are separated because some options, such // as ctime will not be available #[cfg(unix)] -fn get_time(md: &Metadata, config: &Config) -> Option { - Some(match config.time { - Time::Change => md.ctime(), - Time::Modification => md.mtime(), - Time::Access => md.atime(), - }) +fn get_system_time(md: &Metadata, config: &Config) -> Option { + match config.time { + Time::Change => Some(UNIX_EPOCH + Duration::new(md.ctime() as u64, md.ctime_nsec() as u32)), + Time::Modification => md.modified().ok(), + Time::Access => md.accessed().ok(), + } } #[cfg(not(unix))] -fn get_time(md: &Metadata, config: &Config) -> Option { - let time = match config.time { - Time::Modification => md.modified().ok()?, - Time::Access => md.accessed().ok()?, - _ => return None, - }; - Some( - time.duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() as i64, - ) +fn get_system_time(md: &Metadata, config: &Config) -> Option { + match config.time { + Time::Modification => md.modified().ok(), + Time::Access => md.accessed().ok(), + _ => None, + } +} + +fn get_time(md: &Metadata, config: &Config) -> Option { + let duration = get_system_time(md, config)? + .duration_since(UNIX_EPOCH) + .ok()?; + let secs = duration.as_secs() as i64; + let nsec = duration.subsec_nanos() as i32; + Some(time::at(Timespec::new(secs, nsec))) } fn display_date(metadata: &Metadata, config: &Config) -> String { match get_time(metadata, config) { - Some(secs) => { - let time = time::at(Timespec::new(secs, 0)); - strftime("%F %R", &time).unwrap() - } + Some(time) => strftime("%F %R", &time).unwrap(), None => "???".into(), } } From 20094127c3cbe6e74dc77ed75c81944716a5c761 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Mon, 15 Mar 2021 12:21:08 +0100 Subject: [PATCH 18/21] ls: --color back on windows as noop --- src/uu/ls/src/ls.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 90e4e2b22..24adaa649 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -102,7 +102,6 @@ pub mod options { pub static NUMERIC_UID_GID: &str = "numeric-uid-gid"; pub static REVERSE: &str = "reverse"; pub static RECURSIVE: &str = "recursive"; - #[cfg(unix)] pub static COLOR: &str = "color"; pub static PATHS: &str = "paths"; } @@ -451,13 +450,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .long(options::RECURSIVE) .help("List the contents of all directories recursively."), ) - - // Positional arguments - .arg(Arg::with_name(options::PATHS).multiple(true).takes_value(true)); - - #[cfg(unix)] - let app = { - app.arg( + .arg( Arg::with_name(options::COLOR) .long(options::COLOR) .help("Color output based on file type.") @@ -465,7 +458,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .require_equals(true) .min_values(0), ) - }; + + // Positional arguments + .arg(Arg::with_name(options::PATHS).multiple(true).takes_value(true)); let matches = app.get_matches_from(args); From 10135dccef1d3c0844963831acadf1127dbdfb17 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Mon, 15 Mar 2021 13:46:21 +0100 Subject: [PATCH 19/21] ls: fix unused import and improve coverage --- src/uu/ls/src/ls.rs | 10 +-- tests/by-util/test_ls.rs | 127 +++++++++++++++++++++++++++++++++++---- 2 files changed, 120 insertions(+), 17 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 24adaa649..5c4cdaa80 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -27,7 +27,9 @@ use std::os::unix::fs::MetadataExt; #[cfg(windows)] use std::os::windows::fs::MetadataExt; use std::path::{Path, PathBuf}; -use std::time::{Duration, SystemTime, UNIX_EPOCH}; +#[cfg(unix)] +use std::time::Duration; +use std::time::{SystemTime, UNIX_EPOCH}; use term_grid::{Cell, Direction, Filling, Grid, GridOptions}; use time::{strftime, Timespec}; @@ -165,7 +167,7 @@ impl Config { "single-column" => Format::OneLine, "columns" => Format::Columns, // below should never happen as clap already restricts the values. - _ => panic!("Invalid field for --format"), + _ => unreachable!("Invalid field for --format"), } } else if options.is_present(options::format::LONG) { Format::Long @@ -192,7 +194,7 @@ impl Config { "time" => Sort::Time, "size" => Sort::Size, // below should never happen as clap already restricts the values. - _ => panic!("Invalid field for --sort"), + _ => unreachable!("Invalid field for --sort"), } } else if options.is_present(options::sort::TIME) { Sort::Time @@ -209,7 +211,7 @@ impl Config { "ctime" | "status" => Time::Change, "access" | "atime" | "use" => Time::Access, // below should never happen as clap already restricts the values. - _ => panic!("Invalid field for --time"), + _ => unreachable!("Invalid field for --time"), } } else if options.is_present(options::time::ACCESS) { Time::Access diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 422db8df9..e8ef18966 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -57,6 +57,36 @@ fn test_ls_a() { assert!(!result.stdout.contains("..")); } +#[test] +fn test_ls_columns() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch(&at.plus_as_string("test-columns-1")); + at.touch(&at.plus_as_string("test-columns-2")); + + // Columns is the default + let result = scene.ucmd().run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(result.success); + + #[cfg(not(windows))] + assert_eq!(result.stdout, "test-columns-1\ntest-columns-2\n"); + #[cfg(windows)] + assert_eq!(result.stdout, "test-columns-1 test-columns-2\n"); + + for option in &["-C", "--format=columns"] { + let result = scene.ucmd().arg(option).run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(result.success); + #[cfg(not(windows))] + assert_eq!(result.stdout, "test-columns-1\ntest-columns-2\n"); + #[cfg(windows)] + assert_eq!(result.stdout, "test-columns-1 test-columns-2\n"); + } +} + #[test] fn test_ls_long() { #[cfg(not(windows))] @@ -71,16 +101,20 @@ fn test_ls_long() { } } - let (at, mut ucmd) = at_and_ucmd!(); + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; at.touch(&at.plus_as_string("test-long")); - let result = ucmd.arg("-l").arg("test-long").succeeds(); - println!("stderr = {:?}", result.stderr); - println!("stdout = {:?}", result.stdout); - #[cfg(not(windows))] - assert!(result.stdout.contains("-rw-rw-r--")); - #[cfg(windows)] - assert!(result.stdout.contains("---------- 1 somebody somegroup")); + for arg in &["-l", "--long", "--format=long", "--format=verbose"] { + let result = scene.ucmd().arg(arg).arg("test-long").succeeds(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + #[cfg(not(windows))] + assert!(result.stdout.contains("-rw-rw-r--")); + + #[cfg(windows)] + assert!(result.stdout.contains("---------- 1 somebody somegroup")); + } #[cfg(not(windows))] { @@ -90,6 +124,24 @@ fn test_ls_long() { } } +#[test] +fn test_ls_oneline() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch(&at.plus_as_string("test-oneline-1")); + at.touch(&at.plus_as_string("test-oneline-2")); + + // Bit of a weird situation: in the tests oneline and columns have the same output, + // except on Windows. + for option in &["-1", "--format=single-column"] { + let result = scene.ucmd().arg(option).run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(result.success); + assert_eq!(result.stdout, "test-oneline-1\ntest-oneline-2\n"); + } +} + #[test] fn test_ls_deref() { let scene = TestScenario::new(util_name!()); @@ -166,27 +218,54 @@ fn test_ls_order_size() { } #[test] -fn test_ls_order_creation() { +fn test_ls_long_ctime() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.touch("test-long-ctime-1"); + let result = scene.ucmd().arg("-lc").succeeds(); + + // Should show the time on Unix, but question marks on windows. + #[cfg(unix)] + assert!(result.stdout.contains(":")); + #[cfg(not(unix))] + assert!(result.stdout.contains("???")); +} + +#[test] +fn test_ls_order_time() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; at.touch("test-1"); at.append("test-1", "1"); - sleep(Duration::from_millis(500)); + sleep(Duration::from_millis(100)); at.touch("test-2"); at.append("test-2", "22"); - sleep(Duration::from_millis(500)); + sleep(Duration::from_millis(100)); at.touch("test-3"); at.append("test-3", "333"); - sleep(Duration::from_millis(500)); + sleep(Duration::from_millis(100)); at.touch("test-4"); at.append("test-4", "4444"); + sleep(Duration::from_millis(100)); + + // Read test-3, only changing access time + at.read("test-3"); + + // Set permissions of test-2, only changing ctime + std::fs::set_permissions( + at.plus_as_string("test-2"), + at.metadata("test-2").permissions(), + ) + .unwrap(); let result = scene.ucmd().arg("-al").run(); println!("stderr = {:?}", result.stderr); println!("stdout = {:?}", result.stdout); assert!(result.success); + // ctime was changed at write, so the order is 4 3 2 1 let result = scene.ucmd().arg("-t").run(); println!("stderr = {:?}", result.stderr); println!("stdout = {:?}", result.stdout); @@ -196,7 +275,7 @@ fn test_ls_order_creation() { #[cfg(windows)] assert_eq!(result.stdout, "test-4 test-3 test-2 test-1\n"); - let result = scene.ucmd().arg("-t").arg("-r").run(); + let result = scene.ucmd().arg("-tr").run(); println!("stderr = {:?}", result.stderr); println!("stdout = {:?}", result.stdout); assert!(result.success); @@ -204,6 +283,28 @@ fn test_ls_order_creation() { assert_eq!(result.stdout, "test-1\ntest-2\ntest-3\ntest-4\n"); #[cfg(windows)] assert_eq!(result.stdout, "test-1 test-2 test-3 test-4\n"); + + // 3 was accessed last in the read + // So the order should be 2 3 4 1 + let result = scene.ucmd().arg("-tu").run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(result.success); + #[cfg(not(windows))] + assert_eq!(result.stdout, "test-3\ntest-4\ntest-2\ntest-1\n"); + #[cfg(windows)] + assert_eq!(result.stdout, "test-3 test-4 test-2 test-1\n"); + + // test-2 had the last ctime change when the permissions were set + // So the order should be 2 4 3 1 + #[cfg(unix)] + { + let result = scene.ucmd().arg("-tc").run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert!(result.success); + assert_eq!(result.stdout, "test-2\ntest-4\ntest-3\ntest-1\n"); + } } #[test] From fd957dd1487c1d25d733ae655cc945c339337145 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Mon, 15 Mar 2021 14:09:29 +0100 Subject: [PATCH 20/21] ls: fix access time on windows --- tests/by-util/test_ls.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index e8ef18966..fea912695 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -292,8 +292,11 @@ fn test_ls_order_time() { assert!(result.success); #[cfg(not(windows))] assert_eq!(result.stdout, "test-3\ntest-4\ntest-2\ntest-1\n"); + + // Access time does not seem to be set on Windows on read call + // so the order is 4 3 2 1 #[cfg(windows)] - assert_eq!(result.stdout, "test-3 test-4 test-2 test-1\n"); + assert_eq!(result.stdout, "test-4 test-3 test-2 test-1\n"); // test-2 had the last ctime change when the permissions were set // So the order should be 2 4 3 1 From c5792a4c477fdf80b3932935e452615b225832e1 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Wed, 17 Mar 2021 23:15:03 +0100 Subject: [PATCH 21/21] tests/ls: add tests for colors --- tests/by-util/test_ls.rs | 51 ++++++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index fea912695..52678b2fa 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -374,19 +374,56 @@ fn test_ls_recursive() { assert!(result.stdout.contains("a\\b:\nb")); } +#[cfg(unix)] #[test] fn test_ls_ls_color() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; at.mkdir("a"); + at.mkdir("a/nested_dir"); at.mkdir("z"); - at.touch(&at.plus_as_string("a/a")); - scene.ucmd().arg("--color").succeeds(); - scene.ucmd().arg("--color=always").succeeds(); - scene.ucmd().arg("--color=never").succeeds(); - scene.ucmd().arg("--color").arg("a").succeeds(); - scene.ucmd().arg("--color=always").arg("a/a").succeeds(); - scene.ucmd().arg("--color=never").arg("z").succeeds(); + at.touch(&at.plus_as_string("a/nested_file")); + at.touch("test-color"); + + let a_with_colors = "\x1b[01;34ma\x1b[0m"; + let z_with_colors = "\x1b[01;34mz\x1b[0m"; + let nested_dir_with_colors = "\x1b[01;34mnested_dir\x1b[0m"; + + // Color is disabled by default + let result = scene.ucmd().succeeds(); + assert!(!result.stdout.contains(a_with_colors)); + assert!(!result.stdout.contains(z_with_colors)); + + // Color should be enabled + let result = scene.ucmd().arg("--color").succeeds(); + assert!(result.stdout.contains(a_with_colors)); + assert!(result.stdout.contains(z_with_colors)); + + // Color should be enabled + let result = scene.ucmd().arg("--color=always").succeeds(); + assert!(result.stdout.contains(a_with_colors)); + assert!(result.stdout.contains(z_with_colors)); + + // Color should be disabled + let result = scene.ucmd().arg("--color=never").succeeds(); + assert!(!result.stdout.contains(a_with_colors)); + assert!(!result.stdout.contains(z_with_colors)); + + // Nested dir should be shown and colored + let result = scene.ucmd().arg("--color").arg("a").succeeds(); + assert!(result.stdout.contains(nested_dir_with_colors)); + + // Color has no effect + let result = scene + .ucmd() + .arg("--color=always") + .arg("a/nested_file") + .succeeds(); + assert!(result.stdout.contains("a/nested_file\n")); + + // No output + let result = scene.ucmd().arg("--color=never").arg("z").succeeds(); + assert_eq!(result.stdout, ""); } #[cfg(not(any(target_os = "macos", target_os = "windows")))] // Truncate not available on mac or win