Skip to content

Commit

Permalink
Merge pull request #5726 from jetlime/clippy_suppress_cognitive_compl…
Browse files Browse the repository at this point in the history 
…exity_warning

Address cognitive complexity
  • Loading branch information
@cakebaker
cakebaker committed Dec 26, 2023
2 parents 6d2486c + eb3dd52 commit 4acc96f
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 103 deletions.
83 changes: 55 additions & 28 deletions src/uu/cat/src/cat.rs
View file
Expand Up @@ -463,7 +463,6 @@ fn write_fast<R: FdReadable>(handle: &mut InputHandle<R>) -> CatResult<()> {

/// Outputs file contents to stdout in a line-by-line fashion,
/// propagating any errors that might occur.
#[allow(clippy::cognitive_complexity)]
fn write_lines<R: FdReadable>(
handle: &mut InputHandle<R>,
options: &OutputOptions,
Expand All @@ -482,22 +481,7 @@ fn write_lines<R: FdReadable>(
while pos < n {
// skip empty line_number enumerating them if needed
if in_buf[pos] == b'\n' {
// \r followed by \n is printed as ^M when show_ends is enabled, so that \r\n prints as ^M$
if state.skipped_carriage_return && options.show_ends {
writer.write_all(b"^M")?;
state.skipped_carriage_return = false;
}
if !state.at_line_start || !options.squeeze_blank || !state.one_blank_kept {
state.one_blank_kept = true;
if state.at_line_start && options.number == NumberingMode::All {
write!(writer, "{0:6}\t", state.line_number)?;
state.line_number += 1;
}
writer.write_all(options.end_of_line().as_bytes())?;
if handle.is_interactive {
writer.flush()?;
}
}
write_new_line(&mut writer, options, state, handle.is_interactive)?;
state.at_line_start = true;
pos += 1;
continue;
Expand All @@ -514,13 +498,8 @@ fn write_lines<R: FdReadable>(
}

// print to end of line or end of buffer
let offset = if options.show_nonprint {
write_nonprint_to_end(&in_buf[pos..], &mut writer, options.tab().as_bytes())
} else if options.show_tabs {
write_tab_to_end(&in_buf[pos..], &mut writer)
} else {
write_to_end(&in_buf[pos..], &mut writer)
};
let offset = write_end(&mut writer, &in_buf[pos..], options);

// end of buffer?
if offset + pos == in_buf.len() {
state.at_line_start = false;
Expand All @@ -531,10 +510,11 @@ fn write_lines<R: FdReadable>(
} else {
assert_eq!(in_buf[pos + offset], b'\n');
// print suitable end of line
writer.write_all(options.end_of_line().as_bytes())?;
if handle.is_interactive {
writer.flush()?;
}
write_end_of_line(
&mut writer,
options.end_of_line().as_bytes(),
handle.is_interactive,
)?;
state.at_line_start = true;
}
pos += offset + 1;
Expand All @@ -544,6 +524,41 @@ fn write_lines<R: FdReadable>(
Ok(())
}

// \r followed by \n is printed as ^M when show_ends is enabled, so that \r\n prints as ^M$
fn write_new_line<W: Write>(
writer: &mut W,
options: &OutputOptions,
state: &mut OutputState,
is_interactive: bool,
) -> CatResult<()> {
if state.skipped_carriage_return && options.show_ends {
writer.write_all(b"^M")?;
state.skipped_carriage_return = false;
}
if !state.at_line_start || !options.squeeze_blank || !state.one_blank_kept {
state.one_blank_kept = true;
if state.at_line_start && options.number == NumberingMode::All {
write!(writer, "{0:6}\t", state.line_number)?;
state.line_number += 1;
}
writer.write_all(options.end_of_line().as_bytes())?;
if is_interactive {
writer.flush()?;
}
}
Ok(())
}

fn write_end<W: Write>(writer: &mut W, in_buf: &[u8], options: &OutputOptions) -> usize {
if options.show_nonprint {
write_nonprint_to_end(in_buf, writer, options.tab().as_bytes())
} else if options.show_tabs {
write_tab_to_end(in_buf, writer)
} else {
write_to_end(in_buf, writer)
}
}

// write***_to_end methods
// Write all symbols till \n or \r or end of buffer is reached
// We need to stop at \r because it may be written as ^M depending on the byte after and settings;
Expand Down Expand Up @@ -610,6 +625,18 @@ fn write_nonprint_to_end<W: Write>(in_buf: &[u8], writer: &mut W, tab: &[u8]) ->
count
}

fn write_end_of_line<W: Write>(
writer: &mut W,
end_of_line: &[u8],
is_interactive: bool,
) -> CatResult<()> {
writer.write_all(end_of_line)?;
if is_interactive {
writer.flush()?;
}
Ok(())
}

#[cfg(test)]
mod tests {
use std::io::{stdout, BufWriter};
Expand Down
165 changes: 90 additions & 75 deletions src/uu/touch/src/touch.rs
View file
Expand Up @@ -11,7 +11,7 @@ use chrono::{
TimeZone, Timelike,
};
use clap::builder::ValueParser;
use clap::{crate_version, Arg, ArgAction, ArgGroup, Command};
use clap::{crate_version, Arg, ArgAction, ArgGroup, ArgMatches, Command};
use filetime::{set_file_times, set_symlink_file_times, FileTime};
use std::ffi::OsString;
use std::fs::{self, File};
Expand Down Expand Up @@ -73,7 +73,6 @@ fn filetime_to_datetime(ft: &FileTime) -> Option<DateTime<Local>> {
}

#[uucore::main]
#[allow(clippy::cognitive_complexity)]
pub fn uumain(args: impl uucore::Args) -> UResult<()> {
let matches = uu_app().try_get_matches_from(args)?;

Expand All @@ -86,37 +85,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
),
)
})?;
let (mut atime, mut mtime) = match (
matches.get_one::<OsString>(options::sources::REFERENCE),
matches.get_one::<String>(options::sources::DATE),
) {
(Some(reference), Some(date)) => {
let (atime, mtime) = stat(Path::new(reference), !matches.get_flag(options::NO_DEREF))?;
let atime = filetime_to_datetime(&atime).ok_or_else(|| {
USimpleError::new(1, "Could not process the reference access time")
})?;
let mtime = filetime_to_datetime(&mtime).ok_or_else(|| {
USimpleError::new(1, "Could not process the reference modification time")
})?;
(parse_date(atime, date)?, parse_date(mtime, date)?)
}
(Some(reference), None) => {
stat(Path::new(reference), !matches.get_flag(options::NO_DEREF))?
}
(None, Some(date)) => {
let timestamp = parse_date(Local::now(), date)?;
(timestamp, timestamp)
}
(None, None) => {
let timestamp = if let Some(ts) = matches.get_one::<String>(options::sources::TIMESTAMP)
{
parse_timestamp(ts)?
} else {
datetime_to_filetime(&Local::now())
};
(timestamp, timestamp)
}
};

let (atime, mtime) = determine_times(&matches)?;

for filename in files {
// FIXME: find a way to avoid having to clone the path
Expand Down Expand Up @@ -165,48 +135,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
}
}

// If changing "only" atime or mtime, grab the existing value of the other.
// Note that "-a" and "-m" may be passed together; this is not an xor.
if matches.get_flag(options::ACCESS)
|| matches.get_flag(options::MODIFICATION)
|| matches.contains_id(options::TIME)
{
let st = stat(path, !matches.get_flag(options::NO_DEREF))?;
let time = matches
.get_one::<String>(options::TIME)
.map(|s| s.as_str())
.unwrap_or("");

if !(matches.get_flag(options::ACCESS)
|| time.contains(&"access".to_owned())
|| time.contains(&"atime".to_owned())
|| time.contains(&"use".to_owned()))
{
atime = st.0;
}

if !(matches.get_flag(options::MODIFICATION)
|| time.contains(&"modify".to_owned())
|| time.contains(&"mtime".to_owned()))
{
mtime = st.1;
}
}

// sets the file access and modification times for a file or a symbolic link.
// The filename, access time (atime), and modification time (mtime) are provided as inputs.

// If the filename is not "-", indicating a special case for touch -h -,
// the code checks if the NO_DEREF flag is set, which means the user wants to
// set the times for a symbolic link itself, rather than the file it points to.
if filename == "-" {
filetime::set_file_times(path, atime, mtime)
} else if matches.get_flag(options::NO_DEREF) {
set_symlink_file_times(path, atime, mtime)
} else {
set_file_times(path, atime, mtime)
}
.map_err_context(|| format!("setting times of {}", path.quote()))?;
update_times(&matches, path, atime, mtime, filename)?;
}
Ok(())
}
Expand Down Expand Up @@ -307,6 +236,92 @@ pub fn uu_app() -> Command {
)
}

// Determine the access and modification time
fn determine_times(matches: &ArgMatches) -> UResult<(FileTime, FileTime)> {
match (
matches.get_one::<OsString>(options::sources::REFERENCE),
matches.get_one::<String>(options::sources::DATE),
) {
(Some(reference), Some(date)) => {
let (atime, mtime) = stat(Path::new(&reference), !matches.get_flag(options::NO_DEREF))?;
let atime = filetime_to_datetime(&atime).ok_or_else(|| {
USimpleError::new(1, "Could not process the reference access time")
})?;
let mtime = filetime_to_datetime(&mtime).ok_or_else(|| {
USimpleError::new(1, "Could not process the reference modification time")
})?;
Ok((parse_date(atime, date)?, parse_date(mtime, date)?))
}
(Some(reference), None) => {
stat(Path::new(&reference), !matches.get_flag(options::NO_DEREF))
}
(None, Some(date)) => {
let timestamp = parse_date(Local::now(), date)?;
Ok((timestamp, timestamp))
}
(None, None) => {
let timestamp = if let Some(ts) = matches.get_one::<String>(options::sources::TIMESTAMP)
{
parse_timestamp(ts)?
} else {
datetime_to_filetime(&Local::now())
};
Ok((timestamp, timestamp))
}
}
}

// Updating file access and modification times based on user-specified options
fn update_times(
matches: &ArgMatches,
path: &Path,
mut atime: FileTime,
mut mtime: FileTime,
filename: &OsString,
) -> UResult<()> {
// If changing "only" atime or mtime, grab the existing value of the other.
// Note that "-a" and "-m" may be passed together; this is not an xor.
if matches.get_flag(options::ACCESS)
|| matches.get_flag(options::MODIFICATION)
|| matches.contains_id(options::TIME)
{
let st = stat(path, !matches.get_flag(options::NO_DEREF))?;
let time = matches
.get_one::<String>(options::TIME)
.map(|s| s.as_str())
.unwrap_or("");

if !(matches.get_flag(options::ACCESS)
|| time.contains(&"access".to_owned())
|| time.contains(&"atime".to_owned())
|| time.contains(&"use".to_owned()))
{
atime = st.0;
}

if !(matches.get_flag(options::MODIFICATION)
|| time.contains(&"modify".to_owned())
|| time.contains(&"mtime".to_owned()))
{
mtime = st.1;
}
}

// sets the file access and modification times for a file or a symbolic link.
// The filename, access time (atime), and modification time (mtime) are provided as inputs.

// If the filename is not "-", indicating a special case for touch -h -,
// the code checks if the NO_DEREF flag is set, which means the user wants to
// set the times for a symbolic link itself, rather than the file it points to.
if filename == "-" {
filetime::set_file_times(path, atime, mtime)
} else if matches.get_flag(options::NO_DEREF) {
set_symlink_file_times(path, atime, mtime)
} else {
set_file_times(path, atime, mtime)
}
.map_err_context(|| format!("setting times of {}", path.quote()))
}
// Get metadata of the provided path
// If `follow` is `true`, the function will try to follow symlinks
// If `follow` is `false` or the symlink is broken, the function will return metadata of the symlink itself
Expand Down

0 comments on commit 4acc96f

Please sign in to comment.