From d8375ceaa7fce008ca3b5f88b0a19229d08ac02c Mon Sep 17 00:00:00 2001
From: Edward Langley <el-github@elangley.org>
Date: Wed, 15 Apr 2026 03:01:30 -0700
Subject: [PATCH] refactor(command): simplify commit_cell_value by extracting
 helper functions

Simplify the commit_cell_value function by extracting its core logic into
specialized helper functions: commit_regular_cell_value, stage_drill_edit,
and commit_plain_records_edit. This improves readability and reduces
nesting.

Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-31B-it-UD-Q4_K_XL.gguf)
---
 src/command/cmd/commit.rs | 132 ++++++++++++++++++++------------------
 1 file changed, 69 insertions(+), 63 deletions(-)

diff --git a/src/command/cmd/commit.rs b/src/command/cmd/commit.rs
index b969be8..7fe2ca2 100644
--- a/src/command/cmd/commit.rs
+++ b/src/command/cmd/commit.rs
@@ -139,6 +139,71 @@ mod tests {
 /// Commit a cell value: for synthetic records keys, stage in drill pending edits
 /// in drill mode, or apply directly in plain records mode; for real keys, write
 /// to the model.
+fn commit_regular_cell_value(key: &CellKey, value: &str, effects: &mut Vec<Box<dyn Effect>>) {
+    if value.is_empty() {
+        effects.push(Box::new(effect::ClearCell(key.clone())));
+    } else if let Ok(n) = value.parse::<f64>() {
+        effects.push(Box::new(effect::SetCell(key.clone(), CellValue::Number(n))));
+    } else {
+        effects.push(Box::new(effect::SetCell(
+            key.clone(),
+            CellValue::Text(value.to_string()),
+        )));
+    }
+    effects.push(effect::mark_dirty());
+}
+
+/// Stage a synthetic edit in drill state so it can be applied atomically on exit.
+fn stage_drill_edit(record_idx: usize, col_name: String, value: &str) -> Box<dyn Effect> {
+    Box::new(effect::SetDrillPendingEdit {
+        record_idx,
+        col_name,
+        new_value: value.to_string(),
+    })
+}
+
+/// Apply a synthetic records-mode edit directly to the underlying model cell.
+fn commit_plain_records_edit(
+    ctx: &CmdContext,
+    record_idx: usize,
+    col_name: &str,
+    value: &str,
+    effects: &mut Vec<Box<dyn Effect>>,
+) {
+    let Some((orig_key, _)) = ctx
+        .layout
+        .records
+        .as_ref()
+        .and_then(|records| records.get(record_idx))
+    else {
+        return;
+    };
+
+    if col_name == "Value" {
+        commit_regular_cell_value(orig_key, value, effects);
+        return;
+    }
+
+    if value.is_empty() {
+        effects.push(effect::set_status(effect::RECORD_COORDS_CANNOT_BE_EMPTY));
+        return;
+    }
+
+    let Some(existing_value) = ctx.model.get_cell(orig_key).cloned() else {
+        return;
+    };
+    effects.push(Box::new(effect::ClearCell(orig_key.clone())));
+    effects.push(Box::new(effect::AddItem {
+        category: col_name.to_string(),
+        item: value.to_string(),
+    }));
+    effects.push(Box::new(effect::SetCell(
+        orig_key.clone().with(col_name, value),
+        existing_value,
+    )));
+    effects.push(effect::mark_dirty());
+}
+
 fn commit_cell_value(
     ctx: &CmdContext,
     key: &CellKey,
@@ -147,73 +212,14 @@ fn commit_cell_value(
 ) {
     if let Some((record_idx, col_name)) = crate::view::synthetic_record_info(key) {
         if ctx.has_drill_state {
-            effects.push(Box::new(effect::SetDrillPendingEdit {
-                record_idx,
-                col_name,
-                new_value: value.to_string(),
-            }));
+            effects.push(stage_drill_edit(record_idx, col_name, value));
             return;
         }
-
-        let Some((orig_key, _)) = ctx
-            .layout
-            .records
-            .as_ref()
-            .and_then(|records| records.get(record_idx))
-        else {
-            return;
-        };
-
-        if col_name == "Value" {
-            if value.is_empty() {
-                effects.push(Box::new(effect::ClearCell(orig_key.clone())));
-            } else if let Ok(n) = value.parse::<f64>() {
-                effects.push(Box::new(effect::SetCell(
-                    orig_key.clone(),
-                    CellValue::Number(n),
-                )));
-            } else {
-                effects.push(Box::new(effect::SetCell(
-                    orig_key.clone(),
-                    CellValue::Text(value.to_string()),
-                )));
-            }
-            effects.push(effect::mark_dirty());
-            return;
-        }
-
-        if value.is_empty() {
-            effects.push(effect::set_status("Record coordinates cannot be empty"));
-            return;
-        }
-
-        let Some(existing_value) = ctx.model.get_cell(orig_key).cloned() else {
-            return;
-        };
-        effects.push(Box::new(effect::ClearCell(orig_key.clone())));
-        effects.push(Box::new(effect::AddItem {
-            category: col_name.clone(),
-            item: value.to_string(),
-        }));
-        effects.push(Box::new(effect::SetCell(
-            orig_key.clone().with(col_name, value),
-            existing_value,
-        )));
-        effects.push(effect::mark_dirty());
+        commit_plain_records_edit(ctx, record_idx, &col_name, value, effects);
         return;
-    } else if value.is_empty() {
-        effects.push(Box::new(effect::ClearCell(key.clone())));
-        effects.push(effect::mark_dirty());
-    } else if let Ok(n) = value.parse::<f64>() {
-        effects.push(Box::new(effect::SetCell(key.clone(), CellValue::Number(n))));
-        effects.push(effect::mark_dirty());
-    } else {
-        effects.push(Box::new(effect::SetCell(
-            key.clone(),
-            CellValue::Text(value.to_string()),
-        )));
-        effects.push(effect::mark_dirty());
     }
+
+    commit_regular_cell_value(key, value, effects);
 }
 
 /// Direction to advance after committing a cell edit.