Makes code more rusty/idiomatic (#264)

* makes eww code more idiomatic

* makes simplexpr more idiomatic

* makes yuck more idiomatic

crates/eww/src/app.rs

@@ -153,7 +153,7 @@ impl App {
                     sender.respond_with_result(result)?;
                 }
                 DaemonCommand::CloseWindows { windows, sender } => {
-                    let errors = windows.iter().map(|window| self.close_window(&window)).filter_map(Result::err);
+                    let errors = windows.iter().map(|window| self.close_window(window)).filter_map(Result::err);
                     sender.respond_with_error_list(errors)?;
                 }
                 DaemonCommand::PrintState { all, sender } => {
@@ -238,7 +238,7 @@ impl App {
             window_def.geometry = window_def.geometry.map(|x| x.override_if_given(anchor, pos, size));
 
             let root_widget =
-                window_def.widget.render(&mut self.eww_state, window_name, &self.eww_config.get_widget_definitions())?;
+                window_def.widget.render(&mut self.eww_state, window_name, self.eww_config.get_widget_definitions())?;
 
             root_widget.get_style_context().add_class(&window_name.to_string());
 
@@ -352,7 +352,7 @@ fn initialize_window(
         if let Some(geometry) = window_def.geometry {
             let _ = apply_window_position(geometry, monitor_geometry, &window);
             window.connect_configure_event(move |window, _| {
-                let _ = apply_window_position(geometry, monitor_geometry, &window);
+                let _ = apply_window_position(geometry, monitor_geometry, window);
                 false
             });
         }

crates/eww/src/config/eww_config.rs

@@ -70,7 +70,7 @@ impl EwwConfig {
         &self.windows
     }
 
-    pub fn get_window(&self, name: &String) -> Result<&EwwWindowDefinition> {
+    pub fn get_window(&self, name: &str) -> Result<&EwwWindowDefinition> {
         self.windows.get(name).with_context(|| {
             format!(
                 "No window named '{}' exists in config.\nThis may also be caused by your config failing to load properly, \

crates/eww/src/opts.rs

@@ -173,10 +173,7 @@ fn parse_var_update_arg(s: &str) -> Result<(VarName, DynVal)> {
 
 impl ActionWithServer {
     pub fn can_start_daemon(&self) -> bool {
-        match self {
-            ActionWithServer::OpenWindow { .. } | ActionWithServer::OpenMany { .. } => true,
-            _ => false,
-        }
+        matches!(self, ActionWithServer::OpenWindow {..} | ActionWithServer::OpenMany { .. })
     }
 
     pub fn into_daemon_command(self) -> (app::DaemonCommand, Option<daemon_response::DaemonResponseReceiver>) {

crates/eww/src/server.rs

@@ -152,7 +152,7 @@ async fn run_filewatch<P: AsRef<Path>>(config_dir: P, evt_send: UnboundedSender<
         Ok(_) => {}
         Err(e) => log::error!("Encountered Error While Watching Files: {}", e),
     })?;
-    watcher.watch(&config_dir.as_ref(), RecursiveMode::Recursive)?;
+    watcher.watch(config_dir.as_ref(), RecursiveMode::Recursive)?;
 
     // make sure to not trigger reloads too much by only accepting one reload every 500ms.
     let debounce_done = Arc::new(std::sync::atomic::AtomicBool::new(true));
@@ -184,7 +184,7 @@ async fn run_filewatch<P: AsRef<Path>>(config_dir: P, evt_send: UnboundedSender<
         },
         else => break
     };
-    return Ok(());
+    Ok(())
 }
 
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -213,7 +213,7 @@ fn do_detach(log_file_path: impl AsRef<Path>) -> Result<ForkResult> {
         .create(true)
         .append(true)
         .open(&log_file_path)
-        .expect(&format!("Error opening log file ({}), for writing", log_file_path.as_ref().to_string_lossy()));
+        .unwrap_or_else(|_| panic!("Error opening log file ({}), for writing", log_file_path.as_ref().to_string_lossy()));
     let fd = file.as_raw_fd();
 
     if nix::unistd::isatty(1)? {

crates/eww/src/widgets/widget_definitions.rs

@@ -8,7 +8,7 @@ use gdk::WindowExt;
 use glib;
 use gtk::{self, prelude::*, ImageExt};
 use itertools::Itertools;
-use std::{cell::RefCell, collections::HashMap, rc::Rc, time::Duration};
+use std::{cell::RefCell, collections::HashMap, rc::Rc, time::Duration, cmp::Ordering};
 use yuck::{
     config::validate::ValidationError,
     error::{AstError, AstResult, AstResultExt},
@@ -41,7 +41,7 @@ pub(super) fn widget_to_gtk_widget(bargs: &mut BuilderArgs) -> Result<gtk::Widge
         "revealer" => build_gtk_revealer(bargs)?.upcast(),
         "if-else" => build_if_else(bargs)?.upcast(),
         _ => {
-            Err(AstError::ValidationError(ValidationError::UnknownWidget(bargs.widget.name_span, bargs.widget.name.to_string())))?
+            return Err(AstError::ValidationError(ValidationError::UnknownWidget(bargs.widget.name_span, bargs.widget.name.to_string())).into())
         }
     };
     Ok(gtk_widget)
@@ -508,16 +508,18 @@ fn build_center_box(bargs: &mut BuilderArgs) -> Result<gtk::Box> {
         prop(orientation: as_string) { gtk_widget.set_orientation(parse_orientation(&orientation)?) },
     });
 
-    if bargs.widget.children.len() < 3 {
-        Err(DiagError::new(gen_diagnostic!("centerbox must contain exactly 3 elements", bargs.widget.span)))?
-    } else if bargs.widget.children.len() > 3 {
+    match bargs.widget.children.len().cmp(&3) {
+        Ordering::Less => {
+            Err(DiagError::new(gen_diagnostic!("centerbox must contain exactly 3 elements", bargs.widget.span)).into())
+        }
+        Ordering::Greater => {
             let (_, additional_children) = bargs.widget.children.split_at(3);
             // we know that there is more than three children, so unwrapping on first and left here is fine.
             let first_span = additional_children.first().unwrap().span();
             let last_span = additional_children.last().unwrap().span();
-        Err(DiagError::new(gen_diagnostic!("centerbox must contain exactly 3 elements, but got more", first_span.to(last_span))))?
+            Err(DiagError::new(gen_diagnostic!("centerbox must contain exactly 3 elements, but got more", first_span.to(last_span))).into())
         }
-
+        Ordering::Equal => {
             let mut children =
             bargs.widget.children.iter().map(|child| child.render(bargs.eww_state, bargs.window_name, bargs.widget_definitions));
             // we know that we have exactly three children here, so we can unwrap here.
@@ -530,6 +532,8 @@ fn build_center_box(bargs: &mut BuilderArgs) -> Result<gtk::Box> {
             center.show();
             end.show();
             Ok(gtk_widget)
+        }
+    }
 }
 
 /// @widget label

crates/eww/src/widgets/widget_node.rs

@@ -113,7 +113,7 @@ pub fn generate_generic_widget_node(
 ) -> AstResult<Box<dyn WidgetNode>> {
     if let Some(def) = defs.get(&w.name) {
         if !w.children.is_empty() {
-            Err(AstError::TooManyNodes(w.children_span(), 0).note("User-defined widgets cannot be given children."))?
+            return Err(AstError::TooManyNodes(w.children_span(), 0).note("User-defined widgets cannot be given children."))
         }
 
         let mut new_local_env = w
@@ -126,9 +126,7 @@ pub fn generate_generic_widget_node(
         // handle default value for optional arguments
         for expected in def.expected_args.iter().filter(|x| x.optional) {
             let var_name = VarName(expected.name.clone().0);
-            if !new_local_env.contains_key(&var_name) {
-                new_local_env.insert(var_name, SimplExpr::literal(expected.span, String::new()));
-            }
+            new_local_env.entry(var_name).or_insert_with(|| SimplExpr::literal(expected.span, String::new()));
         }
 
         let content = generate_generic_widget_node(defs, &new_local_env, def.widget.clone())?;

crates/simplexpr/src/eval.rs

@@ -230,19 +230,19 @@ impl SimplExpr {
                 }
             }
             SimplExpr::FunctionCall(span, function_name, args) => {
-                let args = args.into_iter().map(|a| a.eval(values)).collect::<Result<_, EvalError>>()?;
-                call_expr_function(&function_name, args).map(|x| x.at(*span)).map_err(|e| e.at(*span))
+                let args = args.iter().map(|a| a.eval(values)).collect::<Result<_, EvalError>>()?;
+                call_expr_function(function_name, args).map(|x| x.at(*span)).map_err(|e| e.at(*span))
             }
             SimplExpr::JsonArray(span, entries) => {
                 let entries = entries
-                    .into_iter()
+                    .iter()
                     .map(|v| Ok(serde_json::Value::String(v.eval(values)?.as_string()?)))
                     .collect::<Result<_, EvalError>>()?;
                 Ok(DynVal::try_from(serde_json::Value::Array(entries))?.at(*span))
             }
             SimplExpr::JsonObject(span, entries) => {
                 let entries = entries
-                    .into_iter()
+                    .iter()
                     .map(|(k, v)| Ok((k.eval(values)?.as_string()?, serde_json::Value::String(v.eval(values)?.as_string()?))))
                     .collect::<Result<_, EvalError>>()?;
                 Ok(DynVal::try_from(serde_json::Value::Object(entries))?.at(*span))

crates/simplexpr/src/parser/lalrpop_helpers.rs

@@ -21,7 +21,7 @@ pub fn parse_stringlit(
         match seg {
             StrLitSegment::Literal(lit) => Ok(SimplExpr::Literal(DynVal(lit, span))),
             StrLitSegment::Interp(toks) => {
-                let token_stream = toks.into_iter().map(|x| Ok(x));
+                let token_stream = toks.into_iter().map(Ok);
                 parser.parse(file_id, token_stream)
             }
         }
@@ -34,7 +34,7 @@ pub fn parse_stringlit(
                     StrLitSegment::Literal(lit) if lit.is_empty() => None,
                     StrLitSegment::Literal(lit) => Some(Ok(SimplExpr::Literal(DynVal(lit, span)))),
                     StrLitSegment::Interp(toks) => {
-                        let token_stream = toks.into_iter().map(|x| Ok(x));
+                        let token_stream = toks.into_iter().map(Ok);
                         Some(parser.parse(file_id, token_stream))
                     }
                 }

crates/simplexpr/src/parser/lexer.rs

@@ -103,8 +103,8 @@ regex_rules! {
     r"[ \n\n\f]+" => |_| Token::Skip,
     r";.*"=> |_| Token::Comment,
 
-    r"[a-zA-Z_][a-zA-Z0-9_-]*" => |x| Token::Ident(x.to_string()),
-    r"[+-]?(?:[0-9]+[.])?[0-9]+" => |x| Token::NumLit(x.to_string())
+    r"[a-zA-Z_][a-zA-Z0-9_-]*" => |x| Token::Ident(x),
+    r"[+-]?(?:[0-9]+[.])?[0-9]+" => |x| Token::NumLit(x)
 }
 
 #[derive(Debug)]

crates/yuck/src/config/attributes.rs

@@ -87,12 +87,12 @@ impl Attributes {
         E: std::error::Error + 'static + Sync + Send,
         T: FromDynVal<Err = E>,
     {
-        let ast: SimplExpr = self.ast_required(&key)?;
+        let ast: SimplExpr = self.ast_required(key)?;
         Ok(ast
             .eval_no_vars()
             .map_err(|err| AttrError::EvaluationError(ast.span(), err))?
             .read_as()
-            .map_err(|e| AttrError::Other(ast.span().into(), Box::new(e)))?)
+            .map_err(|e| AttrError::Other(ast.span(), Box::new(e)))?)
     }
 
     pub fn primitive_optional<T, E>(&mut self, key: &str) -> Result<Option<T>, AstError>
@@ -108,7 +108,7 @@ impl Attributes {
             ast.eval_no_vars()
                 .map_err(|err| AttrError::EvaluationError(ast.span(), err))?
                 .read_as()
-                .map_err(|e| AttrError::Other(ast.span().into(), Box::new(e)))?,
+                .map_err(|e| AttrError::Other(ast.span(), Box::new(e)))?,
         ))
     }
 

crates/yuck/src/config/file_provider.rs

@@ -98,7 +98,7 @@ impl YuckFiles {
         let yuck_file =
             YuckFile { name, line_starts, source_len_bytes: content.len(), source: YuckSource::Literal(content.to_string()) };
         let file_id = self.insert_file(yuck_file);
-        Ok(crate::parser::parse_toplevel(file_id, content)?)
+        crate::parser::parse_toplevel(file_id, content)
     }
 
     pub fn unload(&mut self, id: usize) {
@@ -116,7 +116,7 @@ impl<'a> Files<'a> for YuckFiles {
     }
 
     fn source(&'a self, id: Self::FileId) -> Result<Self::Source, codespan_reporting::files::Error> {
-        Ok(self.get_file(id)?.source.read_content().map_err(codespan_reporting::files::Error::Io)?)
+        self.get_file(id)?.source.read_content().map_err(codespan_reporting::files::Error::Io)
     }
 
     fn line_index(&self, id: Self::FileId, byte_index: usize) -> Result<usize, codespan_reporting::files::Error> {

crates/yuck/src/config/validate.rs

@@ -47,7 +47,7 @@ pub fn validate(config: &Config, additional_globals: Vec<VarName>) -> Result<(),
         validate_variables_in_widget_use(&config.widget_definitions, &var_names, &window.widget, false)?;
     }
     for def in config.widget_definitions.values() {
-        validate_widget_definition(&config.widget_definitions, &var_names, &def)?;
+        validate_widget_definition(&config.widget_definitions, &var_names, def)?;
     }
     Ok(())
 }
@@ -97,7 +97,7 @@ pub fn validate_variables_in_widget_use(
             .find(|(_, var_ref)| !variables.contains(var_ref))
     });
     if let Some((span, var)) = unknown_var {
-        return Err(ValidationError::UnknownVariable { span, name: var.clone(), in_definition: is_in_definition });
+        return Err(ValidationError::UnknownVariable { span, name: var, in_definition: is_in_definition });
     }
 
     for child in widget.children.iter() {

crates/yuck/src/config/widget_use.rs

@@ -54,7 +54,7 @@ fn label_from_simplexpr(value: SimplExpr, span: Span) -> WidgetUse {
             maplit::hashmap! {
                 AttrName("text".to_string()) => AttrEntry::new(
                     span,
-                    Ast::SimplExpr(span.into(), value.clone())
+                    Ast::SimplExpr(span, value)
                 )
             },
         ),

crates/yuck/src/config/window_definition.rs

@@ -70,7 +70,7 @@ macro_rules! enum_parse {
         match input.as_str() {
             $( $( $s )|* => Ok($val) ),*,
             _ => Err(EnumParseError {
-                input: input,
+                input,
                 expected: vec![$($($s),*),*],
             })
         }

crates/yuck/src/parser/from_ast.rs

@@ -50,7 +50,7 @@ impl<T: FromAstElementContent> FromAst for T {
 impl FromAst for SimplExpr {
     fn from_ast(e: Ast) -> AstResult<Self> {
         match e {
-            Ast::Symbol(span, x) => Ok(SimplExpr::VarRef(span.into(), VarName(x))),
+            Ast::Symbol(span, x) => Ok(SimplExpr::VarRef(span, VarName(x))),
             Ast::SimplExpr(span, x) => Ok(x),
             _ => Err(AstError::NotAValue(e.span(), e.expr_type())),
         }