Repository: pier-cli/pier (language: rust)

Issue:
Fix duplicate `alias` in configuration.
Currently the config has a duplicate definition of the script alias, would be nice to remove one of them. Will probably require a bit of code shuffling to make work though.

```toml
[scripts.this-is-the-alias]
alias = 'this-is-the-alias'
command = '''
#!/bin/sh
echo "Hello $1, my name is $2. -- script $0"
'''
```


Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 248c35d..00fcca4 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -460,7 +460,7 @@ dependencies = [
 
 [[package]]
 name = "pier"
-version = "0.1.5"
+version = "0.2.0"
 dependencies = [
  "assert_cmd",
  "assert_fs",
diff --git a/Cargo.toml b/Cargo.toml
index 5afb7ab..ebed323 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "pier"
-version = "0.1.5"
+version = "0.2.0"
 authors = [
     "Benjamin Scholtz",
     "Isak Johansson"
diff --git a/README.md b/README.md
index 575dd38..20a9943 100644
--- a/README.md
+++ b/README.md
@@ -106,20 +106,16 @@ SUBCOMMANDS:
 
 ```toml
 [scripts.refresh-wifi]
-alias = "refresh-wifi"
 command = "ip link set wlp58s0 down && sleep 5 && ip link set wlp58s0 up"
 
 [scripts.twa-analyze]
-alias = "twa-analyze"
 command = "docker run --rm -t trailofbits/twa -vw"
 tags = [ "infosec" ]
 
 [scripts.enabled-services]
-alias = "enabled-services"
 command = "systemctl list-unit-files --state=enabled"
 
 [scripts.flush-docker]
-alias = "flush-docker"
 command = "docker container stop $(docker container ls -a -q) && docker system prune -a -f --volumes"
 description = "A script to clear out old Docker containers and images"
 tags = [ "docker", "flush" ]
@@ -161,7 +157,6 @@ Scripts starting with a shebang `#!` will be run with the specified interpeter j
 
 ```
 [scripts.run_rust_script]
-alias = "run_rust_script"
 command = '''
 #!/usr/bin/env scriptisto
 
@@ -184,7 +179,6 @@ fn main() {
 '''
 
 [scripts.run_python]
-alias = "run_python"
 command = '''
 #!/usr/bin/env python3
 import sys
@@ -205,7 +199,6 @@ interpreter = ["node", "-e"]
 
 # Runs as the fallback interpreter nodejs as it's lacking a shebang
 [scripts.hello_world_nodejs]
-alias = "hello_world_nodejs"
 command = '''
 console.log("Hello world!")
 
@@ -213,7 +206,6 @@ console.log("Hello world!")
 
 # This will be run as a posix sh script as it has a shebang
 [scripts.a_shell_script]
-alias = "a_shell_script"
 command = '''
 #!/bin/sh
 
diff --git a/examples/example.tml b/examples/example.tml
index e87e537..c096c8f 100644
--- a/examples/example.tml
+++ b/examples/example.tml
@@ -1,11 +1,8 @@
 [scripts.list2]
-alias = "list2"
 command = "ls"
 
 [scripts.list]
-alias = "list"
 command = "ls"
 
 [scripts.touch]
-alias = "touch"
 command = "touch test"
diff --git a/src/config.rs b/src/config.rs
index dce7e11..1770dc8 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -1,9 +1,14 @@
+use std::{collections::BTreeMap, fmt, fs, path::PathBuf};
+use std::collections::btree_map::Iter;
+use std::marker::PhantomData;
+
+use serde::{Deserialize, Deserializer, Serialize};
+use serde::de::{MapAccess, Visitor};
+use snafu::ResultExt;
+
 use super::error::*;
-use super::script::Script;
 use super::Result;
-use serde::{Deserialize, Serialize};
-use snafu::ResultExt;
-use std::{collections::BTreeMap, fs, path::PathBuf};
+use super::script::Script;
 
 #[derive(Serialize, Deserialize, Debug, Default)]
 pub struct ConfigDefaultOpts {
@@ -18,14 +23,16 @@ pub struct ConfigDefaultOpts {
 
 #[derive(Serialize, Deserialize, Debug, Default)]
 pub struct Config {
-    // Don't write anything to config if map is empty
-    #[serde(default = "BTreeMap::new", skip_serializing_if = "BTreeMap::is_empty")]
-    pub scripts: BTreeMap<String, Script>,
+    #[serde(default)]
+    pub scripts: Scripts,
 
     #[serde(default)]
     pub default: ConfigDefaultOpts,
 }
 
+#[derive(Serialize, Debug, Default)]
+pub struct Scripts(BTreeMap<String, Script>);
+
 impl Config {
     /// Helper function to read file.
     pub fn read(path: &PathBuf) -> Result<String> {
@@ -51,3 +58,79 @@ impl Config {
         Ok(config)
     }
 }
+
+impl Scripts {
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+
+    pub fn get(&self, key: &str) -> Option<&Script> {
+        self.0.get(key)
+    }
+
+    pub fn get_mut(&mut self, key: &str) -> Option<&mut Script> {
+        self.0.get_mut(key)
+    }
+
+    pub fn contains_key(&self, key: &str) -> bool {
+        self.0.contains_key(key)
+    }
+
+    pub fn insert(&mut self, key: String, script: Script) -> Option<Script> {
+        self.0.insert(key, script)
+    }
+
+    pub fn remove(&mut self, key: &str) -> Option<Script> {
+        self.0.remove(key)
+    }
+
+    pub fn iter(&self) -> Iter<'_, String, Script> {
+        self.0.iter()
+    }
+}
+
+struct ScriptsVisitor {
+    marker: PhantomData<fn() -> Scripts>
+}
+
+impl ScriptsVisitor {
+    fn new() -> Self {
+        ScriptsVisitor {
+            marker: PhantomData
+        }
+    }
+}
+// Gives us an iterating over map entries during deserialization process where we can access entry
+// key and modify the script to populate alias.
+impl<'de> Visitor<'de> for ScriptsVisitor
+{
+    type Value = Scripts;
+
+    fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+        formatter.write_str("map is expected")
+    }
+
+    fn visit_map<M>(self, mut access: M) -> Result<Self::Value, M::Error>
+        where
+            M: MapAccess<'de>,
+    {
+        let mut map = Scripts(BTreeMap::new());
+
+        while let Some((key, mut value)) = access.next_entry::<String, Script>()? {
+            value.alias = key.clone();
+            map.0.insert(key, value);
+        }
+
+        Ok(map)
+    }
+}
+// Custom serde deserialization
+impl<'de> Deserialize<'de> for Scripts
+{
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+        where
+            D: Deserializer<'de>,
+    {
+        deserializer.deserialize_map(ScriptsVisitor::new())
+    }
+}
diff --git a/src/lib.rs b/src/lib.rs
index 77ff9b4..7b0861e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -92,7 +92,7 @@ impl Pier {
         let script = self
             .config
             .scripts
-            .get(&alias.to_string())
+            .get(alias)
             .context(AliasNotFound {
                 alias: &alias.to_string(),
             })?;
@@ -107,7 +107,7 @@ impl Pier {
         let mut script =
             self.config
                 .scripts
-                .get_mut(&alias.to_string())
+                .get_mut(alias)
                 .context(AliasNotFound {
                     alias: &alias.to_string(),
                 })?;
@@ -125,7 +125,7 @@ impl Pier {
 
         self.config
             .scripts
-            .remove(&alias.to_string())
+            .remove(alias)
             .context(AliasNotFound {
                 alias: &alias.to_string(),
             })?;
@@ -157,7 +157,7 @@ impl Pier {
     pub fn list_aliases(&self, tags: Option<Vec<String>>) -> Result<()> {
         ensure!(!self.config.scripts.is_empty(), NoScriptsExists);
 
-        for (alias, script) in &self.config.scripts {
+        for (alias, script) in self.config.scripts.iter() {
             match (&tags, &script.tags) {
                 (Some(list_tags), Some(script_tags)) => {
                     for tag in list_tags {
@@ -183,7 +183,7 @@ impl Pier {
     /// Copy an alias a script that matches the alias
     pub fn copy_script(&mut self, from_alias: &str, new_alias: &str) -> Result<()> {
         ensure!(
-            !&self.config.scripts.contains_key(&new_alias.to_string()),
+            !&self.config.scripts.contains_key(new_alias),
             AliasAlreadyExists { alias: new_alias }
         );
 
@@ -191,7 +191,7 @@ impl Pier {
         let script = self
             .config
             .scripts
-            .get(&from_alias.to_string())
+            .get(from_alias)
             .context(AliasNotFound {
                 alias: &from_alias.to_string(),
             })?
@@ -212,7 +212,7 @@ impl Pier {
     pub fn move_script(&mut self, from_alias: &str, new_alias: &str, force: bool) -> Result<()> {
         if !force {
             ensure!(
-                !&self.config.scripts.contains_key(&new_alias.to_string()),
+                !&self.config.scripts.contains_key(new_alias),
                 AliasAlreadyExists { alias: new_alias }
             );
         }
@@ -220,7 +220,7 @@ impl Pier {
         let script = self
             .config
             .scripts
-            .remove(&from_alias.to_string())
+            .remove(from_alias)
             .context(AliasNotFound {
                 alias: &from_alias.to_string(),
             })?
@@ -256,7 +256,7 @@ impl Pier {
         table.set_format(*format::consts::FORMAT_DEFAULT);
         table.set_titles(row!["Alias", "Tag(s)", "Description", "Command"]);
 
-        for (alias, script) in &self.config.scripts {
+        for (alias, script) in self.config.scripts.iter() {
 
             match (&tags, &script.tags, &script.description) {
                 (Some(list_tags), Some(script_tags), Some(description)) => {
diff --git a/src/script.rs b/src/script.rs
index 45dddfc..28e77cd 100644
--- a/src/script.rs
+++ b/src/script.rs
@@ -10,6 +10,7 @@ use tempfile;
 
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub struct Script {
+    #[serde(skip)]
     pub alias: String,
     pub command: String,
     pub description: Option<String>,


Test patch:
diff --git a/tests/cli.rs b/tests/cli.rs
index d10a5c2..2055bf6 100644
--- a/tests/cli.rs
+++ b/tests/cli.rs
@@ -12,7 +12,6 @@ command = 'echo test_1'
 tags = ['info', 'echo', 'grp_1']
 
 [scripts.test_cmd_2]
-alias = 'test_cmd_2'
 command = 'echo test_2'
 tags = ['debug', 'echo']
 
@@ -184,7 +183,6 @@ pier_test!(cli => test_add_script, cfg => CONFIG_1,
 
     cfg.assert(contains(trim!(r#"
         [scripts.test_cmd_3]
-        alias = 'test_cmd_3'
         command = 'echo test_3'
         description = 'Test Description.'
     "#)).trim()
@@ -199,7 +197,6 @@ pier_test!(cli => test_add_script_force_script, cfg => CONFIG_1,
 
     cfg.assert(contains(trim!(r#"
         [scripts.test_cmd_1]
-        alias = 'test_cmd_1'
         command = 'echo test_3'
         description = 'Test Description.'
     "#)).trim()
@@ -212,9 +209,14 @@ pier_test!(cli => test_copy_script, cfg => CONFIG_1,
     cmd.args(&["copy", "test_cmd_1", "test_cmd_4"]);
     cmd.assert().success();
 
+    cfg.assert(contains(trim!(r#"
+            [scripts.test_cmd_1]
+            command = 'echo test_1'
+        "#)).trim()
+    );
+
     cfg.assert(contains(trim!(r#"
             [scripts.test_cmd_4]
-            alias = 'test_cmd_1'
             command = 'echo test_1'
         "#)).trim()
     );
@@ -228,7 +230,6 @@ pier_test!(cli => test_move_script, cfg => CONFIG_1,
 
     cfg.assert(contains(trim!(r#"
             [scripts.test_cmd_4]
-            alias = 'test_cmd_1'
             command = 'echo test_1'
         "#)).trim()
     );
@@ -248,13 +249,11 @@ pier_test!(cli => test_move_with_force_script, cfg => CONFIG_1,
 
     cfg.assert(contains(trim!(r#"
             [scripts.test_cmd_2]
-            alias = 'test_cmd_1'
             command = 'echo test_1'
         "#)).trim()
     );
     cfg.assert(contains(trim!(r#"
             [scripts.test_cmd_1]
-            alias = 'test_cmd_1'
             command = 'echo test_1'
         "#)).trim().not()
     );