From a9b93a4fdba5e9587e01e37bfb1f19e387303dda Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Sat, 30 Aug 2025 17:25:21 +0100 Subject: [PATCH] trim --- sql-cli/src/data/recursive_where_evaluator.rs | 81 ++++++++++ sql-cli/src/sql/cursor_aware_parser.rs | 4 +- sql-cli/src/ui/enhanced_tui.rs | 85 ++-------- sql-cli/tests/test_trim_methods.rs | 151 ++++++++++++++++++ 4 files changed, 250 insertions(+), 71 deletions(-) create mode 100644 sql-cli/tests/test_trim_methods.rs diff --git a/sql-cli/src/data/recursive_where_evaluator.rs b/sql-cli/src/data/recursive_where_evaluator.rs index bbee90d1..86b0c92c 100644 --- a/sql-cli/src/data/recursive_where_evaluator.rs +++ b/sql-cli/src/data/recursive_where_evaluator.rs @@ -246,6 +246,87 @@ impl<'a> RecursiveWhereEvaluator<'a> { } (index_value, format!("{}.IndexOf('{}')", object, search_str)) } + "trim" => { + if !args.is_empty() { + return Err(anyhow::anyhow!("Trim() takes no arguments")); + } + let col_index = self + .table + .get_column_index(object) + .ok_or_else(|| anyhow::anyhow!("Column '{}' not found", object))?; + + let value = self.table.get_value(row_index, col_index); + let trimmed_value = match value { + Some(DataValue::String(s)) => { + Some(DataValue::String(s.trim().to_string())) + } + Some(DataValue::InternedString(s)) => { + Some(DataValue::String(s.trim().to_string())) + } + Some(DataValue::Integer(n)) => { + Some(DataValue::String(n.to_string().trim().to_string())) + } + Some(DataValue::Float(f)) => { + Some(DataValue::String(f.to_string().trim().to_string())) + } + _ => Some(DataValue::String(String::new())), + }; + (trimmed_value, format!("{}.Trim()", object)) + } + "trimstart" => { + if !args.is_empty() { + return Err(anyhow::anyhow!("TrimStart() takes no arguments")); + } + let col_index = self + .table + .get_column_index(object) + .ok_or_else(|| anyhow::anyhow!("Column '{}' not found", object))?; + + let value = self.table.get_value(row_index, col_index); + let trimmed_value = match value { + Some(DataValue::String(s)) => { + Some(DataValue::String(s.trim_start().to_string())) + } + Some(DataValue::InternedString(s)) => { + Some(DataValue::String(s.trim_start().to_string())) + } + Some(DataValue::Integer(n)) => { + Some(DataValue::String(n.to_string().trim_start().to_string())) + } + Some(DataValue::Float(f)) => { + Some(DataValue::String(f.to_string().trim_start().to_string())) + } + _ => Some(DataValue::String(String::new())), + }; + (trimmed_value, format!("{}.TrimStart()", object)) + } + "trimend" => { + if !args.is_empty() { + return Err(anyhow::anyhow!("TrimEnd() takes no arguments")); + } + let col_index = self + .table + .get_column_index(object) + .ok_or_else(|| anyhow::anyhow!("Column '{}' not found", object))?; + + let value = self.table.get_value(row_index, col_index); + let trimmed_value = match value { + Some(DataValue::String(s)) => { + Some(DataValue::String(s.trim_end().to_string())) + } + Some(DataValue::InternedString(s)) => { + Some(DataValue::String(s.trim_end().to_string())) + } + Some(DataValue::Integer(n)) => { + Some(DataValue::String(n.to_string().trim_end().to_string())) + } + Some(DataValue::Float(f)) => { + Some(DataValue::String(f.to_string().trim_end().to_string())) + } + _ => Some(DataValue::String(String::new())), + }; + (trimmed_value, format!("{}.TrimEnd()", object)) + } _ => { return Err(anyhow::anyhow!( "Method '{}' cannot be used in comparisons", diff --git a/sql-cli/src/sql/cursor_aware_parser.rs b/sql-cli/src/sql/cursor_aware_parser.rs index 1e4afb0a..d33f9690 100644 --- a/sql-cli/src/sql/cursor_aware_parser.rs +++ b/sql-cli/src/sql/cursor_aware_parser.rs @@ -759,8 +759,10 @@ impl CursorAwareParser { "Substring(0, 5)", "ToLower()", "ToUpper()", - "IsNullOrEmpty()", "Trim()", + "TrimStart()", + "TrimEnd()", + "IsNullOrEmpty()", "Replace('', '')", "Length()", // Changed from "Length" to "Length()" ]; diff --git a/sql-cli/src/ui/enhanced_tui.rs b/sql-cli/src/ui/enhanced_tui.rs index 1e23f38d..bb182104 100644 --- a/sql-cli/src/ui/enhanced_tui.rs +++ b/sql-cli/src/ui/enhanced_tui.rs @@ -168,6 +168,18 @@ impl CommandEditor { self.delete_word_forward(); return Ok(false); } + 'v' | 'V' => { + // Paste from clipboard + // We need to handle this at the parent level since clipboard access + // requires state_container. Return a special indicator. + // For now, just don't handle it here so it falls through to parent + debug!("CommandEditor: Ctrl+V detected, letting parent handle clipboard paste"); + } + 'y' | 'Y' => { + // Yank (paste from kill ring) + // Similar to Ctrl+V, needs parent handling + debug!("CommandEditor: Ctrl+Y detected, letting parent handle yank"); + } _ => {} } } @@ -2653,21 +2665,6 @@ impl EnhancedTuiApp { self.move_cursor_word_forward(); return Ok(Some(false)); } - // TODO: NOT IN COMMAND_EDITOR - Keep for Phase 4 (SQL navigation) - "jump_to_prev_token" => { - self.jump_to_prev_token(); - return Ok(Some(false)); - } - // TODO: NOT IN COMMAND_EDITOR - Keep for Phase 4 (SQL navigation) - "jump_to_next_token" => { - self.jump_to_next_token(); - return Ok(Some(false)); - } - // TODO: NOT IN COMMAND_EDITOR - Keep for Phase 3 (clipboard operations) - "paste_from_clipboard" => { - self.paste_from_clipboard(); - return Ok(Some(false)); - } _ => {} // Not a text editing action, fall through } } @@ -2699,28 +2696,14 @@ impl EnhancedTuiApp { InputBehavior::kill_line_backward(self); Ok(Some(false)) } - // TODO: NOT IN COMMAND_EDITOR - Keep for Phase 3 (clipboard operations) - KeyCode::Char('y') if key.modifiers.contains(KeyModifiers::CONTROL) => { - // Yank - paste from kill ring - self.yank(); - Ok(Some(false)) - } - // TODO: NOT IN COMMAND_EDITOR - Keep for Phase 3 (clipboard operations) KeyCode::Char('v') if key.modifiers.contains(KeyModifiers::CONTROL) => { // Paste from system clipboard self.paste_from_clipboard(); Ok(Some(false)) } - // TODO: NOT IN COMMAND_EDITOR - Keep for Phase 4 (SQL navigation) - KeyCode::Char('[') if key.modifiers.contains(KeyModifiers::ALT) => { - // Jump to previous SQL token - self.jump_to_prev_token(); - Ok(Some(false)) - } - // TODO: NOT IN COMMAND_EDITOR - Keep for Phase 4 (SQL navigation) - KeyCode::Char(']') if key.modifiers.contains(KeyModifiers::ALT) => { - // Jump to next SQL token - self.jump_to_next_token(); + KeyCode::Char('y') if key.modifiers.contains(KeyModifiers::CONTROL) => { + // Yank - paste from kill ring (if we have one) + self.yank(); Ok(Some(false)) } // TODO: DUPLICATED IN COMMAND_EDITOR - Can be removed after full migration @@ -6919,44 +6902,6 @@ impl EnhancedTuiApp { debug_info } - fn debug_generate_key_renderer_info(&self) -> String { - let mut debug_info = String::new(); - debug_info.push_str("\n========== KEY SEQUENCE RENDERER ==========\n"); - debug_info.push_str(&format!( - "Enabled: {}\n", - self.key_sequence_renderer.is_enabled() - )); - debug_info.push_str(&format!( - "Has Content: {}\n", - self.key_sequence_renderer.has_content() - )); - debug_info.push_str(&format!( - "Display String: '{}'\n", - self.key_sequence_renderer.get_display() - )); - - // Show detailed state if enabled - if self.key_sequence_renderer.is_enabled() { - debug_info.push_str(&format!( - "Chord Mode: {:?}\n", - self.key_sequence_renderer.get_chord_mode() - )); - debug_info.push_str(&format!( - "Key History Size: {}\n", - self.key_sequence_renderer.sequence_count() - )); - let sequences = self.key_sequence_renderer.get_sequences(); - if !sequences.is_empty() { - debug_info.push_str("Recent Keys:\n"); - for (key, count) in sequences { - debug_info.push_str(&format!(" - '{}' ({} times)\n", key, count)); - } - } - } - debug_info.push_str("==========================================\n"); - debug_info - } - pub(crate) fn debug_generate_trace_logs(&self) -> String { let mut debug_info = String::from("\n========== TRACE LOGS ==========\n"); debug_info.push_str("(Most recent at bottom, last 100 entries)\n"); diff --git a/sql-cli/tests/test_trim_methods.rs b/sql-cli/tests/test_trim_methods.rs new file mode 100644 index 00000000..a675355a --- /dev/null +++ b/sql-cli/tests/test_trim_methods.rs @@ -0,0 +1,151 @@ +use sql_cli::data::recursive_where_evaluator::RecursiveWhereEvaluator; +use sql_cli::datatable::{DataColumn, DataRow, DataTable, DataValue}; +use sql_cli::recursive_parser::{Parser, WhereClause}; + +fn create_test_table() -> DataTable { + let mut table = DataTable::new("test"); + table.add_column(DataColumn::new("id")); + table.add_column(DataColumn::new("book")); + table.add_column(DataColumn::new("description")); + + // Add test data with various whitespace patterns + table + .add_row(DataRow::new(vec![ + DataValue::Integer(1), + DataValue::String(" derivatives ".to_string()), // spaces both sides + DataValue::String("with spaces".to_string()), + ])) + .unwrap(); + + table + .add_row(DataRow::new(vec![ + DataValue::Integer(2), + DataValue::String(" equity trading".to_string()), // leading spaces only + DataValue::String("no trim needed".to_string()), + ])) + .unwrap(); + + table + .add_row(DataRow::new(vec![ + DataValue::Integer(3), + DataValue::String("FX ".to_string()), // trailing spaces only + DataValue::String("clean".to_string()), + ])) + .unwrap(); + + table + .add_row(DataRow::new(vec![ + DataValue::Integer(4), + DataValue::String("bonds".to_string()), // no spaces + DataValue::String("already clean".to_string()), + ])) + .unwrap(); + + table + .add_row(DataRow::new(vec![ + DataValue::Integer(5), + DataValue::String(" ".to_string()), // only spaces + DataValue::String("empty after trim".to_string()), + ])) + .unwrap(); + + table +} + +#[test] +fn test_trim_method() { + let table = create_test_table(); + let evaluator = RecursiveWhereEvaluator::new(&table); + + // Test Trim() = 'derivatives' - should match row 0 after trimming + let mut parser = Parser::new("SELECT * FROM test WHERE book.Trim() = 'derivatives'"); + let statement = parser.parse().expect("Failed to parse"); + let where_clause = statement.where_clause.expect("Expected WHERE clause"); + + assert_eq!(evaluator.evaluate(&where_clause, 0).unwrap(), true); // " derivatives " -> "derivatives" + assert_eq!(evaluator.evaluate(&where_clause, 1).unwrap(), false); // " equity trading" -> "equity trading" + assert_eq!(evaluator.evaluate(&where_clause, 2).unwrap(), false); // "FX " -> "FX" + assert_eq!(evaluator.evaluate(&where_clause, 3).unwrap(), false); // "bonds" -> "bonds" + assert_eq!(evaluator.evaluate(&where_clause, 4).unwrap(), false); // " " -> "" +} + +#[test] +fn test_trimstart_method() { + let table = create_test_table(); + let evaluator = RecursiveWhereEvaluator::new(&table); + + // Test TrimStart() = 'equity trading' - should match row 1 after trimming start + let mut parser = Parser::new("SELECT * FROM test WHERE book.TrimStart() = 'equity trading'"); + let statement = parser.parse().expect("Failed to parse"); + let where_clause = statement.where_clause.expect("Expected WHERE clause"); + + assert_eq!(evaluator.evaluate(&where_clause, 0).unwrap(), false); // " derivatives " -> "derivatives " + assert_eq!(evaluator.evaluate(&where_clause, 1).unwrap(), true); // " equity trading" -> "equity trading" + assert_eq!(evaluator.evaluate(&where_clause, 2).unwrap(), false); // "FX " -> "FX " + assert_eq!(evaluator.evaluate(&where_clause, 3).unwrap(), false); // "bonds" -> "bonds" + assert_eq!(evaluator.evaluate(&where_clause, 4).unwrap(), false); // " " -> "" +} + +#[test] +fn test_trimend_method() { + let table = create_test_table(); + let evaluator = RecursiveWhereEvaluator::new(&table); + + // Test TrimEnd() = 'FX' - should match row 2 after trimming end + let mut parser = Parser::new("SELECT * FROM test WHERE book.TrimEnd() = 'FX'"); + let statement = parser.parse().expect("Failed to parse"); + let where_clause = statement.where_clause.expect("Expected WHERE clause"); + + assert_eq!(evaluator.evaluate(&where_clause, 0).unwrap(), false); // " derivatives " -> " derivatives" + assert_eq!(evaluator.evaluate(&where_clause, 1).unwrap(), false); // " equity trading" -> " equity trading" + assert_eq!(evaluator.evaluate(&where_clause, 2).unwrap(), true); // "FX " -> "FX" + assert_eq!(evaluator.evaluate(&where_clause, 3).unwrap(), false); // "bonds" -> "bonds" + assert_eq!(evaluator.evaluate(&where_clause, 4).unwrap(), false); // " " -> " " (TrimEnd leaves leading spaces) +} + +#[test] +fn test_trim_empty_string() { + let table = create_test_table(); + let evaluator = RecursiveWhereEvaluator::new(&table); + + // Test Trim() = '' - should match row 4 which has only spaces + let mut parser = Parser::new("SELECT * FROM test WHERE book.Trim() = ''"); + let statement = parser.parse().expect("Failed to parse"); + let where_clause = statement.where_clause.expect("Expected WHERE clause"); + + assert_eq!(evaluator.evaluate(&where_clause, 0).unwrap(), false); // " derivatives " -> "derivatives" + assert_eq!(evaluator.evaluate(&where_clause, 1).unwrap(), false); // " equity trading" -> "equity trading" + assert_eq!(evaluator.evaluate(&where_clause, 2).unwrap(), false); // "FX " -> "FX" + assert_eq!(evaluator.evaluate(&where_clause, 3).unwrap(), false); // "bonds" -> "bonds" + assert_eq!(evaluator.evaluate(&where_clause, 4).unwrap(), true); // " " -> "" +} + +#[test] +fn test_trim_with_startswith() { + let table = create_test_table(); + let evaluator = RecursiveWhereEvaluator::new(&table); + + // Test combining Trim() with StartsWith + let mut parser = Parser::new("SELECT * FROM test WHERE book.Trim().StartsWith('equity')"); + let statement = parser.parse().expect("Failed to parse"); + let where_clause = statement.where_clause.expect("Expected WHERE clause"); + + // This should fail for now as we don't support chained methods yet + // But it demonstrates the intention for future enhancement + + // For now, we'd need to use: book.Trim() = 'equity trading' AND book.StartsWith('equity') +} + +#[test] +fn test_trim_preserves_internal_spaces() { + let table = create_test_table(); + let evaluator = RecursiveWhereEvaluator::new(&table); + + // Verify that Trim only removes leading/trailing spaces, not internal ones + let mut parser = Parser::new("SELECT * FROM test WHERE book.TrimStart() = 'equity trading'"); + let statement = parser.parse().expect("Failed to parse"); + let where_clause = statement.where_clause.expect("Expected WHERE clause"); + + // " equity trading" should become "equity trading" (with space preserved between words) + assert_eq!(evaluator.evaluate(&where_clause, 1).unwrap(), true); +}