Plugin repeated (#417)

* not repeated decl in file and help

* implemented heashmap for repeated

* sorted scope commands

Cargo.lock

@@ -1479,7 +1479,6 @@ version = "0.1.0"
 dependencies = [
  "chrono",
  "itertools",
- "nu-parser",
  "nu-path",
  "nu-protocol",
 ]
@@ -1503,6 +1502,7 @@ version = "0.1.0"
 dependencies = [
  "miette",
  "nu-path",
+ "nu-plugin",
  "nu-protocol",
  "serde_json",
  "thiserror",

crates/nu-engine/Cargo.toml

@@ -4,7 +4,6 @@ version = "0.1.0"
 edition = "2018"
 
 [dependencies]
-nu-parser = { path = "../nu-parser" }
 nu-protocol = { path = "../nu-protocol", features = ["plugin"] }
 nu-path = { path = "../nu-path" }
 itertools = "0.10.1"

crates/nu-engine/src/eval.rs

@@ -1,3 +1,5 @@
+use std::cmp::Ordering;
+
 use nu_protocol::ast::{Block, Call, Expr, Expression, Operator, Statement};
 use nu_protocol::engine::{EngineState, Stack};
 use nu_protocol::{
@@ -637,18 +639,37 @@ pub fn eval_variable(
             span,
         });
 
+        commands.sort_by(|a, b| match (a, b) {
+            (Value::Record { vals: rec_a, .. }, Value::Record { vals: rec_b, .. }) => {
+                // Comparing the first value from the record
+                // It is expected that the first value is the name of the column
+                // The names of the commands should be a value string
+                match (rec_a.get(0), rec_b.get(0)) {
+                    (Some(val_a), Some(val_b)) => match (val_a, val_b) {
+                        (Value::String { val: str_a, .. }, Value::String { val: str_b, .. }) => {
+                            str_a.cmp(str_b)
+                        }
+                        _ => Ordering::Equal,
+                    },
+                    _ => Ordering::Equal,
+                }
+            }
+            _ => Ordering::Equal,
+        });
         output_cols.push("commands".to_string());
         output_vals.push(Value::List {
             vals: commands,
             span,
         });
 
+        aliases.sort_by(|a, b| a.partial_cmp(b).unwrap_or(Ordering::Equal));
         output_cols.push("aliases".to_string());
         output_vals.push(Value::List {
             vals: aliases,
             span,
         });
 
+        overlays.sort_by(|a, b| a.partial_cmp(b).unwrap_or(Ordering::Equal));
         output_cols.push("overlays".to_string());
         output_vals.push(Value::List {
             vals: overlays,

crates/nu-parser/Cargo.toml

@@ -9,6 +9,7 @@ thiserror = "1.0.29"
 serde_json = "1.0"
 nu-path = {path = "../nu-path"}
 nu-protocol = { path = "../nu-protocol"}
+nu-plugin = { path = "../nu-plugin", optional = true }
 
 [features]
-plugin = []
+plugin = ["nu-plugin"]

crates/nu-parser/src/lib.rs

@@ -16,4 +16,4 @@ pub use parse_keywords::{
 pub use parser::{find_captures_in_expr, parse, Import};
 
 #[cfg(feature = "plugin")]
-pub use parse_keywords::parse_plugin;
+pub use parse_keywords::parse_register;

crates/nu-parser/src/parse_keywords.rs

@@ -1084,13 +1084,14 @@ pub fn parse_source(
 }
 
 #[cfg(feature = "plugin")]
-pub fn parse_plugin(
+pub fn parse_register(
     working_set: &mut StateWorkingSet,
     spans: &[Span],
 ) -> (Statement, Option<ParseError>) {
     use std::{path::PathBuf, str::FromStr};
 
     use nu_path::canonicalize;
+    use nu_plugin::plugin::{get_signature, PluginDeclaration};
     use nu_protocol::Signature;
 
     let name = working_set.get_span_contents(spans[0]);
@@ -1124,18 +1125,35 @@ pub fn parse_plugin(
                         })
                         .and_then(|path| {
                             if path.exists() & path.is_file() {
-                                working_set.add_plugin_signature(path, None);
+                                get_signature(path.as_path())
-                                Ok(())
+                                    .map_err(|err| {
+                                        ParseError::LabeledError(
+                                            "Error getting signatures".into(),
+                                            err.to_string(),
+                                            spans[0],
+                                        )
+                                    })
+                                    .map(|signatures| (path, signatures))
                             } else {
                                 Err(ParseError::FileNotFound(format!("{:?}", path)))
                             }
                         })
+                        .map(|(path, signatures)| {
+                            for signature in signatures {
+                                // create plugin command declaration (need struct impl Command)
+                                // store declaration in working set
+                                let plugin_decl = PluginDeclaration::new(path.clone(), signature);
+
+                                working_set.add_decl(Box::new(plugin_decl));
+                            }
+
+                            working_set.mark_plugins_file_dirty();
+                        })
                         .err()
                 }
                 3 => {
                     let filename_slice = working_set.get_span_contents(spans[1]);
                     let signature = working_set.get_span_contents(spans[2]);
-                    let mut path = PathBuf::new();
 
                     String::from_utf8(filename_slice.to_vec())
                         .map_err(|_| ParseError::NonUtf8(spans[1]))
@@ -1148,18 +1166,23 @@ pub fn parse_plugin(
                             })
                         })
                         .and_then(|path_inner| {
-                            path = path_inner;
+                            serde_json::from_slice::<Signature>(signature)
-                            serde_json::from_slice::<Signature>(signature).map_err(|_| {
+                                .map_err(|_| {
-                                ParseError::LabeledError(
+                                    ParseError::LabeledError(
-                                    "Signature deserialization error".into(),
+                                        "Signature deserialization error".into(),
-                                    "unable to deserialize signature".into(),
+                                        "unable to deserialize signature".into(),
-                                    spans[0],
+                                        spans[0],
-                                )
+                                    )
-                            })
+                                })
+                                .map(|signature| (path_inner, signature))
                         })
-                        .and_then(|signature| {
+                        .and_then(|(path, signature)| {
                             if path.exists() & path.is_file() {
-                                working_set.add_plugin_signature(path, Some(signature));
+                                let plugin_decl = PluginDeclaration::new(path, signature);
+
+                                working_set.add_decl(Box::new(plugin_decl));
+
+                                working_set.mark_plugins_file_dirty();
                                 Ok(())
                             } else {
                                 Err(ParseError::FileNotFound(format!("{:?}", path)))

crates/nu-parser/src/parser.rs

@@ -23,7 +23,7 @@ use crate::parse_keywords::{
 use std::collections::HashSet;
 
 #[cfg(feature = "plugin")]
-use crate::parse_keywords::parse_plugin;
+use crate::parse_keywords::parse_register;
 
 #[derive(Debug, Clone)]
 pub enum Import {}
@@ -3226,7 +3226,7 @@ pub fn parse_statement(
         ),
         b"hide" => parse_hide(working_set, spans),
         #[cfg(feature = "plugin")]
-        b"register" => parse_plugin(working_set, spans),
+        b"register" => parse_register(working_set, spans),
         _ => {
             let (expr, err) = parse_expression(working_set, spans, true);
             (Statement::Pipeline(Pipeline::from_vec(vec![expr])), err)

crates/nu-plugin/src/plugin.rs

@@ -3,7 +3,7 @@ use std::io::BufReader;
 use std::path::{Path, PathBuf};
 use std::process::{Command as CommandSys, Stdio};
 
-use nu_protocol::engine::{Command, EngineState, Stack, StateWorkingSet};
+use nu_protocol::engine::{Command, EngineState, Stack};
 use nu_protocol::{ast::Call, Signature, Value};
 use nu_protocol::{PipelineData, ShellError};
 
@@ -268,37 +268,3 @@ pub fn serve_plugin(plugin: &mut impl Plugin) {
         }
     }
 }
-
-pub fn eval_plugin_signatures(working_set: &mut StateWorkingSet) -> Result<(), ShellError> {
-    let decls = working_set
-        .get_signatures()
-        .map(|(path, signature)| match signature {
-            Some(signature) => {
-                let plugin_decl = PluginDeclaration::new(path.clone(), signature.clone());
-                let plugin_decl: Box<dyn Command> = Box::new(plugin_decl);
-                Ok(vec![plugin_decl])
-            }
-            None => match get_signature(path.as_path()) {
-                Ok(signatures) => Ok(signatures
-                    .into_iter()
-                    .map(|signature| {
-                        let plugin_decl = PluginDeclaration::new(path.clone(), signature);
-                        let plugin_decl: Box<dyn Command> = Box::new(plugin_decl);
-                        plugin_decl
-                    })
-                    .collect::<Vec<Box<dyn Command>>>()),
-                Err(err) => Err(ShellError::PluginFailedToLoad(format!("{}", err))),
-            },
-        })
-        // Need to collect the vector in order to check the error from getting the signature
-        .collect::<Result<Vec<Vec<Box<dyn Command>>>, ShellError>>()?;
-
-    let decls = decls
-        .into_iter()
-        .flatten()
-        .collect::<Vec<Box<dyn Command>>>();
-
-    working_set.add_plugin_decls(decls);
-
-    Ok(())
-}

crates/nu-protocol/src/engine/command.rs

@@ -48,7 +48,7 @@ pub trait Command: Send + Sync + CommandClone {
         self.name().contains(' ')
     }
 
-    // Is a plugin command
+    // Is a plugin command (returns plugin's name if yes)
     fn is_plugin(&self) -> Option<&PathBuf> {
         None
     }

crates/nu-protocol/src/engine/engine_state.rs

@@ -11,6 +11,7 @@ use std::{
 
 #[cfg(feature = "plugin")]
 use std::path::PathBuf;
+
 // Tells whether a decl etc. is visible or not
 #[derive(Debug, Clone)]
 struct Visibility {
@@ -197,17 +198,14 @@ impl EngineState {
             last.visibility.merge_with(first.visibility);
 
             #[cfg(feature = "plugin")]
-            if !delta.plugin_decls.is_empty() {
+            if delta.plugins_changed {
-                for decl in delta.plugin_decls {
+                let result = self.update_plugin_file();
-                    let name = decl.name().as_bytes().to_vec();
-                    self.decls.push_back(decl);
-                    let decl_id = self.decls.len() - 1;
 
-                    last.decls.insert(name, decl_id);
+                if result.is_ok() {
-                    last.visibility.use_decl_id(&decl_id);
+                    delta.plugins_changed = false;
                 }
 
-                return self.update_plugin_file();
+                return result;
             }
         }
 
@@ -230,7 +228,7 @@ impl EngineState {
                     let file_name = path.to_str().expect("path should be a str");
 
                     let line = serde_json::to_string_pretty(&decl.signature())
-                        .map(|signature| format!("register {} {}\n", file_name, signature))
+                        .map(|signature| format!("register {} {}\n\n", file_name, signature))
                         .map_err(|err| ShellError::PluginFailedToLoad(err.to_string()))?;
 
                     plugin_file
@@ -250,6 +248,40 @@ impl EngineState {
         }
     }
 
+    #[cfg(feature = "plugin")]
+    pub fn update_plugin_file_1(&self) -> Result<(), ShellError> {
+        use std::io::Write;
+
+        // Updating the signatures plugin file with the added signatures
+        if let Some(plugin_path) = &self.plugin_signatures {
+            // Always creating the file which will erase previous signatures
+            let mut plugin_file = std::fs::File::create(plugin_path.as_path())
+                .map_err(|err| ShellError::PluginFailedToLoad(err.to_string()))?;
+
+            // Plugin definitions with parsed signature
+            for decl in self.plugin_decls() {
+                // A successful plugin registration already includes the plugin filename
+                // No need to check the None option
+                let path = decl.is_plugin().expect("plugin should have file name");
+                let file_name = path.to_str().expect("path should be a str");
+
+                let line = serde_json::to_string_pretty(&decl.signature())
+                    .map(|signature| format!("register {} {}\n\n", file_name, signature))
+                    .map_err(|err| ShellError::PluginFailedToLoad(err.to_string()))?;
+
+                plugin_file
+                    .write_all(line.as_bytes())
+                    .map_err(|err| ShellError::PluginFailedToLoad(err.to_string()))?;
+            }
+
+            Ok(())
+        } else {
+            Err(ShellError::PluginFailedToLoad(
+                "Plugin file not found".into(),
+            ))
+        }
+    }
+
     pub fn num_files(&self) -> usize {
         self.files.len()
     }
@@ -311,8 +343,21 @@ impl EngineState {
         None
     }
 
+    #[cfg(feature = "plugin")]
     pub fn plugin_decls(&self) -> impl Iterator<Item = &Box<dyn Command + 'static>> {
-        self.decls.iter().filter(|decl| decl.is_plugin().is_some())
+        let mut unique_plugin_decls = HashMap::new();
+
+        // Make sure there are no duplicate decls: Newer one overwrites the older one
+        for decl in self.decls.iter().filter(|d| d.is_plugin().is_some()) {
+            unique_plugin_decls.insert(decl.name(), decl);
+        }
+
+        let mut plugin_decls: Vec<(&str, &Box<dyn Command>)> =
+            unique_plugin_decls.into_iter().collect();
+
+        // Sort the plugins by name so we don't end up with a random plugin file each time
+        plugin_decls.sort_by(|a, b| a.0.cmp(b.0));
+        plugin_decls.into_iter().map(|(_, decl)| decl)
     }
 
     pub fn find_overlay(&self, name: &[u8]) -> Option<OverlayId> {
@@ -423,6 +468,8 @@ impl EngineState {
         }
 
         output.sort_by(|a, b| a.0.name.cmp(&b.0.name));
+        output.dedup_by(|a, b| a.0.name.cmp(&b.0.name).is_eq());
+
         output
     }
 
@@ -519,9 +566,7 @@ pub struct StateDelta {
     overlays: Vec<Overlay>,       // indexed by OverlayId
     pub scope: Vec<ScopeFrame>,
     #[cfg(feature = "plugin")]
-    pub plugin_signatures: Vec<(PathBuf, Option<Signature>)>,
+    plugins_changed: bool, // marks whether plugin file should be updated
-    #[cfg(feature = "plugin")]
-    plugin_decls: Vec<Box<dyn Command>>,
 }
 
 impl StateDelta {
@@ -562,9 +607,7 @@ impl<'a> StateWorkingSet<'a> {
                 overlays: vec![],
                 scope: vec![ScopeFrame::new()],
                 #[cfg(feature = "plugin")]
-                plugin_signatures: vec![],
+                plugins_changed: false,
-                #[cfg(feature = "plugin")]
-                plugin_decls: vec![],
             },
             permanent_state,
         }
@@ -633,20 +676,8 @@ impl<'a> StateWorkingSet<'a> {
     }
 
     #[cfg(feature = "plugin")]
-    pub fn add_plugin_decls(&mut self, decls: Vec<Box<dyn Command>>) {
+    pub fn mark_plugins_file_dirty(&mut self) {
-        for decl in decls {
+        self.delta.plugins_changed = true;
-            self.delta.plugin_decls.push(decl);
-        }
-    }
-
-    #[cfg(feature = "plugin")]
-    pub fn add_plugin_signature(&mut self, path: PathBuf, signature: Option<Signature>) {
-        self.delta.plugin_signatures.push((path, signature));
-    }
-
-    #[cfg(feature = "plugin")]
-    pub fn get_signatures(&self) -> impl Iterator<Item = &(PathBuf, Option<Signature>)> {
-        self.delta.plugin_signatures.iter()
     }
 
     pub fn merge_predecl(&mut self, name: &[u8]) -> Option<DeclId> {

src/main.rs

@@ -23,9 +23,6 @@ use nu_protocol::{
 };
 use reedline::{Completer, CompletionActionHandler, DefaultPrompt, LineBuffer, Prompt};
 
-#[cfg(feature = "plugin")]
-use nu_plugin::plugin::eval_plugin_signatures;
-
 #[cfg(test)]
 mod tests;
 
@@ -439,11 +436,6 @@ fn eval_source(
             return false;
         }
 
-        #[cfg(feature = "plugin")]
-        if let Err(err) = eval_plugin_signatures(&mut working_set) {
-            report_error(&working_set, &err);
-        }
-
         (output, working_set.render())
     };