Commit 686ca23a authored by stratact's avatar stratact Committed by Michael Aaron Murphy
Browse files

Clean-up and refactor by removing tons of unneeded conversions and...

Clean-up and refactor by removing tons of unneeded conversions and collapsing/condensing conditionals and other performance enhancing code
parent cbf2347e
use std::{mem::transmute, ops::DerefMut};
use std::ops::DerefMut;
// TODO: These could be generalised to work on non-ASCII characters (and even
// strings!) as long as the byte size of the needle and haystack match.
......@@ -34,7 +34,7 @@ impl AsciiReplaceInPlace for str {
let (needle, haystack) = (needle as u8, haystack as u8);
// This is safe because we verify that we don't modify non-ascii bytes
let mut_bytes: &mut [u8] = unsafe { transmute(self) };
let mut_bytes: &mut [u8] = unsafe { &mut *(self as *mut str as *mut [u8]) };
// NOTE: When str_mut_extras is stable, use this line instead
// let mut_bytes = unsafe { self.as_bytes_mut() };
for chr in mut_bytes.iter_mut() {
......
use calc::{eval, eval_polish, CalcError, Value};
use std::io::{self, Write};
fn calc_or_polish_calc(args: String) -> Result<Value, CalcError> {
fn calc_or_polish_calc(args: &str) -> Result<Value, CalcError> {
match eval(&args) {
Ok(t) => Ok(t),
Err(_) => eval_polish(&args),
......@@ -12,7 +12,7 @@ pub(crate) fn calc(args: &[String]) -> Result<(), String> {
let stdout = io::stdout();
let mut stdout = stdout.lock();
if !args.is_empty() {
let result = calc_or_polish_calc(args.join(" "));
let result = calc_or_polish_calc(&args.join(" "));
let _ = match result {
Ok(v) => writeln!(stdout, "{}", v),
Err(e) => writeln!(stdout, "{}", e),
......@@ -31,7 +31,7 @@ pub(crate) fn calc(args: &[String]) -> Result<(), String> {
"" => (),
"exit" => break,
s => {
let result = calc_or_polish_calc(s.to_string());
let result = calc_or_polish_calc(s);
let _ = match result {
Ok(v) => writeln!(stdout, "{}", v),
Err(e) => writeln!(stdout, "{}", e),
......
......@@ -70,7 +70,7 @@ pub(crate) fn get_command_info<'a>(command: &str, shell: &mut Shell) -> Result<C
return Ok("builtin".into());
} else {
for path in env::var("PATH")
.unwrap_or("/bin".to_string())
.unwrap_or_else(|_| String::from("/bin"))
.split(sys::PATH_SEPARATOR)
{
let executable = Path::new(path).join(command);
......
......@@ -27,7 +27,7 @@ fn evaluate_arguments(arguments: &[String], shell: &Shell) -> Result<bool, Strin
Ok(match_option_argument(option, arg, shell))
})
}
Some(ref s) if s.starts_with("-") => {
Some(ref s) if s.starts_with('-') => {
// Access the second character in the flag string: this will be type of the
// flag. If no flag was given, return `SUCCESS`, as this means a
// string with value "-" was checked.
......@@ -88,7 +88,7 @@ fn binary_is_in_path(binaryname: &str, shell: &Shell) -> bool {
// *might* be possible TODO: that `exists` reports a binary to be in the
// path, while the shell cannot find it or TODO: vice-versa
if let Some(path) = shell.get_var("PATH") {
for dir in path.split(":") {
for dir in path.split(':') {
let fname = format!("{}/{}", dir, binaryname);
if let Ok(metadata) = fs::metadata(&fname) {
if metadata.is_file() && file_has_execute_permission(&fname) {
......@@ -108,7 +108,7 @@ fn binary_is_in_path(binaryname: &str, shell: &Shell) -> bool {
/// Note: This function is 1:1 the same as src/builtins/test.rs:file_has_execute_permission
/// If you change the following function, please also update the one in src/builtins/test.rs
fn file_has_execute_permission(filepath: &str) -> bool {
const USER: u32 = 0b1000000;
const USER: u32 = 0b100_0000;
const GROUP: u32 = 0b1000;
const GUEST: u32 = 0b1;
......
......@@ -11,8 +11,8 @@ pub(crate) fn ion_docs(_: &[String], shell: &mut Shell) -> i32 {
return FAILURE;
}
if let Some(cmd) = shell.get_var("BROWSER".into()) {
if let Ok(_) = Command::new(&cmd).arg(DOCPATH).spawn() {
if let Some(cmd) = shell.get_var("BROWSER") {
if Command::new(&cmd).arg(DOCPATH).spawn().is_ok() {
return SUCCESS;
}
} else {
......
......@@ -24,7 +24,7 @@ pub(crate) fn check_help(args: &[String], man_page: &'static str) -> bool {
false
}
pub(crate) const MAN_STATUS: &'static str = r#"NAME
pub(crate) const MAN_STATUS: &str = r#"NAME
status - Evaluates the current runtime status
SYNOPSIS
......@@ -42,7 +42,7 @@ OPTIONS
prints the filename of the currently running script or else stdio. Also --current-filename.
"#;
pub(crate) const MAN_CD: &'static str = r#"NAME
pub(crate) const MAN_CD: &str = r#"NAME
cd - Change directory.
SYNOPSIS
......@@ -55,7 +55,7 @@ DESCRIPTION
"#;
pub(crate) const MAN_BOOL: &'static str = r#"NAME
pub(crate) const MAN_BOOL: &str = r#"NAME
bool - Returns true if the value given to it is equal to '1' or 'true'.
SYNOPSIS
......@@ -65,7 +65,7 @@ DESCRIPTION
Returns true if the value given to it is equal to '1' or 'true'.
"#;
pub(crate) const MAN_IS: &'static str = r#"NAME
pub(crate) const MAN_IS: &str = r#"NAME
is - Checks if two arguments are the same
SYNOPSIS
......@@ -79,7 +79,7 @@ OPTIONS
returns 0 if the two arguments are not equal.
"#;
pub(crate) const MAN_ISATTY: &'static str = r#"
pub(crate) const MAN_ISATTY: &str = r#"
isatty - Checks if argument is a file descriptor
SYNOPSIS
......@@ -89,7 +89,7 @@ DESCRIPTION
Returns 0 exit status if the supplied file descriptor is a tty.
"#;
pub(crate) const MAN_DIRS: &'static str = r#"NAME
pub(crate) const MAN_DIRS: &str = r#"NAME
dirs - prints the directory stack
SYNOPSIS
......@@ -99,7 +99,7 @@ DESCRIPTION
dirs prints the current directory stack.
"#;
pub(crate) const MAN_PUSHD: &'static str = r#"NAME
pub(crate) const MAN_PUSHD: &str = r#"NAME
pushd - push a directory to the directory stack
SYNOPSIS
......@@ -109,7 +109,7 @@ DESCRIPTION
pushd pushes a directory to the directory stack.
"#;
pub(crate) const MAN_POPD: &'static str = r#"NAME
pub(crate) const MAN_POPD: &str = r#"NAME
popd - shift through the directory stack
SYNOPSIS
......@@ -120,7 +120,7 @@ DESCRIPTION
pushd adds directories to the stack.
"#;
// pub(crate) const MAN_FN: &'static str = r#"NAME
// pub(crate) const MAN_FN: &str = r#"NAME
// fn - print a list of all functions or create a function
//
// SYNOPSIS
......@@ -148,7 +148,7 @@ DESCRIPTION
// example 1
// "#;
pub(crate) const MAN_READ: &'static str = r#"NAME
pub(crate) const MAN_READ: &str = r#"NAME
read - read a line of input into some variables
SYNOPSIS
......@@ -158,7 +158,7 @@ DESCRIPTION
For each variable reads from standard input and stores the results in the variable.
"#;
pub(crate) const MAN_DROP: &'static str = r#"NAME
pub(crate) const MAN_DROP: &str = r#"NAME
drop - delete some variables or arrays
SYNOPSIS
......@@ -173,7 +173,7 @@ OPTIONS
Instead of deleting variables deletes arrays.
"#;
pub(crate) const MAN_SET: &'static str = r#"NAME
pub(crate) const MAN_SET: &str = r#"NAME
set - Set or unset values of shell options and positional parameters.
SYNOPSIS
......@@ -197,7 +197,7 @@ OPTIONS
If no arguments are suppled, arguments will not be unset.
"#;
pub(crate) const MAN_EQ: &'static str = r#"NAME
pub(crate) const MAN_EQ: &str = r#"NAME
eq - Checks if two arguments are the same
SYNOPSIS
......@@ -211,7 +211,7 @@ OPTIONS
returns 0 if the two arguments are not equal.
"#;
pub(crate) const MAN_EVAL: &'static str = r#"NAME
pub(crate) const MAN_EVAL: &str = r#"NAME
eval - evaluates the specified commands
SYNOPSIS
......@@ -222,7 +222,7 @@ DESCRIPTION
all arguments are joined using a space as a separator.
"#;
pub(crate) const MAN_EXEC: &'static str = r#"NAME
pub(crate) const MAN_EXEC: &str = r#"NAME
exec - Replace the shell with the given command.
SYNOPSIS
......@@ -237,7 +237,7 @@ OPTIONS
-c Execute command with an empty environment.
"#;
pub(crate) const MAN_HISTORY: &'static str = r#"NAME
pub(crate) const MAN_HISTORY: &str = r#"NAME
history - print command history
SYNOPSIS
......@@ -247,7 +247,7 @@ DESCRIPTION
Prints the command history.
"#;
pub(crate) const MAN_SOURCE: &'static str = r#"NAME
pub(crate) const MAN_SOURCE: &str = r#"NAME
source - evaluates given file
SYNOPSIS
......@@ -258,7 +258,7 @@ DESCRIPTION
variables will affect the current shell because of this.
"#;
pub(crate) const MAN_ECHO: &'static str = r#"NAME
pub(crate) const MAN_ECHO: &str = r#"NAME
echo - display a line of text
SYNOPSIS
......@@ -289,7 +289,7 @@ OPTIONS
\v vertical tab (VT)
"#;
pub(crate) const MAN_TEST: &'static str = r#"NAME
pub(crate) const MAN_TEST: &str = r#"NAME
test - perform tests on files and text
SYNOPSIS
......@@ -394,7 +394,7 @@ AUTHOR
Written by Michael Murphy.
"#;
pub(crate) const MAN_RANDOM: &'static str = r#"NAME
pub(crate) const MAN_RANDOM: &str = r#"NAME
random - generate a random number
SYNOPSIS
......@@ -407,7 +407,7 @@ DESCRIPTION
If two arguments are given the range is [START, END].
"#;
pub(crate) const MAN_TRUE: &'static str = r#"NAME
pub(crate) const MAN_TRUE: &str = r#"NAME
true - does nothing successfully
SYNOPSIS
......@@ -417,7 +417,7 @@ DESCRIPTION
Sets the exit status to 0.
"#;
pub(crate) const MAN_FALSE: &'static str = r#"NAME
pub(crate) const MAN_FALSE: &str = r#"NAME
false - does nothing unsuccessfully
SYNOPSIS
......@@ -427,7 +427,7 @@ DESCRIPTION
Sets the exit status to 1.
"#;
pub(crate) const MAN_JOBS: &'static str = r#"NAME
pub(crate) const MAN_JOBS: &str = r#"NAME
jobs - list all jobs running in the background
SYNOPSIS
......@@ -437,7 +437,7 @@ DESCRIPTION
Prints a list of all jobs running in the background.
"#;
pub(crate) const MAN_BG: &'static str = r#"NAME
pub(crate) const MAN_BG: &str = r#"NAME
bg - sends jobs to background
SYNOPSIS
......@@ -447,7 +447,7 @@ DESCRIPTION
bg sends the job to the background resuming it if it has stopped.
"#;
pub(crate) const MAN_FG: &'static str = r#"NAME
pub(crate) const MAN_FG: &str = r#"NAME
fg - bring job to foreground
SYNOPSIS
......@@ -457,7 +457,7 @@ DESCRIPTION
fg brings the specified job to foreground resuming it if it has stopped.
"#;
pub(crate) const MAN_SUSPEND: &'static str = r#"NAME
pub(crate) const MAN_SUSPEND: &str = r#"NAME
suspend - suspend the current shell
SYNOPSIS
......@@ -468,7 +468,7 @@ DESCRIPTION
returning to the parent process. It can be resumed by sending it SIGCONT.
"#;
pub(crate) const MAN_DISOWN: &'static str = r#"NAME
pub(crate) const MAN_DISOWN: &str = r#"NAME
disown - Disown processes
SYNOPSIS
......@@ -483,7 +483,7 @@ OPTIONS
-a If no job IDs were supplied, remove all jobs from the background process list.
"#;
pub(crate) const MAN_EXIT: &'static str = r#"NAME
pub(crate) const MAN_EXIT: &str = r#"NAME
exit - exit the shell
SYNOPSIS
......@@ -493,7 +493,7 @@ DESCRIPTION
Makes ion exit. The exit status will be that of the last command executed.
"#;
pub(crate) const MAN_MATCHES: &'static str = r#"NAME
pub(crate) const MAN_MATCHES: &str = r#"NAME
matches - checks if the second argument contains any portion of the first.
SYNOPSIS
......@@ -510,7 +510,7 @@ EXAMPLES
matches x xs
"#;
pub(crate) const MAN_EXISTS: &'static str = r#"NAME
pub(crate) const MAN_EXISTS: &str = r#"NAME
exists - check whether items exist
SYNOPSIS
......@@ -567,7 +567,7 @@ AUTHOR
Heavily based on implementation of the test builtin, which was written by Michael Murph.
"#;
pub(crate) const MAN_WHICH: &'static str = r#"NAME
pub(crate) const MAN_WHICH: &str = r#"NAME
which - locate a program file in the current user's path
SYNOPSIS
......
......@@ -61,7 +61,7 @@ macro_rules! map {
/// }
/// Builtins are in A-Z order.
pub const BUILTINS: &'static BuiltinMap = &map!(
pub const BUILTINS: &BuiltinMap = &map!(
"alias" => builtin_alias : "View, set or unset aliases",
"bg" => builtin_bg : "Resumes a stopped background process",
"bool" => builtin_bool : "If the value is '1' or 'true', return 0 exit status",
......
......@@ -16,10 +16,7 @@ pub(crate) fn status(args: &[String], shell: &mut Shell) -> Result<(), String> {
let mut flags = Flags::empty();
let shell_args: Vec<_> = env::args().collect();
let mut is_login = false;
if shell_args[0].chars().nth(0).unwrap() == '-' {
is_login = true;
}
let is_login = shell_args[0].chars().nth(0).unwrap() == '-';
let args_len = args.len();
if args_len == 1 {
......@@ -54,10 +51,8 @@ pub(crate) fn status(args: &[String], shell: &mut Shell) -> Result<(), String> {
return Err(err);
}
if flags.contains(Flags::INTERACTIVE) {
if shell.is_background_shell || shell.is_library {
return Err(err);
}
if flags.contains(Flags::INTERACTIVE) && shell.is_background_shell || shell.is_library {
return Err(err);
}
if flags.contains(Flags::FILENAME) {
......
......@@ -11,7 +11,7 @@ pub(crate) fn test(args: &[String]) -> Result<bool, String> {
fn evaluate_arguments(arguments: &[String]) -> Result<bool, String> {
match arguments.first() {
Some(ref s) if s.starts_with("-") && s[1..].starts_with(char::is_alphabetic) => {
Some(ref s) if s.starts_with('-') && s[1..].starts_with(char::is_alphabetic) => {
// Access the second character in the flag string: this will be type of the
// flag. If no flag was given, return `SUCCESS`
s.chars().nth(1).map_or(Ok(true), |flag| {
......@@ -112,7 +112,7 @@ fn parse_integers(left: &str, right: &str) -> Result<(Option<isize>, Option<isiz
.parse::<isize>()
.map_err(|_| format!("test: integer expression expected: {:?}", input))
{
Err(why) => Err(String::from(why)),
Err(why) => Err(why),
Ok(res) => Ok(Some(res)),
}
};
......@@ -163,8 +163,8 @@ fn file_size_is_greater_than_zero(filepath: &str) -> bool {
/// To extract the permissions from the mode, the bitwise AND operator will be used and compared
/// with the respective read bits.
fn file_has_read_permission(filepath: &str) -> bool {
const USER: u32 = 0b100000000;
const GROUP: u32 = 0b100000;
const USER: u32 = 0b1_0000_0000;
const GROUP: u32 = 0b10_0000;
const GUEST: u32 = 0b100;
// Collect the mode of permissions for the file
......
bitflags! {
struct ArgumentFlags: u8 {
/// Double quotes
const DOUBLE = 0b00000001;
const DOUBLE = 0b0000_0001;
/// Command flags
const COMM_1 = 0b00000010; // found $
const COMM_2 = 0b00000100; // found ( after $
const COMM_1 = 0b0000_0010; // found $
const COMM_2 = 0b0000_0100; // found ( after $
/// String variable
const VARIAB = 0b00001000;
const VARIAB = 0b0000_1000;
/// Array variable
const ARRAY = 0b00010000;
const METHOD = 0b00100000;
const ARRAY = 0b0001_0000;
const METHOD = 0b0010_0000;
}
}
......
......@@ -126,12 +126,12 @@ impl<'a> KeyIterator<'a> {
match &self.data[start..] {
"]" => {
return Ok(Key {
Ok(Key {
name,
kind: Primitive::AnyArray,
})
}
data @ _ => return Err(TypeError::Invalid(data)),
data @ _ => Err(TypeError::Invalid(data)),
}
}
......
......@@ -13,7 +13,7 @@ pub(crate) enum Operator {
}
impl Operator {
pub(crate) fn parse<'a>(data: &'a str) -> Result<Operator, AssignmentError<'a>> {
pub(crate) fn parse(data: &str) -> Result<Operator, AssignmentError> {
match data {
"=" => Ok(Operator::Equal),
"+=" => Ok(Operator::Add),
......
/// Given an valid assignment expression, this will split it into `keys`,
/// `operator`, `values`.
pub(crate) fn split_assignment<'a>(
statement: &'a str,
) -> (Option<&'a str>, Option<&'a str>, Option<&'a str>) {
pub(crate) fn split_assignment(statement: &str) -> (Option<&str>, Option<&str>, Option<&str>) {
let statement = statement.trim();
if statement.len() == 0 {
if statement.is_empty() {
return (None, None, None);
}
......@@ -14,7 +12,7 @@ pub(crate) fn split_assignment<'a>(
while let Some(byte) = bytes.next() {
if b'=' == byte {
if let None = statement.as_bytes().get(read + 1) {
if statement.as_bytes().get(read + 1).is_none() {
return (Some(&statement[..read].trim()), Some("="), None);
}
start = read;
......
......@@ -13,7 +13,7 @@ pub(crate) struct Collector<'a> {
lazy_static! {
/// The set of bytes that will always indicate an end of an arg
static ref FOLLOW_ARGS: HashSet<u8> = b"&|<> \t".into_iter().map(|b| *b).collect();
static ref FOLLOW_ARGS: HashSet<u8> = b"&|<> \t".into_iter().cloned().collect();
}
impl<'a> Collector<'a> {
......@@ -36,7 +36,7 @@ impl<'a> Collector<'a> {
/// Attempt to add a redirection
macro_rules! try_redir_out {
($from:expr) => {{
if let None = outputs {
if outputs.is_none() {
outputs = Some(Vec::new());
}
let append = if let Some(&(_, b'>')) = bytes.peek() {
......@@ -131,7 +131,7 @@ impl<'a> Collector<'a> {
try_redir_out!(RedirectFrom::Stdout);
}
b'<' => {
if let None = inputs {
if inputs.is_none() {
inputs = Some(Vec::new());
}
bytes.next();
......@@ -142,9 +142,8 @@ impl<'a> Collector<'a> {
bytes.next();
bytes.next();
if let Some(cmd) = self.arg(&mut bytes)? {
inputs
.as_mut()
.map(|x| x.push(Input::HereString(cmd.into())));
if let Some(x) = inputs.as_mut()
{ x.push(Input::HereString(cmd.into())) };
} else {
return Err("expected string argument after '<<<'");
}
......@@ -165,12 +164,12 @@ impl<'a> Collector<'a> {
if heredoc.len() > 1 {
let herestring =
Input::HereString(heredoc[..heredoc.len() - 1].join("\n"));
inputs.as_mut().map(|x| x.push(herestring.clone()));
if let Some(x) = inputs.as_mut() { x.push(herestring.clone()) };
}
}
} else if let Some(file) = self.arg(&mut bytes)? {
// Otherwise interpret it as stdin redirection
inputs.as_mut().map(|x| x.push(Input::File(file.into())));
if let Some(x) = inputs.as_mut() { x.push(Input::File(file.into())) };
} else {
return Err("expected file argument after redirection for input");
}
......@@ -351,13 +350,10 @@ impl<'a> Collector<'a> {
I: Iterator<Item = (usize, u8)>,
{
while let Some(&(i, b)) = bytes.peek() {
match b {
// We return an inclusive range to keep the quote type intact
b'\'' => {
bytes.next();
return Ok(&self.data[start..i + 1]);
}
_ => (),
if let b'\'' = b {
bytes.next();
return Ok(&self.data[start..i + 1]);
}
bytes.next();
}
......
......@@ -47,16 +47,14 @@ impl PipeItem {
pub(crate) fn expand(&mut self, shell: &Shell) {
self.job.expand(shell);
for input in self.inputs.iter_mut() {
for input in &mut self.inputs {
*input = match input {
&mut Input::File(ref s) => Input::File(expand_string(s, shell, false).join(" ")),
&mut Input::HereString(ref s) => {
Input::HereString(expand_string(s, shell, true).join(" "))
}
Input::File(ref s) => Input::File(expand_string(s, shell, false).join(" ")),
Input::HereString(ref s) => Input::HereString(expand_string(s, shell, true).join(" ")),
};
}
for output in self.outputs.iter_mut() {
for output in &mut self.outputs {
output.file = expand_string(output.file.as_str(), shell, false).join(" ");
}
}
......@@ -73,8 +71,8 @@ impl PipeItem {
impl Pipeline {
pub(crate) fn requires_piping(&self) -> bool {
self.items.len() > 1
|| self.items.iter().any(|it| it.outputs.len() > 0)
|| self.items.iter().any(|it| it.inputs.len() > 0)
|| self.items.iter().any(|it| !it.outputs.is_empty())
|| self.items.iter().any(|it| !it.inputs.is_empty())
|| self.items.last().unwrap().job.kind == JobKind::Background
|| self.items.last().unwrap().job.kind == JobKind::Disown
}
......@@ -97,11 +95,11 @@ impl fmt::Display for Pipeline {
tokens.extend(item.job.args.clone().into_iter());
for input in inputs {
match input {
&Input::File(ref file) => {
Input::File(ref file) => {
tokens.push("<".into());
tokens.push(file.clone());
}
&Input::HereString(ref string) => {
Input::HereString(ref string) => {
tokens.push("<<<".into());
tokens.push(string.clone());
}
......
......@@ -143,21 +143,19 @@ impl Terminator {
});
}
false
} else if let Some(b'\\') = self.buffer.bytes().last() {
let _ = self.buffer.pop();