From ad3dfbf300a3762e81be1144ab58e2b9d18107aa Mon Sep 17 00:00:00 2001
From: Ryuichi Ueda <ryuichiueda@gmail.com>
Date: Mon, 20 Jan 2025 16:26:44 +0900
Subject: [PATCH] Simplify

---
 src/core/builtins/option.rs      |  7 +++----
 src/core/builtins/parameter.rs   | 13 ++++++-------
 src/elements/command.rs          |  3 +--
 src/elements/command/for.rs      |  4 ++--
 src/elements/command/simple.rs   |  4 ++--
 src/elements/expr/conditional.rs |  3 +--
 src/elements/io/redirect.rs      |  3 +--
 src/elements/job.rs              |  3 +--
 src/error/exec.rs                | 28 ++++++++++++++++++----------
 src/main.rs                      |  9 ++++-----
 test/ok                          |  2 +-
 11 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/src/core/builtins/option.rs b/src/core/builtins/option.rs
index dd146036..f32c3ae7 100644
--- a/src/core/builtins/option.rs
+++ b/src/core/builtins/option.rs
@@ -2,7 +2,6 @@
 //SPDX-License-Identifier: BSD-3-Clause
 
 use crate::{error, ShellCore};
-use crate::error::exec;
 use crate::error::exec::ExecError;
 use crate::utils::arg;
 use super::parameter;
@@ -63,7 +62,7 @@ pub fn set(core: &mut ShellCore, args: &mut Vec<String>) -> i32 {
         return match parameter::set_positions(core, &args) {
             Ok(()) => 0,
             Err(e) => {
-                exec::print_error(e, core);
+                e.print(core);
                 return 1;
             },
         }
@@ -89,11 +88,11 @@ pub fn set(core: &mut ShellCore, args: &mut Vec<String>) -> i32 {
 
     match args[1].starts_with("-") || args[1].starts_with("+") {
         true  => if let Err(e) = set_options(core, &args[1..]) {
-            exec::print_error(e, core);
+            e.print(core);
             return 2;
         },
         false => if let Err(e) = parameter::set_positions(core, &args) {
-            exec::print_error(e, core);
+            e.print(core);
             return 2;
         },
     }
diff --git a/src/core/builtins/parameter.rs b/src/core/builtins/parameter.rs
index 30ab7368..f0bc5e09 100644
--- a/src/core/builtins/parameter.rs
+++ b/src/core/builtins/parameter.rs
@@ -2,7 +2,6 @@
 //SPDX-License-Identifier: BSD-3-Clause
 
 use crate::{ShellCore, utils, Feeder};
-use crate::error::exec;
 use crate::error::exec::ExecError;
 use crate::elements::substitution::Substitution;
 use crate::utils::arg;
@@ -79,12 +78,12 @@ pub fn local(core: &mut ShellCore, args: &mut Vec<String>) -> i32 {
     let layer = if core.db.get_layer_num() > 2 {
         core.db.get_layer_num() - 2//The last element of data.parameters is for local itself. 
     }else{
-        exec::print_error(ExecError::ValidOnlyInFunction("local".to_string()), core);
+        ExecError::ValidOnlyInFunction("local".to_string()).print(core);
         return 1;
     };
 
     if let Err(e) = local_(core, args, layer) {
-         exec::print_error(e, core);
+         e.print(core);
          return 1;
     };
     0
@@ -106,11 +105,11 @@ pub fn declare(core: &mut ShellCore, args: &mut Vec<String>) -> i32 {
     if args.contains(&"-a".to_string()) {
         if ! utils::is_name(&name, core) {
             let e = ExecError::InvalidName(name.to_string());
-            exec::print_error(e, core);
+            e.print(core);
             return 1;
         }
         if let Err(e) = core.db.set_array(&name, vec![], None) {
-            exec::print_error(e, core);
+            e.print(core);
             return 1;
         }
 
@@ -120,11 +119,11 @@ pub fn declare(core: &mut ShellCore, args: &mut Vec<String>) -> i32 {
     if args.contains(&"-A".to_string()) {
         if ! utils::is_name(&name, core) {
             let e = ExecError::InvalidName(name.to_string());
-            exec::print_error(e, core);
+            e.print(core);
             return 1;
         }
         if let Err(e) = core.db.set_assoc(&name, None) {
-            exec::print_error(e, core);
+            e.print(core);
             return 1;
         }
 
diff --git a/src/elements/command.rs b/src/elements/command.rs
index 6b4bc7ad..dcfaa055 100644
--- a/src/elements/command.rs
+++ b/src/elements/command.rs
@@ -13,7 +13,6 @@ pub mod r#while;
 pub mod r#if;
 
 use crate::{proc_ctrl, ShellCore, Feeder, Script};
-use crate::error::exec;
 use crate::error::exec::ExecError;
 use crate::error::parse::ParseError;
 use crate::utils::exit;
@@ -61,7 +60,7 @@ pub trait Command {
                 core.initialize_as_subshell(Pid::from_raw(0), pipe.pgid);
                 io::connect(pipe, self.get_redirects(), core);
                 if let Err(e) = self.run(core, true) {
-                    exec::print_error(e, core);
+                    e.print(core);
                 }
                 exit::normal(core)
             },
diff --git a/src/elements/command/for.rs b/src/elements/command/for.rs
index 6d25977d..d114cefd 100644
--- a/src/elements/command/for.rs
+++ b/src/elements/command/for.rs
@@ -2,7 +2,7 @@
 //SPDX-License-Identifier: BSD-3-Clause
 
 use crate::{ShellCore, Feeder, Script};
-use crate::error::exec;
+
 use crate::error::exec::ExecError;
 use crate::error::parse::ParseError;
 use super::{Command, Redirect};
@@ -59,7 +59,7 @@ impl ForCommand {
             match w.eval(core) {
                 Ok(mut ws) => ans.append(&mut ws),
                 Err(e)     => {
-                    exec::print_error(e, core);
+                    e.print(core);
                     return None;
                 },
             }
diff --git a/src/elements/command/simple.rs b/src/elements/command/simple.rs
index 53c024e7..ec4f8218 100644
--- a/src/elements/command/simple.rs
+++ b/src/elements/command/simple.rs
@@ -4,7 +4,7 @@
 pub mod parser;
 
 use crate::{proc_ctrl, ShellCore};
-use crate::error::exec;
+
 use crate::error::exec::ExecError;
 use crate::utils::exit;
 use super::{Command, Pipe, Redirect};
@@ -141,7 +141,7 @@ impl SimpleCommand {
                 Ok(())
             },
             Err(e) => {
-                exec::print_error(e.clone(), core);
+                e.print(core);
                 if ! core.sigint.load(Relaxed) {
                     core.db.exit_status = 1;
                 }
diff --git a/src/elements/expr/conditional.rs b/src/elements/expr/conditional.rs
index 17ac77a3..7304d41e 100644
--- a/src/elements/expr/conditional.rs
+++ b/src/elements/expr/conditional.rs
@@ -5,7 +5,6 @@ pub mod elem;
 mod parser;
 
 use crate::ShellCore;
-use crate::error::exec;
 use crate::error::exec::ExecError;
 use crate::utils::{file_check, glob};
 use crate::elements::word::Word;
@@ -139,7 +138,7 @@ impl ConditionalExpr {
             let mut err = "syntax error".to_string();
             if stack.len() > 1 {
                 err = format!("syntax error in conditional expression: unexpected token `{}'", &stack[0].to_string());
-                exec::print_error(ExecError::Other(err), core);
+                ExecError::Other(err).print(core);
                 err = format!("syntax error near `{}'", &stack[0].to_string());
             }
             return Err(ExecError::Other(err));
diff --git a/src/elements/io/redirect.rs b/src/elements/io/redirect.rs
index 5405b763..ccd7b20d 100644
--- a/src/elements/io/redirect.rs
+++ b/src/elements/io/redirect.rs
@@ -4,7 +4,6 @@
 use std::fs::{File, OpenOptions};
 use std::os::fd::{IntoRawFd, RawFd};
 use std::io::Error;
-use crate::error::exec;
 use crate::elements::io;
 use crate::elements::word::Word;
 use crate::{Feeder, ShellCore};
@@ -35,7 +34,7 @@ impl Redirect {
         let args = match self.right.eval(core) {
             Ok(v) => v,
             Err(e) => {
-                exec::print_error(e, core);
+                e.print(core);
                 return false;
             },
         };
diff --git a/src/elements/job.rs b/src/elements/job.rs
index 13cb31bb..266cc1d9 100644
--- a/src/elements/job.rs
+++ b/src/elements/job.rs
@@ -5,7 +5,6 @@ use super::pipeline::Pipeline;
 use crate::{proc_ctrl, Feeder, ShellCore};
 use crate::core::jobtable::JobEntry;
 use crate::utils::exit;
-use crate::error::exec;
 use crate::error::exec::ExecError;
 use crate::error::parse::ParseError;
 use nix::sys::wait::WaitStatus;
@@ -103,7 +102,7 @@ impl Job {
             Ok(ForkResult::Child) => {
                 core.initialize_as_subshell(Pid::from_raw(0), pgid);
                 if let Err(e) = self.exec(core, false) {
-                    exec::print_error(e, core);
+                    e.print(core);
                 }
                 exit::normal(core)
             },
diff --git a/src/error/exec.rs b/src/error/exec.rs
index 3e28cc54..3652fe18 100644
--- a/src/error/exec.rs
+++ b/src/error/exec.rs
@@ -28,6 +28,12 @@ pub enum ExecError {
 
 impl From<ExecError> for String {
     fn from(e: ExecError) -> String {
+        Self::from(&e)
+    }
+}
+
+impl From<&ExecError> for String {
+    fn from(e: &ExecError) -> String {
         match e {
             ExecError::Internal => "INTERNAL ERROR".to_string(),
             ExecError::ArrayIndexInvalid(name) => format!("`{}': not a valid index", name),
@@ -43,21 +49,23 @@ impl From<ExecError> for String {
             ExecError::VariableReadOnly(name) => format!("{}: readonly variable", name),
             ExecError::VariableInvalid(name) => format!("`{}': not a valid identifier", name),
             ExecError::OperandExpected(token) => format!("{0}: syntax error: operand expected (error token is \"{0}\")", token),
-            ExecError::ParseError(p) => From::from(&p),
+            ExecError::ParseError(p) => From::from(p),
             ExecError::Recursion(token) => format!("{0}: expression recursion level exceeded (error token is \"{0}\")", token), 
             ExecError::SubstringMinus(n) => format!("{}: substring expression < 0", n),
-            ExecError::Other(name) => name,
+            ExecError::Other(name) => name.to_string(),
         }
     }
 }
 
-pub fn print_error(e: ExecError, core: &mut ShellCore) {
-    let name = core.db.get_param("0").unwrap();
-    let s: String = From::<ExecError>::from(e);
-    if core.db.flags.contains('i') {
-        eprintln!("{}: {}", &name, &s);
-    }else{
-        let lineno = core.db.get_param("LINENO").unwrap_or("".to_string());
-        eprintln!("{}: line {}: {}", &name, &lineno, s);
+impl ExecError {
+    pub fn print(&self, core: &mut ShellCore) {
+        let name = core.db.get_param("0").unwrap();
+        let s: String = From::<&ExecError>::from(self);
+        if core.db.flags.contains('i') {
+            eprintln!("{}: {}", &name, &s);
+        }else{
+            let lineno = core.db.get_param("LINENO").unwrap_or("".to_string());
+            eprintln!("{}: line {}: {}", &name, &lineno, s);
+        }
     }
 }
diff --git a/src/main.rs b/src/main.rs
index bfbefc00..4635d6cc 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -13,7 +13,6 @@ use builtins::{option, parameter};
 use std::{env, process};
 use std::sync::atomic::Ordering::Relaxed;
 use crate::core::{builtins, ShellCore};
-use crate::error::exec;
 use crate::elements::script::Script;
 use crate::feeder::Feeder;
 use utils::{exit, file_check, arg};
@@ -65,11 +64,11 @@ fn configure(args: &Vec<String>) -> ShellCore {
     }
 
     if let Err(e) = option::set_options(&mut core, &options) {
-        exec::print_error(e, &mut core);
+        e.print(&mut core);
         panic!("");
     }
     if let Err(e) = parameter::set_positions(&mut core, &parameters) {
-        exec::print_error(e, &mut core);
+        e.print(&mut core);
         panic!("");
     }
     core
@@ -175,11 +174,11 @@ fn run_and_exit_c_option(args: &Vec<String>, c_parts: &Vec<String>) {
     };
 
     if let Err(e) = option::set_options(&mut core, &mut args[1..].to_vec()) {
-        exec::print_error(e, &mut core);
+        e.print(&mut core);
         panic!("");
     }
     if let Err(e) = parameter::set_positions(&mut core, &parameters) {
-        exec::print_error(e, &mut core);
+        e.print(&mut core);
         panic!("");
     }
     signal::run_signal_check(&mut core);
diff --git a/test/ok b/test/ok
index 3b519473..fb8e72f0 100644
--- a/test/ok
+++ b/test/ok
@@ -5,7 +5,7 @@
 ./test_brace.bash
 ./test_builtins.bash
 ./test_others.bash
-./test_parameters.bash
 ./test_calculation.bash
+./test_parameters.bash
 ./test_compound.bash
 ./test_job.bash