From 49d122582359f1b486a4e21ceef9accb26aeeeab Mon Sep 17 00:00:00 2001
From: CapsizeGlimmer <>
Date: Mon, 11 May 2020 18:02:21 -0400
Subject: [PATCH] Tab completion code review suggestions

---
 client/src/cmd.rs        |  2 +-
 common/src/assets/mod.rs | 31 +++----------------------------
 common/src/cmd.rs        | 36 +++++++++++++++++++++++++++++++++---
 server/src/cmd.rs        | 12 ++++++++++++
 4 files changed, 49 insertions(+), 32 deletions(-)

diff --git a/client/src/cmd.rs b/client/src/cmd.rs
index 9b05f9b779..f24152fd92 100644
--- a/client/src/cmd.rs
+++ b/client/src/cmd.rs
@@ -74,7 +74,7 @@ fn nth_word(line: &str, n: usize) -> Option<usize> {
             return Some(i);
         }
     }
-    return None;
+    None
 }
 
 pub fn complete(line: &str, client: &Client) -> Vec<String> {
diff --git a/common/src/assets/mod.rs b/common/src/assets/mod.rs
index 003ec94a54..58e20d903f 100644
--- a/common/src/assets/mod.rs
+++ b/common/src/assets/mod.rs
@@ -12,7 +12,7 @@ use std::{
     fmt,
     fs::{self, File, ReadDir},
     io::{BufReader, Read},
-    path::{Path, PathBuf},
+    path::PathBuf,
     sync::{Arc, RwLock},
 };
 
@@ -57,33 +57,8 @@ impl From<std::io::Error> for Error {
 
 lazy_static! {
     /// The HashMap where all loaded assets are stored in.
-    pub static ref ASSETS: RwLock<HashMap<String, Arc<dyn Any + 'static + Sync + Send>>> =
+    static ref ASSETS: RwLock<HashMap<String, Arc<dyn Any + 'static + Sync + Send>>> =
         RwLock::new(HashMap::new());
-
-    /// List of item specifiers. Useful for tab completing
-    pub static ref ITEM_SPECS: Vec<String> = {
-        let base = ASSETS_PATH.join("common").join("items");
-        let mut items = vec![];
-        fn list_items (path: &Path, base: &Path, mut items: &mut Vec<String>) -> std::io::Result<()>{
-            for entry in std::fs::read_dir(path)? {
-                let path = entry?.path();
-                if path.is_dir(){
-                    list_items(&path, &base, &mut items)?;
-                } else {
-                    if let Ok(path) = path.strip_prefix(base) {
-                        let path = path.to_string_lossy().trim_end_matches(".ron").replace('/', ".");
-                        items.push(path);
-                    }
-                }
-            }
-            Ok(())
-        }
-        if list_items(&base, &ASSETS_PATH, &mut items).is_err() {
-            warn!("There was a problem listing some item assets");
-        }
-        items.sort();
-        items
-    };
 }
 
 // TODO: Remove this function. It's only used in world/ in a really ugly way.To
@@ -301,7 +276,7 @@ impl Asset for String {
 
 lazy_static! {
     /// Lazy static to find and cache where the asset directory is.
-    static ref ASSETS_PATH: PathBuf = {
+    pub static ref ASSETS_PATH: PathBuf = {
         let mut paths = Vec::new();
 
         // VELOREN_ASSETS environment variable
diff --git a/common/src/cmd.rs b/common/src/cmd.rs
index be265b5203..66531fbd92 100644
--- a/common/src/cmd.rs
+++ b/common/src/cmd.rs
@@ -1,10 +1,10 @@
 use crate::{assets, comp, npc};
 use lazy_static::lazy_static;
-use std::{ops::Deref, str::FromStr};
+use std::{ops::Deref, path::Path, str::FromStr};
 
 /// Struct representing a command that a user can run from server chat.
 pub struct ChatCommandData {
-    /// A format string for parsing arguments.
+    /// A list of arguments useful for both tab completion and parsing
     pub args: Vec<ArgumentSpec>,
     /// A one-line message that explains what the command does
     pub description: &'static str,
@@ -108,6 +108,31 @@ lazy_static! {
     .iter()
     .map(|s| s.to_string())
     .collect();
+
+    /// List of item specifiers. Useful for tab completing
+    static ref ITEM_SPECS: Vec<String> = {
+        let path = assets::ASSETS_PATH.join("common").join("items");
+        let mut items = vec![];
+        fn list_items (path: &Path, base: &Path, mut items: &mut Vec<String>) -> std::io::Result<()>{
+            for entry in std::fs::read_dir(path)? {
+                let path = entry?.path();
+                if path.is_dir(){
+                    list_items(&path, &base, &mut items)?;
+                } else {
+                    if let Ok(path) = path.strip_prefix(base) {
+                        let path = path.to_string_lossy().trim_end_matches(".ron").replace('/', ".");
+                        items.push(path);
+                    }
+                }
+            }
+            Ok(())
+        }
+        if list_items(&path, &assets::ASSETS_PATH, &mut items).is_err() {
+            warn!("There was a problem listing item assets");
+        }
+        items.sort();
+        items
+    };
 }
 
 impl ChatCommand {
@@ -141,7 +166,7 @@ impl ChatCommand {
             ),
             ChatCommand::GiveItem => cmd(
                 vec![
-                    Enum("item", assets::ITEM_SPECS.clone(), Required),
+                    Enum("item", ITEM_SPECS.clone(), Required),
                     Integer("num", 1, Optional),
                 ],
                 "Give yourself some items",
@@ -252,6 +277,7 @@ impl ChatCommand {
         }
     }
 
+    /// The keyword used to invoke the command, omitting the leading '/'.
     pub fn keyword(&self) -> &'static str {
         match self {
             ChatCommand::Adminify => "adminify",
@@ -284,6 +310,7 @@ impl ChatCommand {
         }
     }
 
+    /// A message that explains what the command does
     pub fn help_string(&self) -> String {
         let data = self.data();
         let usage = std::iter::once(format!("/{}", self.keyword()))
@@ -293,8 +320,11 @@ impl ChatCommand {
         format!("{}: {}", usage, data.description)
     }
 
+    /// A boolean that is used to check whether the command requires
+    /// administrator permissions or not.
     pub fn needs_admin(&self) -> bool { self.data().needs_admin }
 
+    /// Returns a format string for parsing arguments with scan_fmt
     pub fn arg_fmt(&self) -> String {
         self.data()
             .args
diff --git a/server/src/cmd.rs b/server/src/cmd.rs
index 9beaf292c3..38f405198e 100644
--- a/server/src/cmd.rs
+++ b/server/src/cmd.rs
@@ -46,6 +46,18 @@ impl ChatCommandExt for ChatCommand {
     }
 }
 
+/// Handler function called when the command is executed.
+/// # Arguments
+/// * `&mut Server` - the `Server` instance executing the command.
+/// * `EcsEntity` - an `Entity` corresponding to the player that invoked the
+///   command.
+/// * `EcsEntity` - an `Entity` for the player on whom the command is invoked.
+///   This differs from the previous argument when using /sudo
+/// * `String` - a `String` containing the part of the command after the
+///   keyword.
+/// * `&ChatCommand` - the command to execute with the above arguments.
+/// Handler functions must parse arguments from the the given `String`
+/// (`scan_fmt!` is included for this purpose).
 type CommandHandler = fn(&mut Server, EcsEntity, EcsEntity, String, &ChatCommand);
 fn get_handler(cmd: &ChatCommand) -> CommandHandler {
     match cmd {