Merge pull request #484 from Bluemangoo/feature/skill-snice-w-i-n

skill&snice: implement `no-action` `warnings` `interactive`

Author: Krysztal112233

src/uu/skill/src/skill.rs

@@ -3,7 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-use clap::{arg, crate_version, value_parser, Arg, Command};
+use clap::{crate_version, Arg, Command};
 #[cfg(unix)]
 use nix::{sys::signal, sys::signal::Signal, unistd::Pid};
 use uu_snice::{
@@ -29,6 +29,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     }
 
     // Case1: Send signal
+    let take_action = !settings.no_action;
     if let Some(targets) = settings.expressions {
         let pids = collect_pids(&targets);
 
@@ -43,33 +44,46 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         };
 
         #[cfg(unix)]
-        let results = perform_action(&pids, &signal);
+        let results = perform_action(&pids, &signal, take_action, settings.interactive);
         #[cfg(not(unix))]
         let results: Vec<Option<ActionResult>> = Vec::new();
 
         if results.iter().all(|it| it.is_none()) || results.is_empty() {
             return Err(USimpleError::new(1, "no process selection criteria"));
         }
 
-        if settings.verbose {
-            let output = construct_verbose_result(&pids, &results).trim().to_owned();
+        let error_only = settings.warnings || !settings.verbose;
+        if settings.verbose || settings.warnings {
+            let output = construct_verbose_result(&pids, &results, error_only, take_action)
+                .trim()
+                .to_owned();
             println!("{output}");
+        } else if !take_action {
+            pids.iter().for_each(|pid| println!("{pid}"));
         }
     }
 
     Ok(())
 }
 
 #[cfg(unix)]
-fn perform_action(pids: &[u32], signal: &Signal) -> Vec<Option<ActionResult>> {
+fn perform_action(
+    pids: &[u32],
+    signal: &Signal,
+    take_action: bool,
+    ask: bool,
+) -> Vec<Option<ActionResult>> {
+    let sig = if take_action { Some(*signal) } else { None };
     pids.iter()
         .map(|pid| {
-            {
-                Some(match signal::kill(Pid::from_raw(*pid as i32), *signal) {
+            if !ask || uu_snice::ask_user(*pid) {
+                Some(match signal::kill(Pid::from_raw(*pid as i32), sig) {
                     Ok(_) => ActionResult::Success,
-
                     Err(_) => ActionResult::PermissionDenied,
                 })
+            } else {
+                // won't be used, but we need to return (not None)
+                Some(ActionResult::Success)
             }
         })
         .collect()
@@ -84,23 +98,5 @@ pub fn uu_app() -> Command {
         .infer_long_args(true)
         .arg_required_else_help(true)
         .arg(Arg::new("signal"))
-        .args([
-            // arg!(-f --fast          "fast mode (not implemented)"),
-            // arg!(-i --interactive   "interactive"),
-            arg!(-l --list                  "list all signal names"),
-            arg!(-L --table                 "list all signal names in a nice table"),
-            // arg!(-n --"no-action"   "do not actually kill processes; just print what would happen"),
-            arg!(-v --verbose               "explain what is being done"),
-            // arg!(-w --warnings      "enable warnings (not implemented)"),
-            // Expressions
-            arg!(-c --command   <command>   ...   "expression is a command name"),
-            arg!(-p --pid       <pid>       ...   "expression is a process id number")
-                .value_parser(value_parser!(u32)),
-            arg!(-t --tty       <tty>       ...   "expression is a terminal"),
-            arg!(-u --user      <username>  ...   "expression is a username"),
-            // arg!(--ns <PID>                "match the processes that belong to the same namespace as <pid>"),
-            // arg!(--nslist <ns>             "list which namespaces will be considered for the --ns option.")
-            //     .value_delimiter(',')
-            //     .value_parser(["ipc", "mnt", "net", "pid", "user", "uts"]),
-        ])
+        .args(uu_snice::clap_args())
 }

src/uu/snice/src/action.rs

@@ -3,6 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
+use crate::ask_user;
 use crate::priority::Priority;
 use std::{
     fmt::{self, Display, Formatter},
@@ -92,7 +93,7 @@ impl SelectedTarget {
 }
 
 #[allow(unused)]
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub enum ActionResult {
     PermissionDenied,
     Success,
@@ -171,7 +172,15 @@ pub(crate) fn perform_action(
     pids: &[u32],
     prio: &Priority,
     take_action: bool,
+    ask: bool,
 ) -> Vec<Option<ActionResult>> {
-    let f = |pid: &u32| set_priority(*pid, prio, take_action);
+    let f = |pid: &u32| {
+        if !ask || ask_user(*pid) {
+            set_priority(*pid, prio, take_action)
+        } else {
+            // won't be used, but we need to return (not None)
+            Some(ActionResult::Success)
+        }
+    };
     pids.iter().map(f).collect()
 }

src/uu/snice/src/process_matcher.rs

@@ -14,6 +14,9 @@ pub struct Settings {
     pub display: Option<SignalDisplay>,
     pub expressions: Option<Vec<SelectedTarget>>,
     pub verbose: bool,
+    pub warnings: bool,
+    pub interactive: bool,
+    pub no_action: bool,
 }
 
 impl Settings {
@@ -30,6 +33,9 @@ impl Settings {
             display,
             expressions: Self::targets(matches),
             verbose: matches.get_flag("verbose"),
+            warnings: matches.get_flag("warnings"),
+            interactive: matches.get_flag("interactive"),
+            no_action: matches.get_flag("no-action"),
         })
     }
 
@@ -81,12 +87,12 @@ impl Settings {
 pub fn clap_args() -> Vec<Arg> {
     vec![
         // arg!(-f --fast          "fast mode (not implemented)"),
-        // arg!(-i --interactive   "interactive"),
+        arg!(-i --interactive   "interactive").conflicts_with_all(["verbose", "no-action"]),
         arg!(-l --list                  "list all signal names"),
         arg!(-L --table                 "list all signal names in a nice table"),
         arg!(-n --"no-action"   "do not actually kill processes; just print what would happen"),
         arg!(-v --verbose               "explain what is being done"),
-        // arg!(-w --warnings      "enable warnings (not implemented)"),
+        arg!(-w --warnings      "enable warnings (not implemented)"),
         // Expressions
         arg!(-c --command   <command>   ...   "expression is a command name"),
         arg!(-p --pid       <pid>       ...   "expression is a process id number")

src/uu/snice/src/snice.rs

@@ -3,14 +3,15 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-use std::{collections::HashSet, path::PathBuf, str::FromStr};
-
 use crate::priority::Priority;
 pub use action::ActionResult;
 use action::{perform_action, process_snapshot, users, SelectedTarget};
 use clap::{crate_version, Arg, Command};
 use prettytable::{format::consts::FORMAT_CLEAN, row, Table};
+pub use process_matcher::clap_args;
 use process_matcher::*;
+use std::io::Write;
+use std::{collections::HashSet, path::PathBuf, str::FromStr};
 use sysinfo::Pid;
 use uu_pgrep::process::ProcessInformation;
 #[cfg(target_family = "unix")]
@@ -92,7 +93,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     }
 
     // Case1: Perform priority
-    let take_action = !matches.get_flag("no-action");
+    let take_action = !settings.no_action;
     if let Some(targets) = settings.expressions {
         let priority_str = matches.get_one::<String>("priority").cloned();
 
@@ -104,14 +105,17 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         };
 
         let pids = collect_pids(&targets);
-        let results = perform_action(&pids, &priority, take_action);
+        let results = perform_action(&pids, &priority, take_action, settings.interactive);
 
         if results.iter().all(|it| it.is_none()) || results.is_empty() {
             return Err(USimpleError::new(1, "no process selection criteria"));
         }
 
-        if settings.verbose {
-            let output = construct_verbose_result(&pids, &results).trim().to_owned();
+        let error_only = settings.warnings || !settings.verbose;
+        if settings.verbose || settings.warnings {
+            let output = construct_verbose_result(&pids, &results, error_only, take_action)
+                .trim()
+                .to_owned();
             println!("{output}");
         } else if !take_action {
             pids.iter().for_each(|pid| println!("{pid}"));
@@ -121,15 +125,66 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     Ok(())
 }
 
+pub fn ask_user(pid: u32) -> bool {
+    let process = process_snapshot().process(Pid::from_u32(pid)).unwrap();
+
+    let tty = ProcessInformation::try_new(PathBuf::from_str(&format!("/proc/{pid}")).unwrap())
+        .map(|v| v.tty().to_string())
+        .unwrap_or(String::from("?"));
+
+    let user = process
+        .user_id()
+        .and_then(|uid| users().iter().find(|it| it.id() == uid))
+        .map(|it| it.name())
+        .unwrap_or("?")
+        .to_owned();
+
+    let cmd = process
+        .exe()
+        .and_then(|it| it.iter().next_back())
+        .unwrap_or("?".as_ref());
+    let cmd = cmd.to_str().unwrap();
+
+    // no newline at the end
+    print!("{tty:<8} {user:<8} {pid:<5} {cmd:<18} ? ");
+    std::io::stdout().flush().unwrap();
+    let mut input = String::new();
+    if std::io::stdin().read_line(&mut input).is_err() {
+        return false;
+    }
+    let input = input.trim();
+    if input.eq_ignore_ascii_case("y") || input.eq_ignore_ascii_case("yes") {
+        return true;
+    }
+
+    false
+}
+
 #[allow(unused)]
-pub fn construct_verbose_result(pids: &[u32], action_results: &[Option<ActionResult>]) -> String {
+pub fn construct_verbose_result(
+    pids: &[u32],
+    action_results: &[Option<ActionResult>],
+    error_only: bool,
+    take_action: bool,
+) -> String {
     let mut table = action_results
         .iter()
         .enumerate()
         .map(|(index, it)| (pids[index], it))
         .filter(|(_, it)| it.is_some())
+        .filter(|v| {
+            !error_only
+                || !take_action
+                || v.1
+                    .clone()
+                    .is_some_and(|v| v == ActionResult::PermissionDenied)
+        })
         .map(|(pid, action)| (pid, action.clone().unwrap()))
         .map(|(pid, action)| {
+            if !take_action && action == ActionResult::Success {
+                return (pid, None);
+            }
+
             let process = process_snapshot().process(Pid::from_u32(pid)).unwrap();
 
             let tty =
@@ -148,10 +203,20 @@ pub fn construct_verbose_result(pids: &[u32], action_results: &[Option<ActionRes
                 .unwrap_or("?".as_ref());
             let cmd = cmd.to_str().unwrap();
 
-            (tty, user, pid, cmd, action)
+            (pid, Some((tty, user, cmd, action)))
+        })
+        .filter(|(_, v)| match v {
+            None => true,
+            Some((tty, _, _, _)) => tty.is_ok(),
+        })
+        .map(|(pid, v)| match v {
+            None => {
+                row![pid]
+            }
+            Some((tty, user, cmd, action)) => {
+                row![tty.unwrap().tty(), user, pid, cmd, action]
+            }
         })
-        .filter(|(tty, _, _, _, _)| tty.is_ok())
-        .map(|(tty, user, pid, cmd, action)| row![tty.unwrap().tty(), user, pid, cmd, action])
         .collect::<Table>();
 
     table.set_format(*FORMAT_CLEAN);

tests/by-util/test_snice.rs

@@ -16,3 +16,9 @@ fn test_no_args() {
 fn test_no_process_selected() {
     new_ucmd!().arg("-u=invalid_user").fails().code_is(1);
 }
+
+#[test]
+fn test_interactive_conflict_args() {
+    new_ucmd!().args(&["-i", "-v"]).fails().code_is(1);
+    new_ucmd!().args(&["-i", "-n"]).fails().code_is(1);
+}