Refactor Shell, Command, and Launcher to share impl

alacritty/src/cli.rs

@@ -18,7 +18,7 @@ use std::path::PathBuf;
 use clap::{crate_authors, crate_description, crate_name, crate_version, App, Arg};
 use log::{self, error, LevelFilter};
 
-use alacritty_terminal::config::{Delta, Dimensions, Shell, DEFAULT_NAME};
+use alacritty_terminal::config::{Delta, Dimensions, Program, DEFAULT_NAME};
 use alacritty_terminal::index::{Column, Line};
 
 use crate::config::Config;
@@ -41,7 +41,7 @@ pub struct Options {
     pub class: Option<String>,
     pub embed: Option<String>,
     pub log_level: LevelFilter,
-    pub command: Option<Shell<'static>>,
+    pub command: Option<Program>,
     pub hold: bool,
     pub working_dir: Option<PathBuf>,
     pub config: Option<PathBuf>,
@@ -243,9 +243,9 @@ impl Options {
             // The following unwrap is guaranteed to succeed.
             // If `command` exists it must also have a first item since
             // `Arg::min_values(1)` is set.
-            let command = String::from(args.next().unwrap());
+            let program = String::from(args.next().unwrap());
             let args = args.map(String::from).collect();
-            options.command = Some(Shell::new_with_args(command, args));
+            options.command = Some(Program::WithArgs { program, args });
         }
 
         if matches.is_present("hold") {

alacritty/src/config/bindings.rs

@@ -22,6 +22,7 @@ use serde::de::{self, MapAccess, Unexpected, Visitor};
 use serde::{Deserialize, Deserializer};
 use serde_yaml::Value as SerdeValue;
 
+use alacritty_terminal::config::Program;
 use alacritty_terminal::term::TermMode;
 use alacritty_terminal::vi_mode::ViMotion;
 
@@ -94,7 +95,7 @@ pub enum Action {
 
     /// Run given command.
     #[serde(skip)]
-    Command(String, Vec<String>),
+    Command(Program),
 
     /// Move vi mode cursor.
     #[serde(skip)]
@@ -763,7 +764,7 @@ impl<'a> Deserialize<'a> for RawBinding {
                 let mut mode: Option<TermMode> = None;
                 let mut not_mode: Option<TermMode> = None;
                 let mut mouse: Option<MouseButton> = None;
-                let mut command: Option<CommandWrapper> = None;
+                let mut command: Option<Program> = None;
 
                 use de::Error;
 
@@ -860,7 +861,7 @@ impl<'a> Deserialize<'a> for RawBinding {
                                 return Err(<V::Error as Error>::duplicate_field("command"));
                             }
 
-                            command = Some(map.next_value::<CommandWrapper>()?);
+                            command = Some(map.next_value::<Program>()?);
                         },
                     }
                 }
@@ -882,12 +883,7 @@ impl<'a> Deserialize<'a> for RawBinding {
                     },
                     (Some(action), None, None) => action,
                     (None, Some(chars), None) => Action::Esc(chars),
-                    (None, None, Some(cmd)) => match cmd {
-                        CommandWrapper::Just(program) => Action::Command(program, vec![]),
-                        CommandWrapper::WithArgs { program, args } => {
-                            Action::Command(program, args)
-                        },
-                    },
+                    (None, None, Some(cmd)) => Action::Command(cmd),
                     _ => {
                         return Err(V::Error::custom(
                             "must specify exactly one of chars, action or command",
@@ -929,33 +925,6 @@ impl<'a> Deserialize<'a> for KeyBinding {
     }
 }
 
-#[serde(untagged)]
-#[derive(Debug, Deserialize, Clone, PartialEq, Eq)]
-pub enum CommandWrapper {
-    Just(String),
-    WithArgs {
-        program: String,
-        #[serde(default)]
-        args: Vec<String>,
-    },
-}
-
-impl CommandWrapper {
-    pub fn program(&self) -> &str {
-        match self {
-            CommandWrapper::Just(program) => program,
-            CommandWrapper::WithArgs { program, .. } => program,
-        }
-    }
-
-    pub fn args(&self) -> &[String] {
-        match self {
-            CommandWrapper::Just(_) => &[],
-            CommandWrapper::WithArgs { args, .. } => args,
-        }
-    }
-}
-
 /// Newtype for implementing deserialize on glutin Mods.
 ///
 /// Our deserialize impl wouldn't be covered by a derive(Deserialize); see the

alacritty/src/config/mouse.rs

@@ -4,9 +4,9 @@ use glutin::event::ModifiersState;
 use log::error;
 use serde::{Deserialize, Deserializer};
 
-use alacritty_terminal::config::{failure_default, LOG_TARGET_CONFIG};
+use alacritty_terminal::config::{failure_default, Program, LOG_TARGET_CONFIG};
 
-use crate::config::bindings::{CommandWrapper, ModsWrapper};
+use crate::config::bindings::ModsWrapper;
 
 #[serde(default)]
 #[derive(Default, Clone, Debug, Deserialize, PartialEq, Eq)]
@@ -26,7 +26,7 @@ pub struct Mouse {
 pub struct Url {
     /// Program for opening links.
     #[serde(deserialize_with = "deserialize_launcher")]
-    pub launcher: Option<CommandWrapper>,
+    pub launcher: Option<Program>,
 
     /// Modifier used to open links.
     #[serde(deserialize_with = "failure_default")]
@@ -39,9 +39,7 @@ impl Url {
     }
 }
 
-fn deserialize_launcher<'a, D>(
-    deserializer: D,
-) -> ::std::result::Result<Option<CommandWrapper>, D::Error>
+fn deserialize_launcher<'a, D>(deserializer: D) -> std::result::Result<Option<Program>, D::Error>
 where
     D: Deserializer<'a>,
 {
@@ -55,7 +53,7 @@ where
         return Ok(None);
     }
 
-    match <Option<CommandWrapper>>::deserialize(val) {
+    match <Option<Program>>::deserialize(val) {
         Ok(launcher) => Ok(launcher),
         Err(err) => {
             error!(
@@ -73,11 +71,11 @@ impl Default for Url {
     fn default() -> Url {
         Url {
             #[cfg(not(any(target_os = "macos", windows)))]
-            launcher: Some(CommandWrapper::Just(String::from("xdg-open"))),
+            launcher: Some(Program::Just(String::from("xdg-open"))),
             #[cfg(target_os = "macos")]
-            launcher: Some(CommandWrapper::Just(String::from("open"))),
+            launcher: Some(Program::Just(String::from("open"))),
             #[cfg(windows)]
-            launcher: Some(CommandWrapper::Just(String::from("explorer"))),
+            launcher: Some(Program::Just(String::from("explorer"))),
             modifiers: Default::default(),
         }
     }

alacritty/src/input.rs

@@ -147,7 +147,9 @@ impl<T: EventListener> Execute<T> for Action {
                 let text = ctx.terminal_mut().clipboard().load(ClipboardType::Selection);
                 paste(ctx, &text);
             },
-            Action::Command(ref program, ref args) => {
+            Action::Command(ref program) => {
+                let args = program.args();
+                let program = program.program();
                 trace!("Running command {} with args {:?}", program, args);
 
                 match start_daemon(program, args) {

alacritty_terminal/src/config/mod.rs

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::borrow::Cow;
 use std::collections::HashMap;
 use std::fmt::Display;
 use std::path::PathBuf;
@@ -78,8 +77,8 @@ pub struct Config<T> {
     pub selection: Selection,
 
     /// Path to a shell program to run on startup.
-    #[serde(default, deserialize_with = "from_string_or_deserialize")]
-    pub shell: Option<Shell<'static>>,
+    #[serde(default, deserialize_with = "failure_default")]
+    pub shell: Option<Program>,
 
     /// Path where config was loaded from.
     #[serde(default, deserialize_with = "failure_default")]
@@ -286,33 +285,30 @@ where
         .unwrap_or_else(|_| Percentage::new(DEFAULT_CURSOR_THICKNESS)))
 }
 
-#[derive(Clone, Debug, Deserialize, PartialEq, Eq)]
-pub struct Shell<'a> {
-    pub program: Cow<'a, str>,
-
-    #[serde(default, deserialize_with = "failure_default")]
-    pub args: Vec<String>,
+#[serde(untagged)]
+#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
+pub enum Program {
+    Just(String),
+    WithArgs {
+        program: String,
+        #[serde(default, deserialize_with = "failure_default")]
+        args: Vec<String>,
+    },
 }
 
-impl<'a> Shell<'a> {
-    pub fn new<S>(program: S) -> Shell<'a>
-    where
-        S: Into<Cow<'a, str>>,
-    {
-        Shell { program: program.into(), args: Vec::new() }
+impl Program {
+    pub fn program(&self) -> &str {
+        match self {
+            Program::Just(program) => program,
+            Program::WithArgs { program, .. } => program,
+        }
     }
 
-    pub fn new_with_args<S>(program: S, args: Vec<String>) -> Shell<'a>
-    where
-        S: Into<Cow<'a, str>>,
-    {
-        Shell { program: program.into(), args }
-    }
-}
-
-impl FromString for Option<Shell<'_>> {
-    fn from(input: String) -> Self {
-        Some(Shell::new(input))
+    pub fn args(&self) -> &[String] {
+        match self {
+            Program::Just(_) => &[],
+            Program::WithArgs { args, .. } => args,
+        }
     }
 }
 
@@ -395,19 +391,3 @@ where
         value => Some(T::deserialize(value).unwrap_or_else(fallback_default)),
     })
 }
-
-pub fn from_string_or_deserialize<'de, T, D>(deserializer: D) -> Result<T, D::Error>
-where
-    D: Deserializer<'de>,
-    T: Deserialize<'de> + FromString + Default,
-{
-    Ok(match Value::deserialize(deserializer)? {
-        Value::String(value) => T::from(value),
-        value => T::deserialize(value).unwrap_or_else(fallback_default),
-    })
-}
-
-// Used over From<String>, to allow implementation for foreign types.
-pub trait FromString {
-    fn from(input: String) -> Self;
-}

alacritty_terminal/src/config/window.rs

@@ -1,10 +1,10 @@
 use std::os::raw::c_ulong;
 
-use serde::Deserialize;
+use log::error;
+use serde::{Deserialize, Deserializer};
+use serde_yaml::Value;
 
-use crate::config::{
-    failure_default, from_string_or_deserialize, option_explicit_none, Delta, FromString,
-};
+use crate::config::{failure_default, option_explicit_none, Delta, LOG_TARGET_CONFIG};
 use crate::index::{Column, Line};
 
 /// Default Alacritty name, used for window title and class.
@@ -42,7 +42,7 @@ pub struct WindowConfig {
     pub title: String,
 
     /// Window class.
-    #[serde(deserialize_with = "from_string_or_deserialize")]
+    #[serde(deserialize_with = "failure_default")]
     pub class: Class,
 
     /// XEmbed parent.
@@ -158,8 +158,7 @@ impl Dimensions {
 }
 
 /// Window class hint.
-#[serde(default)]
-#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Class {
     pub instance: String,
     pub general: String,
@@ -171,8 +170,26 @@ impl Default for Class {
     }
 }
 
-impl FromString for Class {
-    fn from(value: String) -> Self {
-        Class { instance: value, general: DEFAULT_NAME.into() }
+impl<'a> Deserialize<'a> for Class {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'a>,
+    {
+        let value = Value::deserialize(deserializer)?;
+
+        if let Value::String(instance) = value {
+            return Ok(Class { instance, general: DEFAULT_NAME.into() });
+        }
+
+        match Self::deserialize(value) {
+            Ok(value) => Ok(value),
+            Err(err) => {
+                error!(
+                    target: LOG_TARGET_CONFIG,
+                    "Problem with config: {}; using class Alacritty", err
+                );
+                Ok(Self::default())
+            },
+        }
     }
 }

alacritty_terminal/src/tty/unix.rs

@@ -14,7 +14,7 @@
 //
 //! TTY related functionality.
 
-use crate::config::{Config, Shell};
+use crate::config::{Config, Program};
 use crate::event::OnResize;
 use crate::term::SizeInfo;
 use crate::tty::{ChildEvent, EventedPty, EventedReadWrite};
@@ -163,14 +163,14 @@ pub fn new<C>(config: &Config<C>, size: &SizeInfo, window_id: Option<usize>) ->
         let shell_name = pw.shell.rsplit('/').next().unwrap();
         let argv = vec![String::from("-c"), format!("exec -a -{} {}", shell_name, pw.shell)];
 
-        Shell::new_with_args("/bin/bash", argv)
+        Program::WithArgs { program: "/bin/bash".to_owned(), args: argv }
     } else {
-        Shell::new(pw.shell)
+        Program::Just(pw.shell.to_owned())
     };
     let shell = config.shell.as_ref().unwrap_or(&default_shell);
 
-    let mut builder = Command::new(&*shell.program);
-    for arg in &shell.args {
+    let mut builder = Command::new(&*shell.program());
+    for arg in shell.args() {
         builder.arg(arg);
     }
 
@@ -246,7 +246,7 @@ pub fn new<C>(config: &Config<C>, size: &SizeInfo, window_id: Option<usize>) ->
             pty.on_resize(size);
             pty
         },
-        Err(err) => die!("Failed to spawn command '{}': {}", shell.program, err),
+        Err(err) => die!("Failed to spawn command '{}': {}", shell.program(), err),
     }
 }
 

alacritty_terminal/src/tty/windows/mod.rs

@@ -18,7 +18,7 @@ use std::iter::once;
 use std::os::windows::ffi::OsStrExt;
 use std::sync::mpsc::TryRecvError;
 
-use crate::config::{Config, Shell};
+use crate::config::{Config, Program};
 use crate::event::OnResize;
 use crate::term::SizeInfo;
 use crate::tty::windows::child::ChildExitWatcher;
@@ -197,11 +197,11 @@ impl OnResize for Pty {
 }
 
 fn cmdline<C>(config: &Config<C>) -> String {
-    let default_shell = Shell::new("powershell");
+    let default_shell = Program::Just("powershell".to_owned());
     let shell = config.shell.as_ref().unwrap_or(&default_shell);
 
-    once(shell.program.as_ref())
-        .chain(shell.args.iter().map(|a| a.as_ref()))
+    once(shell.program().as_ref())
+        .chain(shell.args().iter().map(|a| a.as_ref()))
         .collect::<Vec<_>>()
         .join(" ")
 }