fix: split .id into .id, .id unpack and .id pack so each is typed exactly

Author: cablehead

src/nu/commands/scru128_command.rs

@@ -22,7 +22,7 @@ fn get_string_input(
     input: PipelineData,
     span: nu_protocol::Span,
 ) -> Result<String, ShellError> {
-    if let Some(id) = call.opt::<String>(engine_state, stack, 1)? {
+    if let Some(id) = call.opt::<String>(engine_state, stack, 0)? {
         Ok(id)
     } else {
         match input {
@@ -44,7 +44,7 @@ fn get_record_input(
     input: PipelineData,
     span: nu_protocol::Span,
 ) -> Result<Value, ShellError> {
-    if let Some(arg) = call.opt::<Value>(engine_state, stack, 1)? {
+    if let Some(arg) = call.opt::<Value>(engine_state, stack, 0)? {
         Ok(arg)
     } else {
         match input {
@@ -107,26 +107,67 @@ impl Command for Scru128Command {
 
     fn signature(&self) -> Signature {
         Signature::build(".id")
+            .input_output_types(vec![(Type::Nothing, Type::String)])
+            .category(Category::Experimental)
+    }
+
+    fn description(&self) -> &str {
+        "Generate a SCRU128 ID"
+    }
+
+    fn extra_description(&self) -> &str {
+        "Use `.id unpack` to read an id's components, and `.id pack` to build one from them."
+    }
+
+    fn run(
+        &self,
+        _engine_state: &EngineState,
+        _stack: &mut Stack,
+        call: &Call,
+        _input: PipelineData,
+    ) -> Result<PipelineData, ShellError> {
+        let span = call.head;
+        let result = crate::scru128::generate()
+            .map_err(|e| scru128_error(format!("Failed to generate ID: {e}"), span))?;
+        Ok(PipelineData::Value(Value::string(result, span), None))
+    }
+}
+
+#[derive(Clone, Default)]
+pub struct Scru128UnpackCommand;
+
+impl Scru128UnpackCommand {
+    pub fn new() -> Self {
+        Self
+    }
+}
+
+impl Command for Scru128UnpackCommand {
+    fn name(&self) -> &str {
+        ".id unpack"
+    }
+
+    fn signature(&self) -> Signature {
+        // Both input types produce a record, so nu knows the output whether
+        // the id arrives on the pipeline or as the argument. One command per
+        // subcommand is what makes that possible: a single `.id` taking the
+        // subcommand as a positional has one output type per input type, and
+        // no input has to cover generate, unpack and pack at once.
+        Signature::build(".id unpack")
             .input_output_types(vec![
-                (Type::Nothing, Type::String),
+                (Type::Nothing, Type::Record(vec![].into())),
                 (Type::String, Type::Record(vec![].into())),
-                (Type::Record(vec![].into()), Type::String),
             ])
             .optional(
-                "subcommand",
+                "id",
                 SyntaxShape::String,
-                "subcommand: 'unpack' or 'pack'",
-            )
-            .optional(
-                "input",
-                SyntaxShape::Any,
-                "input for subcommand (ID string for unpack, record for pack)",
+                "the id to unpack, if not given on the pipeline",
             )
             .category(Category::Experimental)
     }
 
     fn description(&self) -> &str {
-        "Generate SCRU128 IDs or manipulate them with unpack/pack operations"
+        "Unpack a SCRU128 ID into its components"
     }
 
     fn run(
@@ -137,45 +178,64 @@ impl Command for Scru128Command {
         input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
         let span = call.head;
+        let id_string = get_string_input(call, engine_state, stack, input, span)?;
+        let result = crate::scru128::unpack_to_json(&id_string)
+            .map_err(|e| scru128_error(format!("Failed to unpack ID: {e}"), span))?;
 
-        // Check if there's a subcommand
-        let subcommand: Option<String> = call.opt(engine_state, stack, 0)?;
+        let nu_value = util::json_to_value(&result, span);
+        let nu_value = add_when_field(nu_value, span);
 
-        match subcommand.as_deref() {
-            Some("unpack") => {
-                let id_string = get_string_input(call, engine_state, stack, input, span)?;
-                let result = crate::scru128::unpack_to_json(&id_string)
-                    .map_err(|e| scru128_error(format!("Failed to unpack ID: {e}"), span))?;
+        Ok(PipelineData::Value(nu_value, None))
+    }
+}
 
-                let nu_value = util::json_to_value(&result, span);
-                let nu_value = add_when_field(nu_value, span);
+#[derive(Clone, Default)]
+pub struct Scru128PackCommand;
 
-                Ok(PipelineData::Value(nu_value, None))
-            }
-            Some("pack") => {
-                let components = get_record_input(call, engine_state, stack, input, span)?;
-                let json_value = util::value_to_json(&components);
-                let json_value = drop_when_field(json_value);
+impl Scru128PackCommand {
+    pub fn new() -> Self {
+        Self
+    }
+}
 
-                let result = crate::scru128::pack_from_json(json_value)
-                    .map_err(|e| scru128_error(format!("Failed to pack components: {e}"), span))?;
+impl Command for Scru128PackCommand {
+    fn name(&self) -> &str {
+        ".id pack"
+    }
 
-                Ok(PipelineData::Value(Value::string(result, span), None))
-            }
-            Some(unknown) => Err(ShellError::Generic(
-                GenericError::new(
-                    "Invalid subcommand",
-                    format!("Unknown subcommand: {unknown}"),
-                    span,
-                )
-                .with_help("Available subcommands: unpack, pack"),
-            )),
-            None => {
-                let result = crate::scru128::generate()
-                    .map_err(|e| scru128_error(format!("Failed to generate ID: {e}"), span))?;
+    fn signature(&self) -> Signature {
+        Signature::build(".id pack")
+            .input_output_types(vec![
+                (Type::Nothing, Type::String),
+                (Type::Record(vec![].into()), Type::String),
+            ])
+            .optional(
+                "components",
+                SyntaxShape::Record(vec![].into()),
+                "the components to pack, if not given on the pipeline",
+            )
+            .category(Category::Experimental)
+    }
 
-                Ok(PipelineData::Value(Value::string(result, span), None))
-            }
-        }
+    fn description(&self) -> &str {
+        "Pack SCRU128 components back into an ID"
+    }
+
+    fn run(
+        &self,
+        engine_state: &EngineState,
+        stack: &mut Stack,
+        call: &Call,
+        input: PipelineData,
+    ) -> Result<PipelineData, ShellError> {
+        let span = call.head;
+        let components = get_record_input(call, engine_state, stack, input, span)?;
+        let json_value = util::value_to_json(&components);
+        let json_value = drop_when_field(json_value);
+
+        let result = crate::scru128::pack_from_json(json_value)
+            .map_err(|e| scru128_error(format!("Failed to pack components: {e}"), span))?;
+
+        Ok(PipelineData::Value(Value::string(result, span), None))
     }
 }

src/nu/engine.rs

@@ -432,6 +432,8 @@ pub fn add_core_commands(engine: &mut Engine, store: &Store) -> Result<(), Error
         Box::new(commands::get_command::GetCommand::new(store.clone())),
         Box::new(commands::remove_command::RemoveCommand::new(store.clone())),
         Box::new(commands::scru128_command::Scru128Command::new()),
+        Box::new(commands::scru128_command::Scru128UnpackCommand::new()),
+        Box::new(commands::scru128_command::Scru128PackCommand::new()),
     ])
 }
 

src/nu/test_commands.rs

@@ -48,9 +48,11 @@ mod tests {
     fn setup_scru128_test_env() -> Engine {
         let (_store, mut engine) = setup_test_env();
         engine
-            .add_commands(vec![Box::new(
-                commands::scru128_command::Scru128Command::new(),
-            )])
+            .add_commands(vec![
+                Box::new(commands::scru128_command::Scru128Command::new()),
+                Box::new(commands::scru128_command::Scru128UnpackCommand::new()),
+                Box::new(commands::scru128_command::Scru128PackCommand::new()),
+            ])
             .unwrap();
         engine
     }
@@ -627,6 +629,34 @@ mod tests {
         }
     }
 
+    /// `.id unpack $id` takes its id as an argument, so its pipeline input
+    /// is nothing. Nu picks an output type at parse time from the input type
+    /// alone, so the type declared for a nothing input decides what it allows
+    /// downstream. A wrong one rejects field access before the command runs.
+    #[test]
+    fn test_scru128_unpack_argument_form_parses_as_a_record() {
+        let engine = setup_scru128_test_env();
+        let id = nu_eval(&engine, PipelineData::empty(), ".id");
+        let id = id.as_str().unwrap();
+
+        // Binding the id to a variable first is what exposes it: with a
+        // literal, nu infers a type for the whole expression instead.
+        let ts = nu_eval(
+            &engine,
+            PipelineData::empty(),
+            format!("let id = \"{id}\"; .id unpack $id | get ts_ms"),
+        );
+        assert!(ts.as_int().is_ok(), "ts_ms was {ts:?}");
+
+        // pack goes the other way, and has the same shape.
+        let packed = nu_eval(
+            &engine,
+            PipelineData::empty(),
+            format!("let r = (\"{id}\" | .id unpack); .id pack $r"),
+        );
+        assert_eq!(packed.as_str().unwrap(), id);
+    }
+
     #[test]
     fn test_scru128_invalid_id() {
         let engine = setup_scru128_test_env();