From 3eb9c2a5651e2dc31cd32e997fee1a6c2f6ab7d3 Mon Sep 17 00:00:00 2001
From: Ian Manske <ian.manske@pm.me>
Date: Fri, 12 Apr 2024 15:44:00 +0000
Subject: [PATCH] `drop` refactor (#12479)

# Description
Refactors `drop` using `Vec::truncate` and adds a `NeedsPositiveValue`
error.

# User-Facing Changes
Breaking change: `drop` now errors if the number of rows/columns is
negative.
---
 crates/nu-command/src/filters/drop/drop_.rs | 33 ++++++++-------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/crates/nu-command/src/filters/drop/drop_.rs b/crates/nu-command/src/filters/drop/drop_.rs
index ea180d9816..9498cede71 100644
--- a/crates/nu-command/src/filters/drop/drop_.rs
+++ b/crates/nu-command/src/filters/drop/drop_.rs
@@ -62,8 +62,8 @@ impl Command for Drop {
                 description: "Remove the last row in a table",
                 example: "[[a, b]; [1, 2] [3, 4]] | drop 1",
                 result: Some(Value::test_list(vec![Value::test_record(record! {
-                    "a" =>  Value::test_int(1),
-                    "b" =>  Value::test_int(2),
+                    "a" => Value::test_int(1),
+                    "b" => Value::test_int(2),
                 })])),
             },
         ]
@@ -76,30 +76,23 @@ impl Command for Drop {
         call: &Call,
         input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
+        let head = call.head;
         let metadata = input.metadata();
-        let rows: Option<i64> = call.opt(engine_state, stack, 0)?;
-        let v: Vec<_> = input.into_iter_strict(call.head)?.collect();
-        let vlen: i64 = v.len() as i64;
+        let rows: Option<Spanned<i64>> = call.opt(engine_state, stack, 0)?;
+        let mut values = input.into_iter_strict(head)?.collect::<Vec<_>>();
 
-        let rows_to_drop = if let Some(quantity) = rows {
-            quantity
+        let rows_to_drop = if let Some(rows) = rows {
+            if rows.item < 0 {
+                return Err(ShellError::NeedsPositiveValue { span: rows.span });
+            } else {
+                rows.item as usize
+            }
         } else {
             1
         };
 
-        if rows_to_drop == 0 {
-            Ok(v.into_iter()
-                .into_pipeline_data_with_metadata(metadata, engine_state.ctrlc.clone()))
-        } else {
-            let k = if vlen < rows_to_drop {
-                0
-            } else {
-                vlen - rows_to_drop
-            };
-
-            let iter = v.into_iter().take(k as usize);
-            Ok(iter.into_pipeline_data_with_metadata(metadata, engine_state.ctrlc.clone()))
-        }
+        values.truncate(values.len().saturating_sub(rows_to_drop));
+        Ok(Value::list(values, head).into_pipeline_data_with_metadata(metadata))
     }
 }