From 0b83f32eda004a36606dc599b70fe285fc515445 Mon Sep 17 00:00:00 2001 From: Olivier Mansour Date: Sat, 12 Jul 2025 11:18:58 +0200 Subject: [PATCH] feat: add configurable command history truncation - Add `chat.historyMaxLength` setting (default: 80 chars) - Truncate history hints at word boundaries with "..." - Preserve full commands for input/navigation - Support unicode and edge cases - Add comprehensive test coverage Improves chat interface readability while maintaining full command functionality. --- crates/chat-cli/src/cli/chat/history_utils.rs | 749 ++++++++++++++++++ crates/chat-cli/src/cli/chat/input_source.rs | 4 +- crates/chat-cli/src/cli/chat/mod.rs | 1 + crates/chat-cli/src/cli/chat/prompt.rs | 378 ++++++++- crates/chat-cli/src/database/settings.rs | 51 +- 5 files changed, 1154 insertions(+), 29 deletions(-) create mode 100644 crates/chat-cli/src/cli/chat/history_utils.rs diff --git a/crates/chat-cli/src/cli/chat/history_utils.rs b/crates/chat-cli/src/cli/chat/history_utils.rs new file mode 100644 index 0000000000..fba2925c9b --- /dev/null +++ b/crates/chat-cli/src/cli/chat/history_utils.rs @@ -0,0 +1,749 @@ +use crate::database::{Database, settings::Setting}; + +/// Truncates a command for display purposes based on the ChatHistoryMaxLength setting. +/// +/// This function reads the current setting value from the database and applies truncation +/// logic accordingly. Commands are truncated at word boundaries to avoid cutting words in half. +/// If no word boundary is found, it falls back to character boundary truncation. +/// +/// # Arguments +/// * `command` - The command string to potentially truncate +/// * `database` - Database instance to read the setting from +/// +/// # Returns +/// * For setting > 0: Returns the command truncated at word boundaries with "..." appended if truncated +/// * For setting = 0: Returns empty string (history hidden mode) +/// * For invalid/missing setting: Uses default value of 80 characters +/// +/// # Examples +/// ```ignore +/// let db = Database::new().await.unwrap(); +/// let result = truncate_command_for_display("short command", &db); +/// // Returns "short command" if setting allows +/// +/// let long_cmd = "this is a very long command that exceeds the limit"; +/// let result = truncate_command_for_display(&long_cmd, &db); +/// // Returns "this is a very long command that exceeds..." (truncated at word boundary) +/// ``` +pub fn truncate_command_for_display(command: &str, database: &Database) -> String { + let max_length = database.settings.get_int(Setting::ChatHistoryMaxLength).unwrap_or(80); + + // Handle invalid values (negative) by using default + let max_length = if max_length < 0 { 80 } else { max_length }; + + if max_length == 0 { + // History hidden mode - return empty string for display + return String::new(); + } + + if command.len() <= max_length as usize { + // Command fits within limit + return command.to_string(); + } + + let limit = max_length as usize; + + // First, find a safe character boundary within the limit + let mut safe_limit = 0; + let mut char_count = 0; + + for (byte_index, _) in command.char_indices() { + if char_count >= limit { + break; + } + safe_limit = byte_index; + char_count += 1; + } + + // If we haven't reached the limit in characters, use the full string + if char_count < limit { + safe_limit = command.len(); + } + + // Now try to find the last word boundary (whitespace) before the safe limit + if let Some(last_space) = command[..safe_limit].rfind(char::is_whitespace) { + // Found a word boundary, truncate there + let truncated = command[..last_space].trim_end(); + if !truncated.is_empty() { + return format!("{}...", truncated); + } + } + + // No suitable word boundary found, or the result would be empty + // Fall back to character boundary truncation + let truncate_at = limit.saturating_sub(3); // Reserve space for "..." + let mut char_boundary = 0; + let mut final_char_count = 0; + + for (byte_index, _) in command.char_indices() { + if final_char_count >= truncate_at { + break; + } + char_boundary = byte_index; + final_char_count += 1; + } + + // If we haven't reached the truncation point, use the full string length + if final_char_count < truncate_at { + char_boundary = command.len(); + } + + format!("{}...", &command[..char_boundary]) +} + +/// Determines whether command history should be displayed based on the ChatHistoryMaxLength setting. +/// +/// This helper function checks if the setting allows history display. When the setting is 0, +/// history should be stored but not displayed to the user. +/// +/// # Arguments +/// * `database` - Database instance to read the setting from +/// +/// # Returns +/// * `true` if history should be displayed (setting > 0 or default) +/// * `false` if history should be hidden (setting = 0) +/// +/// # Examples +/// ```ignore +/// let db = Database::new().await.unwrap(); +/// if should_display_history(&db) { +/// // Show history entries to user +/// } else { +/// // Hide history from display but continue storing +/// } +/// ``` +pub fn should_display_history(database: &Database) -> bool { + let max_length = database.settings.get_int(Setting::ChatHistoryMaxLength).unwrap_or(80); + + // Handle invalid values (negative) by using default + let max_length = if max_length < 0 { 80 } else { max_length }; + + max_length != 0 +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::database::Database; + use crate::database::settings::Setting; + use tokio::time::{Duration, sleep}; + + #[tokio::test] + async fn test_truncate_command_for_display_default_setting() { + let db = Database::new().await.unwrap(); + + // Test short command (should not be truncated) + let short_cmd = "echo hello"; + let result = truncate_command_for_display(short_cmd, &db); + assert_eq!(result, "echo hello"); + + // Test command exactly at default limit (80 chars) + let exact_cmd = "a".repeat(80); + let result = truncate_command_for_display(&exact_cmd, &db); + assert_eq!(result, exact_cmd); + + // Test command over default limit with word boundaries + let long_cmd = + "this is a very long command that definitely exceeds the eighty character limit and should be truncated"; + let result = truncate_command_for_display(&long_cmd, &db); + // Should truncate at word boundary before 80 chars + assert!(result.ends_with("...")); + // The result should be shorter than the original and end with ... + assert!(result.len() < long_cmd.len()); + // Should truncate at a reasonable word boundary + assert!(result.starts_with("this is a very long command")); + } + + #[tokio::test] + async fn test_truncate_command_for_display_custom_setting() { + let mut db = Database::new().await.unwrap(); + + // Set custom length + db.settings.set(Setting::ChatHistoryMaxLength, 20).await.unwrap(); + + let long_cmd = "this is a very long command that exceeds 20 characters"; + let result = truncate_command_for_display(long_cmd, &db); + assert!(result.ends_with("...")); + // Should be truncated and shorter than original + assert!(result.len() < long_cmd.len()); + // Should truncate at word boundary + assert!(result.starts_with("this is")); + + // Test command exactly at custom limit + let exact_cmd = "a".repeat(20); + let result = truncate_command_for_display(&exact_cmd, &db); + assert_eq!(result, exact_cmd); + } + + #[tokio::test] + async fn test_truncate_command_for_display_zero_setting() { + let mut db = Database::new().await.unwrap(); + + // Set to zero (hidden mode) + db.settings.set(Setting::ChatHistoryMaxLength, 0).await.unwrap(); + + let cmd = "any command"; + let result = truncate_command_for_display(cmd, &db); + assert_eq!(result, ""); + } + + #[tokio::test] + async fn test_truncate_command_for_display_negative_setting() { + let mut db = Database::new().await.unwrap(); + + // Set negative value (should use default) + db.settings.set(Setting::ChatHistoryMaxLength, -10).await.unwrap(); + + let long_cmd = + "this is a very long command that definitely exceeds the eighty character limit and should be truncated"; + let result = truncate_command_for_display(&long_cmd, &db); + assert!(result.ends_with("...")); + // Should be truncated and shorter than original + assert!(result.len() < long_cmd.len()); + } + + #[tokio::test] + async fn test_truncate_command_for_display_word_boundaries() { + let mut db = Database::new().await.unwrap(); + + // Set a limit that will test word boundary logic + db.settings.set(Setting::ChatHistoryMaxLength, 25).await.unwrap(); + + // Test with clear word boundaries + let cmd = "short words here and more"; + let result = truncate_command_for_display(cmd, &db); + if result.len() < cmd.len() { + assert!(result.ends_with("...")); + // Should not cut words in half + assert!(result.starts_with("short words")); + } + + // Test with no spaces (should fall back to char boundary) + let no_spaces = "verylongcommandwithoutanyspaces"; + let result = truncate_command_for_display(no_spaces, &db); + assert!(result.ends_with("...")); + // Should be truncated to fit within limit + assert!(result.len() <= 28); // 25 + "..." = 28 + } + + #[tokio::test] + async fn test_truncate_command_for_display_unicode_safety() { + let mut db = Database::new().await.unwrap(); + + // Set a small limit to test unicode boundary handling + db.settings.set(Setting::ChatHistoryMaxLength, 15).await.unwrap(); + + // Test with unicode characters and word boundaries + let unicode_cmd = "héllo wörld 🌍 test"; + let result = truncate_command_for_display(unicode_cmd, &db); + assert!(result.ends_with("...")); + // Should truncate at word boundary, likely "héllo wörld..." + assert!(result.starts_with("héllo")); + + // Test with emoji at boundary - no spaces + let emoji_cmd = "test🌍🌍🌍🌍🌍🌍🌍🌍"; + let result = truncate_command_for_display(emoji_cmd, &db); + assert!(result.ends_with("...")); + // Should handle unicode properly in fallback mode + } + + #[tokio::test] + async fn test_truncate_command_for_display_edge_cases() { + let db = Database::new().await.unwrap(); + + // Test empty string + let result = truncate_command_for_display("", &db); + assert_eq!(result, ""); + + // Test whitespace only + let result = truncate_command_for_display(" ", &db); + assert_eq!(result, " "); + + // Test single character + let result = truncate_command_for_display("a", &db); + assert_eq!(result, "a"); + + // Test string that's all spaces up to limit + let mut db = Database::new().await.unwrap(); + db.settings.set(Setting::ChatHistoryMaxLength, 10).await.unwrap(); + let spaces = " ".repeat(15); + let result = truncate_command_for_display(&spaces, &db); + // Should handle gracefully, likely fall back to char truncation + if result.len() < spaces.len() { + assert!(result.ends_with("...")); + } + } + + #[tokio::test] + async fn test_should_display_history_default() { + let db = Database::new().await.unwrap(); + + // Default should be true (80 > 0) + assert!(should_display_history(&db)); + } + + #[tokio::test] + async fn test_should_display_history_custom_positive() { + let mut db = Database::new().await.unwrap(); + + db.settings.set(Setting::ChatHistoryMaxLength, 100).await.unwrap(); + assert!(should_display_history(&db)); + + db.settings.set(Setting::ChatHistoryMaxLength, 1).await.unwrap(); + assert!(should_display_history(&db)); + } + + #[tokio::test] + async fn test_should_display_history_zero() { + let mut db = Database::new().await.unwrap(); + + db.settings.set(Setting::ChatHistoryMaxLength, 0).await.unwrap(); + assert!(!should_display_history(&db)); + } + + #[tokio::test] + async fn test_should_display_history_negative() { + let mut db = Database::new().await.unwrap(); + + // Negative values should fall back to default (true) + db.settings.set(Setting::ChatHistoryMaxLength, -5).await.unwrap(); + assert!(should_display_history(&db)); + } + + #[tokio::test] + async fn test_truncate_command_consistency() { + let mut db = Database::new().await.unwrap(); + + // Test that both functions handle settings consistently + db.settings.set(Setting::ChatHistoryMaxLength, 0).await.unwrap(); + + assert!(!should_display_history(&db)); + assert_eq!(truncate_command_for_display("any command", &db), ""); + + db.settings.set(Setting::ChatHistoryMaxLength, 15).await.unwrap(); + + assert!(should_display_history(&db)); + let result = truncate_command_for_display("this is a long command", &db); + assert!(result.ends_with("...")); + assert!(result.len() < "this is a long command".len()); + } + + #[tokio::test] + async fn test_truncate_command_boundary_cases() { + let mut db = Database::new().await.unwrap(); + + // Test exactly at boundary + db.settings.set(Setting::ChatHistoryMaxLength, 10).await.unwrap(); + + let cmd_10 = "1234567890"; + let result = truncate_command_for_display(cmd_10, &db); + assert_eq!(result, "1234567890"); + + let cmd_11 = "12345678901"; + let result = truncate_command_for_display(cmd_11, &db); + assert!(result.ends_with("...")); + // Should be truncated to fit within the limit + assert!(result.len() <= 13); // 10 + "..." = 13 + + // Test with word boundary exactly at limit + let cmd_with_space = "hello worl"; // 10 chars + let result = truncate_command_for_display(cmd_with_space, &db); + assert_eq!(result, "hello worl"); + + let cmd_with_space_over = "hello world"; // 11 chars + let result = truncate_command_for_display(cmd_with_space_over, &db); + assert!(result.ends_with("...")); + // Should truncate at word boundary, likely "hello..." + assert!(result.starts_with("hello")); + } + + // Integration tests for comprehensive command history truncation feature + + #[tokio::test] + async fn test_integration_setting_changes_affect_display_immediately() { + // Create a database for testing + let mut db = Database::new().await.unwrap(); + + let long_command = "this is a very long command that definitely exceeds the default eighty character limit and should be truncated properly"; + + // Test with default setting (80) + let result = truncate_command_for_display(&long_command, &db); + assert!(result.ends_with("...")); + assert!(result.len() < long_command.len()); + assert!(should_display_history(&db)); + + // Change setting to 30 and verify immediate effect + db.settings.set(Setting::ChatHistoryMaxLength, 30).await.unwrap(); + let result_30 = truncate_command_for_display(&long_command, &db); + assert!(result_30.ends_with("...")); + assert!(result_30.len() < result.len()); // Should be shorter than 80-char version + assert!(should_display_history(&db)); + + // Change setting to 100 and verify immediate effect + db.settings.set(Setting::ChatHistoryMaxLength, 100).await.unwrap(); + let result_100 = truncate_command_for_display(&long_command, &db); + assert!(result_100.ends_with("...")); + assert!(result_100.len() > result.len()); // Should be longer than 80-char version + assert!(should_display_history(&db)); + + // Change setting to 0 (hidden) and verify immediate effect + db.settings.set(Setting::ChatHistoryMaxLength, 0).await.unwrap(); + let result_hidden = truncate_command_for_display(&long_command, &db); + assert_eq!(result_hidden, ""); + assert!(!should_display_history(&db)); + + // Change back to positive value and verify it works again + db.settings.set(Setting::ChatHistoryMaxLength, 40).await.unwrap(); + let result_40 = truncate_command_for_display(&long_command, &db); + assert!(result_40.ends_with("...")); + assert!(!result_40.is_empty()); + assert!(should_display_history(&db)); + } + + #[tokio::test] + async fn test_integration_history_storage_vs_display_with_zero_setting() { + let mut db = Database::new().await.unwrap(); + + // Set history display to hidden (0) + db.settings.set(Setting::ChatHistoryMaxLength, 0).await.unwrap(); + + let commands = vec![ + "echo hello world", + "ls -la /some/very/long/path/that/exceeds/normal/limits", + "git commit -m 'this is a very long commit message that describes many changes'", + ]; + + // Verify that history display is disabled + assert!(!should_display_history(&db)); + + // Verify that truncation returns empty string for display + for cmd in &commands { + let display_result = truncate_command_for_display(cmd, &db); + assert_eq!(display_result, ""); + } + + // Now enable history display and verify commands can be displayed (truncated) + db.settings.set(Setting::ChatHistoryMaxLength, 50).await.unwrap(); + + assert!(should_display_history(&db)); + + // Verify that commands can now be displayed (truncated) + for cmd in &commands { + let display_result = truncate_command_for_display(cmd, &db); + if cmd.len() > 50 { + assert!(display_result.ends_with("...")); + assert!(display_result.len() <= 53); // 50 + "..." = 53 + } else { + assert_eq!(display_result, *cmd); + } + } + } + + #[tokio::test] + async fn test_integration_invalid_setting_values_fallback_to_default() { + let mut db = Database::new().await.unwrap(); + + let test_command = + "this is a test command that is longer than eighty characters and should be truncated properly"; + + // Test negative values fall back to default (80) + let negative_values = vec![-1, -10, -100, -999]; + for negative_val in negative_values { + db.settings + .set(Setting::ChatHistoryMaxLength, negative_val) + .await + .unwrap(); + + let result = truncate_command_for_display(&test_command, &db); + assert!(result.ends_with("...")); + // Should behave like default (80) + assert!(result.len() <= 83); // 80 + "..." = 83 + assert!(should_display_history(&db)); // Should display with default behavior + } + + // Test very large values work correctly + db.settings.set(Setting::ChatHistoryMaxLength, 1000).await.unwrap(); + let result_large = truncate_command_for_display(&test_command, &db); + assert_eq!(result_large, test_command); // Should not be truncated + assert!(should_display_history(&db)); + + // Test boundary value (1) + db.settings.set(Setting::ChatHistoryMaxLength, 1).await.unwrap(); + let result_one = truncate_command_for_display(&test_command, &db); + assert!(result_one.ends_with("...")); + assert!(result_one.len() <= 4); // 1 + "..." = 4 + assert!(should_display_history(&db)); + } + + #[tokio::test] + async fn test_integration_unicode_character_handling_in_truncation() { + let mut db = Database::new().await.unwrap(); + + // Set a small limit to test unicode boundary handling + db.settings.set(Setting::ChatHistoryMaxLength, 20).await.unwrap(); + + // Test various unicode scenarios + let unicode_tests = vec![ + // Basic unicode characters + ("héllo wörld test command", "héllo wörld"), + // Emoji characters + ("test 🌍🌎🌏 command", "test 🌍🌎🌏"), + // Mixed unicode and ASCII + ("café résumé naïve command", "café résumé"), + // Multi-byte characters + ("こんにちは world test", "こんにちは world"), + // Complex emoji sequences + ("test 👨‍👩‍👧‍👦 family emoji", "test 👨‍👩‍👧‍👦"), + ]; + + for (input, expected_start) in unicode_tests { + let result = truncate_command_for_display(input, &db); + + if input.len() > 20 { + assert!(result.ends_with("...")); + // Verify no unicode characters are broken + assert!(result.is_char_boundary(result.len() - 3)); // Before "..." + // Verify it starts with expected content + assert!(result.starts_with(expected_start) || result.contains(expected_start)); + } else { + assert_eq!(result, input); + } + + // Verify the result is valid UTF-8 + assert!(std::str::from_utf8(result.as_bytes()).is_ok()); + } + + // Test edge case: command that's exactly at unicode boundary + let boundary_test = "test🌍"; // 7 bytes but 5 characters + db.settings.set(Setting::ChatHistoryMaxLength, 5).await.unwrap(); + let result = truncate_command_for_display(&boundary_test, &db); + // The emoji might be counted differently, so just verify it handles unicode safely + assert!(result == boundary_test || result.ends_with("...")); + + db.settings.set(Setting::ChatHistoryMaxLength, 4).await.unwrap(); + let result = truncate_command_for_display(&boundary_test, &db); + assert!(result.ends_with("...")); + // With very small limits, the result might be just "..." or have minimal content + assert!(result.len() <= 7); // Should be reasonable length + } + + #[tokio::test] + async fn test_integration_very_long_commands_and_edge_cases() { + let mut db = Database::new().await.unwrap(); + + // Test extremely long command + let very_long_command = "a".repeat(10000); + db.settings.set(Setting::ChatHistoryMaxLength, 50).await.unwrap(); + + let result = truncate_command_for_display(&very_long_command, &db); + assert!(result.ends_with("...")); + assert!(result.len() <= 53); // 50 + "..." = 53 + + // Test command with only spaces + let spaces_command = " ".repeat(100); + let result = truncate_command_for_display(&spaces_command, &db); + if spaces_command.len() > 50 { + assert!(result.ends_with("...")); + } + + // Test empty command + let result = truncate_command_for_display("", &db); + assert_eq!(result, ""); + + // Test single character + let result = truncate_command_for_display("a", &db); + assert_eq!(result, "a"); + + // Test command with newlines and special characters + let special_command = "echo 'hello\nworld'\t&& ls -la | grep test"; + let result = truncate_command_for_display(&special_command, &db); + if special_command.len() > 50 { + assert!(result.ends_with("...")); + assert!(result.len() <= 53); + } else { + assert_eq!(result, special_command); + } + + // Test word boundary truncation with very long words + let no_spaces = "verylongcommandwithoutanyspacesorwordbreaksthatexceedsthelimit"; + let result = truncate_command_for_display(&no_spaces, &db); + assert!(result.ends_with("...")); + assert!(result.len() <= 53); + + // Test command that's exactly at the limit + let exact_limit = "a".repeat(50); + let result = truncate_command_for_display(&exact_limit, &db); + assert_eq!(result, exact_limit); // Should not be truncated + + // Test command that's one character over the limit + let one_over = "a".repeat(51); + let result = truncate_command_for_display(&one_over, &db); + assert!(result.ends_with("...")); + assert!(result.len() <= 53); + } + + #[tokio::test] + async fn test_integration_concurrent_setting_changes() { + let mut db = Database::new().await.unwrap(); + + let test_command = "this is a test command for concurrent access testing"; + + // Test rapid setting changes + let settings_values = vec![10, 20, 30, 40, 50, 0, 80, 100]; + + for &value in &settings_values { + db.settings.set(Setting::ChatHistoryMaxLength, value).await.unwrap(); + + let result = truncate_command_for_display(&test_command, &db); + let should_display = should_display_history(&db); + + if value == 0 { + assert_eq!(result, ""); + assert!(!should_display); + } else { + assert!(should_display); + if test_command.len() > value as usize { + assert!(result.ends_with("...")); + assert!(result.len() <= (value as usize + 3)); + } else { + assert_eq!(result, test_command); + } + } + + // Small delay to simulate real usage + sleep(Duration::from_millis(1)).await; + } + } + + #[tokio::test] + async fn test_integration_word_boundary_truncation_edge_cases() { + let mut db = Database::new().await.unwrap(); + + db.settings.set(Setting::ChatHistoryMaxLength, 20).await.unwrap(); + + // Test various word boundary scenarios + let test_cases = vec![ + // Command with space exactly at limit + ("hello world test cmd", "hello world"), + // Command with no spaces (should fall back to char boundary) + ("verylongcommandwithoutspaces", "verylongcommandwith"), + // Command with multiple consecutive spaces + ("hello world test", "hello world"), + // Command starting with spaces + (" hello world test", " hello world"), + // Command ending with spaces before limit + ("hello world test more", "hello world"), + // Single word longer than limit + ("supercalifragilisticexpialidocious", "supercalifragilis"), + ]; + + for (input, expected_prefix) in test_cases { + let result = truncate_command_for_display(input, &db); + + if input.len() > 20 { + assert!(result.ends_with("...")); + assert!(result.len() <= 23); // 20 + "..." = 23 + + // For word boundary cases, check if it starts with expected prefix + if input.contains(' ') { + // Should truncate at word boundary + let without_ellipsis = &result[..result.len() - 3]; + assert!( + expected_prefix.starts_with(without_ellipsis) || without_ellipsis.starts_with(expected_prefix) + ); + } + } else { + assert_eq!(result, input); + } + } + } + + #[tokio::test] + async fn test_integration_performance_with_large_operations() { + let mut db = Database::new().await.unwrap(); + + db.settings.set(Setting::ChatHistoryMaxLength, 50).await.unwrap(); + + // Test that truncation works efficiently with many operations + let test_command = "this is a test command that should be truncated efficiently"; + + let start = std::time::Instant::now(); + for _ in 0..1000 { + let _result = truncate_command_for_display(&test_command, &db); + } + let duration = start.elapsed(); + + // Should complete quickly (less than 100ms for 1000 operations) + assert!(duration.as_millis() < 100, "Truncation took too long: {:?}", duration); + + // Verify the result is still correct + let result = truncate_command_for_display(&test_command, &db); + assert!(result.ends_with("...")); + assert!(result.len() <= 53); // 50 + "..." = 53 + } + + #[tokio::test] + async fn test_integration_database_settings_behavior() { + // Test that settings work correctly within a single database instance + let mut db = Database::new().await.unwrap(); + + // Set a custom value and test + db.settings.set(Setting::ChatHistoryMaxLength, 25).await.unwrap(); + + let test_command = "this is a test command for settings behavior"; + let result = truncate_command_for_display(&test_command, &db); + assert!(result.ends_with("...")); + assert!(result.len() <= 28); // 25 + "..." = 28 + assert!(should_display_history(&db)); + + // Change to zero setting and test + db.settings.set(Setting::ChatHistoryMaxLength, 0).await.unwrap(); + + let result = truncate_command_for_display(&test_command, &db); + assert_eq!(result, ""); + assert!(!should_display_history(&db)); + + // Change back to positive value and test + db.settings.set(Setting::ChatHistoryMaxLength, 35).await.unwrap(); + + let result = truncate_command_for_display(&test_command, &db); + assert!(result.ends_with("...")); + assert!(result.len() <= 38); // 35 + "..." = 38 + assert!(should_display_history(&db)); + } + + #[tokio::test] + async fn test_integration_function_consistency() { + let mut db = Database::new().await.unwrap(); + + // Test that both functions handle settings consistently across different values + let test_values = vec![0i32, 1, 10, 25, 50, 80, 100, -5, -100]; + let test_command = "this is a test command for consistency checking"; + + for &value in &test_values { + db.settings.set(Setting::ChatHistoryMaxLength, value).await.unwrap(); + + let should_display = should_display_history(&db); + let truncated = truncate_command_for_display(&test_command, &db); + + // Consistency check: if should_display is false, truncated should be empty + if !should_display { + assert_eq!(truncated, ""); + assert_eq!(value, 0); // Only zero should disable display + } else { + // If display is enabled, truncated should not be empty for non-empty input + assert!(!truncated.is_empty()); + + // Effective value should be positive (negative values use default) + let effective_value = if value < 0 { 80 } else { value }; + + if test_command.len() > effective_value as usize { + assert!(truncated.ends_with("...")); + assert!(truncated.len() <= (effective_value as usize + 3)); + } else { + assert_eq!(truncated, test_command); + } + } + } + } +} diff --git a/crates/chat-cli/src/cli/chat/input_source.rs b/crates/chat-cli/src/cli/chat/input_source.rs index 028b2e2889..7ba5bdd45a 100644 --- a/crates/chat-cli/src/cli/chat/input_source.rs +++ b/crates/chat-cli/src/cli/chat/input_source.rs @@ -11,14 +11,14 @@ pub struct InputSource(inner::Inner); mod inner { use rustyline::Editor; - use rustyline::history::FileHistory; + use rustyline::history::DefaultHistory; use super::super::prompt::ChatHelper; #[allow(clippy::large_enum_variant)] #[derive(Debug)] pub enum Inner { - Readline(Editor), + Readline(Editor), #[allow(dead_code)] Mock { index: usize, diff --git a/crates/chat-cli/src/cli/chat/mod.rs b/crates/chat-cli/src/cli/chat/mod.rs index 2c60d5a5fb..0eced14159 100644 --- a/crates/chat-cli/src/cli/chat/mod.rs +++ b/crates/chat-cli/src/cli/chat/mod.rs @@ -3,6 +3,7 @@ mod consts; pub mod context; mod conversation; mod error_formatter; +mod history_utils; mod input_source; mod message; mod parse; diff --git a/crates/chat-cli/src/cli/chat/prompt.rs b/crates/chat-cli/src/cli/chat/prompt.rs index 18f8743144..79ef162739 100644 --- a/crates/chat-cli/src/cli/chat/prompt.rs +++ b/crates/chat-cli/src/cli/chat/prompt.rs @@ -37,7 +37,8 @@ use winnow::stream::AsChar; pub use super::prompt_parser::generate_prompt; use super::prompt_parser::parse_prompt_components; -use crate::database::settings::Setting; +use super::history_utils::{truncate_command_for_display, should_display_history}; +use crate::database::{Database, settings::Setting}; use crate::os::Os; pub const COMMANDS: &[&str] = &[ @@ -215,12 +216,17 @@ impl Completer for ChatCompleter { pub struct ChatHinter { /// Command history for providing suggestions based on past commands history: Vec, + /// Database reference for accessing settings + database: Database, } impl ChatHinter { /// Creates a new ChatHinter instance - pub fn new() -> Self { - Self { history: Vec::new() } + pub fn new(database: Database) -> Self { + Self { + history: Vec::new(), + database, + } } /// Updates the history with a new command @@ -237,6 +243,18 @@ impl ChatHinter { return None; } + // Check if history should be displayed at all + if !should_display_history(&self.database) { + // Only provide command hints when history is disabled + if line.starts_with('/') { + return COMMANDS + .iter() + .find(|cmd| cmd.starts_with(line)) + .map(|cmd| cmd[line.len()..].to_string()); + } + return None; + } + // If line starts with a slash, try to find a command hint if line.starts_with('/') { return COMMANDS @@ -250,7 +268,37 @@ impl ChatHinter { .iter() .rev() // Start from most recent .find(|cmd| cmd.starts_with(line) && cmd.len() > line.len()) - .map(|cmd| cmd[line.len()..].to_string()) + .map(|cmd| { + // Get the full command that matches + let full_command = cmd; + // Get the remaining part after the current input + let remaining_part = &cmd[line.len()..]; + + // Truncate the full command for display purposes + let truncated_display = truncate_command_for_display(full_command, &self.database); + + // If the truncated version is different from the original, we need to adjust the hint + if truncated_display.len() < full_command.len() && truncated_display.ends_with("...") { + // The command was truncated, so we need to calculate what part of the hint to show + let truncated_without_ellipsis = &truncated_display[..truncated_display.len() - 3]; + + if line.len() >= truncated_without_ellipsis.len() { + // The current input is already longer than what we can display + return String::new(); + } + + // Show the remaining part up to the truncation point, then add "..." + let remaining_in_truncated = &truncated_without_ellipsis[line.len()..]; + if remaining_in_truncated.is_empty() { + "...".to_string() + } else { + format!("{}...", remaining_in_truncated) + } + } else { + // Command wasn't truncated, show the normal remaining part + remaining_part.to_string() + } + }) } } @@ -316,6 +364,8 @@ impl Highlighter for ChatHelper { } fn highlight<'l>(&self, line: &'l str, _pos: usize) -> Cow<'l, str> { + // Don't truncate the actual input line - user should see their full input + // Truncation only applies to hints, not to the current input Cow::Borrowed(line) } @@ -368,7 +418,7 @@ pub fn rl( .build(); let h = ChatHelper { completer: ChatCompleter::new(sender, receiver), - hinter: ChatHinter::new(), + hinter: ChatHinter::new(os.database.clone()), validator: MultiLineValidator, }; let mut rl = Editor::with_config(config)?; @@ -401,6 +451,8 @@ mod tests { use rustyline::highlight::Highlighter; use super::*; + use crate::database::Database; + #[test] fn test_chat_completer_command_completion() { let (prompt_request_sender, _) = std::sync::mpsc::channel::>(); @@ -442,13 +494,14 @@ mod tests { assert!(completions.is_empty()); } - #[test] - fn test_highlight_prompt_basic() { + #[tokio::test] + async fn test_highlight_prompt_basic() { + let db = Database::new().await.unwrap(); let (prompt_request_sender, _) = std::sync::mpsc::channel::>(); let (_, prompt_response_receiver) = std::sync::mpsc::channel::>(); let helper = ChatHelper { completer: ChatCompleter::new(prompt_request_sender, prompt_response_receiver), - hinter: ChatHinter::new(), + hinter: ChatHinter::new(db), validator: MultiLineValidator, }; @@ -458,13 +511,14 @@ mod tests { assert_eq!(highlighted, "> ".magenta().to_string()); } - #[test] - fn test_highlight_prompt_with_warning() { + #[tokio::test] + async fn test_highlight_prompt_with_warning() { + let db = Database::new().await.unwrap(); let (prompt_request_sender, _) = std::sync::mpsc::channel::>(); let (_, prompt_response_receiver) = std::sync::mpsc::channel::>(); let helper = ChatHelper { completer: ChatCompleter::new(prompt_request_sender, prompt_response_receiver), - hinter: ChatHinter::new(), + hinter: ChatHinter::new(db), validator: MultiLineValidator, }; @@ -474,13 +528,14 @@ mod tests { assert_eq!(highlighted, format!("{}{}", "!".red(), "> ".magenta())); } - #[test] - fn test_highlight_prompt_with_profile() { + #[tokio::test] + async fn test_highlight_prompt_with_profile() { + let db = Database::new().await.unwrap(); let (prompt_request_sender, _) = std::sync::mpsc::channel::>(); let (_, prompt_response_receiver) = std::sync::mpsc::channel::>(); let helper = ChatHelper { completer: ChatCompleter::new(prompt_request_sender, prompt_response_receiver), - hinter: ChatHinter::new(), + hinter: ChatHinter::new(db), validator: MultiLineValidator, }; @@ -490,13 +545,14 @@ mod tests { assert_eq!(highlighted, format!("{}{}", "[test-profile] ".cyan(), "> ".magenta())); } - #[test] - fn test_highlight_prompt_with_profile_and_warning() { + #[tokio::test] + async fn test_highlight_prompt_with_profile_and_warning() { + let db = Database::new().await.unwrap(); let (prompt_request_sender, _) = std::sync::mpsc::channel::>(); let (_, prompt_response_receiver) = std::sync::mpsc::channel::>(); let helper = ChatHelper { completer: ChatCompleter::new(prompt_request_sender, prompt_response_receiver), - hinter: ChatHinter::new(), + hinter: ChatHinter::new(db), validator: MultiLineValidator, }; @@ -509,13 +565,14 @@ mod tests { ); } - #[test] - fn test_highlight_prompt_invalid_format() { + #[tokio::test] + async fn test_highlight_prompt_invalid_format() { + let db = Database::new().await.unwrap(); let (prompt_request_sender, _) = std::sync::mpsc::channel::>(); let (_, prompt_response_receiver) = std::sync::mpsc::channel::>(); let helper = ChatHelper { completer: ChatCompleter::new(prompt_request_sender, prompt_response_receiver), - hinter: ChatHinter::new(), + hinter: ChatHinter::new(db), validator: MultiLineValidator, }; @@ -525,9 +582,10 @@ mod tests { assert_eq!(highlighted, invalid_prompt); } - #[test] - fn test_chat_hinter_command_hint() { - let hinter = ChatHinter::new(); + #[tokio::test] + async fn test_chat_hinter_command_hint() { + let db = Database::new().await.unwrap(); + let hinter = ChatHinter::new(db); // Test hint for a command let line = "/he"; @@ -549,9 +607,10 @@ mod tests { assert_eq!(hint, None); } - #[test] - fn test_chat_hinter_history_hint() { - let mut hinter = ChatHinter::new(); + #[tokio::test] + async fn test_chat_hinter_history_hint() { + let db = Database::new().await.unwrap(); + let mut hinter = ChatHinter::new(db); // Add some history hinter.update_history("Hello, world!"); @@ -566,4 +625,271 @@ mod tests { let hint = hinter.hint(line, pos, &ctx); assert_eq!(hint, Some(" are you?".to_string())); } + + #[tokio::test] + async fn test_chat_hinter_truncation_basic() { + let mut db = Database::new().await.unwrap(); + // Set a small truncation limit for testing + db.settings.set(Setting::ChatHistoryMaxLength, 20).await.unwrap(); + + let mut hinter = ChatHinter::new(db); + + // Add a long command to history + let long_command = "this is a very long command that exceeds the limit"; + hinter.update_history(long_command); + + // Test hint for partial match + let line = "this"; + let pos = line.len(); + let empty_history = DefaultHistory::new(); + let ctx = Context::new(&empty_history); + + let hint = hinter.hint(line, pos, &ctx); + // Should provide a truncated hint + assert!(hint.is_some()); + let hint_text = hint.unwrap(); + // The hint should end with "..." indicating truncation + assert!(hint_text.ends_with("...")); + } + + #[tokio::test] + async fn test_chat_hinter_truncation_no_truncation_needed() { + let mut db = Database::new().await.unwrap(); + // Set a large truncation limit + db.settings.set(Setting::ChatHistoryMaxLength, 100).await.unwrap(); + + let mut hinter = ChatHinter::new(db); + + // Add a short command to history + let short_command = "echo hello world"; + hinter.update_history(short_command); + + // Test hint for partial match + let line = "echo"; + let pos = line.len(); + let empty_history = DefaultHistory::new(); + let ctx = Context::new(&empty_history); + + let hint = hinter.hint(line, pos, &ctx); + // Should provide the full remaining part without truncation + assert_eq!(hint, Some(" hello world".to_string())); + } + + #[tokio::test] + async fn test_chat_hinter_history_hidden() { + let mut db = Database::new().await.unwrap(); + // Set history to hidden (0) + db.settings.set(Setting::ChatHistoryMaxLength, 0).await.unwrap(); + + let mut hinter = ChatHinter::new(db); + + // Add commands to history + hinter.update_history("some command in history"); + + // Test that history hints are not provided + let line = "some"; + let pos = line.len(); + let empty_history = DefaultHistory::new(); + let ctx = Context::new(&empty_history); + + let hint = hinter.hint(line, pos, &ctx); + // Should not provide history hints when history is hidden + assert_eq!(hint, None); + + // But command hints should still work + let line = "/he"; + let pos = line.len(); + let hint = hinter.hint(line, pos, &ctx); + assert_eq!(hint, Some("lp".to_string())); + } + + #[tokio::test] + async fn test_chat_hinter_truncation_edge_cases() { + let mut db = Database::new().await.unwrap(); + // Set a very small truncation limit + db.settings.set(Setting::ChatHistoryMaxLength, 5).await.unwrap(); + + let mut hinter = ChatHinter::new(db); + + // Add a command that will be heavily truncated + let command = "very long command"; + hinter.update_history(command); + + // Test hint when input is already longer than truncation limit + let line = "very long"; + let pos = line.len(); + let empty_history = DefaultHistory::new(); + let ctx = Context::new(&empty_history); + + let hint = hinter.hint(line, pos, &ctx); + // Should handle gracefully when input exceeds truncation limit + assert!(hint.is_some()); + } + + #[tokio::test] + async fn test_chat_hinter_truncation_with_unicode() { + let mut db = Database::new().await.unwrap(); + // Set truncation limit + db.settings.set(Setting::ChatHistoryMaxLength, 15).await.unwrap(); + + let mut hinter = ChatHinter::new(db); + + // Add a command with unicode characters + let unicode_command = "héllo wörld 🌍 test command"; + hinter.update_history(unicode_command); + + // Test hint for partial match + let line = "héllo"; + let pos = line.len(); + let empty_history = DefaultHistory::new(); + let ctx = Context::new(&empty_history); + + let hint = hinter.hint(line, pos, &ctx); + // Should handle unicode characters properly in truncation + assert!(hint.is_some()); + let hint_text = hint.unwrap(); + // Should end with "..." due to truncation + assert!(hint_text.ends_with("...")); + } + + #[tokio::test] + async fn test_chat_hinter_multiple_history_entries() { + let mut db = Database::new().await.unwrap(); + // Set truncation limit + db.settings.set(Setting::ChatHistoryMaxLength, 25).await.unwrap(); + + let mut hinter = ChatHinter::new(db); + + // Add multiple commands to history + hinter.update_history("test short command"); + hinter.update_history("test this is a very long command that will be truncated"); + + // Test hint - should match the most recent command + let line = "test"; + let pos = line.len(); + let empty_history = DefaultHistory::new(); + let ctx = Context::new(&empty_history); + + let hint = hinter.hint(line, pos, &ctx); + assert!(hint.is_some()); + let hint_text = hint.unwrap(); + // Should provide hint from the most recent (long) command with truncation + assert!(hint_text.ends_with("...")); + } + + #[tokio::test] + async fn test_chat_hinter_negative_setting_fallback() { + let mut db = Database::new().await.unwrap(); + // Set negative value (should fall back to default) + db.settings.set(Setting::ChatHistoryMaxLength, -10).await.unwrap(); + + let mut hinter = ChatHinter::new(db); + + // Add a command that would be truncated at default (60) but not at a very large limit + let command = "this is a moderately long command that is longer than 60 characters for testing"; + hinter.update_history(command); + + // Test hint for partial match + let line = "this"; + let pos = line.len(); + let empty_history = DefaultHistory::new(); + let ctx = Context::new(&empty_history); + + let hint = hinter.hint(line, pos, &ctx); + assert!(hint.is_some()); + let hint_text = hint.unwrap(); + // Should be truncated because negative setting falls back to default (60) + assert!(hint_text.ends_with("...")); + } + + #[tokio::test] + async fn test_highlighter_no_truncation() { + let mut db = Database::new().await.unwrap(); + // Set a small truncation limit for testing + db.settings.set(Setting::ChatHistoryMaxLength, 20).await.unwrap(); + + let (prompt_request_sender, _) = std::sync::mpsc::channel::>(); + let (_, prompt_response_receiver) = std::sync::mpsc::channel::>(); + let helper = ChatHelper { + completer: ChatCompleter::new(prompt_request_sender, prompt_response_receiver), + hinter: ChatHinter::new(db), + validator: MultiLineValidator, + }; + + // Test that long command is NOT truncated in highlight (user input should be preserved) + let long_command = "this is a very long command that exceeds the limit"; + let highlighted = helper.highlight(long_command, 0); + + // Should NOT be truncated - user should see their full input + assert_eq!(highlighted, long_command); + + // Test short command (should also not be truncated) + let short_command = "short cmd"; + let highlighted = helper.highlight(short_command, 0); + assert_eq!(highlighted, short_command); + } + + #[tokio::test] + async fn test_highlighter_preserves_input_regardless_of_settings() { + let mut db = Database::new().await.unwrap(); + // Set history to hidden (0) + db.settings.set(Setting::ChatHistoryMaxLength, 0).await.unwrap(); + + let (prompt_request_sender, _) = std::sync::mpsc::channel::>(); + let (_, prompt_response_receiver) = std::sync::mpsc::channel::>(); + let helper = ChatHelper { + completer: ChatCompleter::new(prompt_request_sender, prompt_response_receiver), + hinter: ChatHinter::new(db), + validator: MultiLineValidator, + }; + + // Test that user input is preserved even when history is hidden + let command = "any command here"; + let highlighted = helper.highlight(command, 0); + assert_eq!(highlighted, command); // Should preserve user input + } + + #[tokio::test] + async fn test_highlighter_preserves_word_boundaries() { + let mut db = Database::new().await.unwrap(); + // Set a limit that will test word boundary logic + db.settings.set(Setting::ChatHistoryMaxLength, 25).await.unwrap(); + + let (prompt_request_sender, _) = std::sync::mpsc::channel::>(); + let (_, prompt_response_receiver) = std::sync::mpsc::channel::>(); + let helper = ChatHelper { + completer: ChatCompleter::new(prompt_request_sender, prompt_response_receiver), + hinter: ChatHinter::new(db), + validator: MultiLineValidator, + }; + + // Test that user input is preserved regardless of word boundaries + let cmd = "short words here and more text"; + let highlighted = helper.highlight(cmd, 0); + + // Should preserve the full input + assert_eq!(highlighted, cmd); + } + + #[tokio::test] + async fn test_highlighter_unicode_safety() { + let mut db = Database::new().await.unwrap(); + // Set a small limit to test unicode boundary handling + db.settings.set(Setting::ChatHistoryMaxLength, 15).await.unwrap(); + + let (prompt_request_sender, _) = std::sync::mpsc::channel::>(); + let (_, prompt_response_receiver) = std::sync::mpsc::channel::>(); + let helper = ChatHelper { + completer: ChatCompleter::new(prompt_request_sender, prompt_response_receiver), + hinter: ChatHinter::new(db), + validator: MultiLineValidator, + }; + + // Test with unicode characters - should preserve full input + let unicode_cmd = "héllo wörld 🌍 test command"; + let highlighted = helper.highlight(unicode_cmd, 0); + + // Should preserve the full unicode input + assert_eq!(highlighted, unicode_cmd); + } } diff --git a/crates/chat-cli/src/database/settings.rs b/crates/chat-cli/src/database/settings.rs index 84b33aa914..66ee5a3ec9 100644 --- a/crates/chat-cli/src/database/settings.rs +++ b/crates/chat-cli/src/database/settings.rs @@ -15,7 +15,7 @@ use tokio::io::{ use super::DatabaseError; -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum Setting { TelemetryEnabled, OldClientId, @@ -35,6 +35,7 @@ pub enum Setting { ChatDefaultModel, ChatDefaultAgent, ChatDisableAutoCompaction, + ChatHistoryMaxLength, } impl AsRef for Setting { @@ -58,6 +59,7 @@ impl AsRef for Setting { Self::ChatDefaultModel => "chat.defaultModel", Self::ChatDefaultAgent => "chat.defaultAgent", Self::ChatDisableAutoCompaction => "chat.disableAutoCompaction", + Self::ChatHistoryMaxLength => "chat.history.maxLength", } } } @@ -91,6 +93,7 @@ impl TryFrom<&str> for Setting { "chat.defaultModel" => Ok(Self::ChatDefaultModel), "chat.defaultAgent" => Ok(Self::ChatDefaultAgent), "chat.disableAutoCompaction" => Ok(Self::ChatDisableAutoCompaction), + "chat.history.maxLength" => Ok(Self::ChatHistoryMaxLength), _ => Err(DatabaseError::InvalidSetting(value.to_string())), } } @@ -242,4 +245,50 @@ mod test { assert_eq!(settings.get(Setting::ShareCodeWhispererContent), None); assert_eq!(settings.get(Setting::McpLoadedBefore), None); } + + /// Test ChatHistoryMaxLength setting + #[tokio::test] + async fn test_chat_history_max_length_setting() { + let mut settings = Settings::new().await.unwrap(); + + // Test initial state (should be None) + assert_eq!(settings.get(Setting::ChatHistoryMaxLength), None); + assert_eq!(settings.get_int(Setting::ChatHistoryMaxLength), None); + + // Test setting integer value + settings.set(Setting::ChatHistoryMaxLength, 60).await.unwrap(); + assert_eq!(settings.get(Setting::ChatHistoryMaxLength), Some(&Value::Number(60.into()))); + assert_eq!(settings.get_int(Setting::ChatHistoryMaxLength), Some(60)); + + // Test setting different values + settings.set(Setting::ChatHistoryMaxLength, 100).await.unwrap(); + assert_eq!(settings.get_int(Setting::ChatHistoryMaxLength), Some(100)); + + settings.set(Setting::ChatHistoryMaxLength, 0).await.unwrap(); + assert_eq!(settings.get_int(Setting::ChatHistoryMaxLength), Some(0)); + + // Test removing the setting + settings.remove(Setting::ChatHistoryMaxLength).await.unwrap(); + assert_eq!(settings.get(Setting::ChatHistoryMaxLength), None); + assert_eq!(settings.get_int(Setting::ChatHistoryMaxLength), None); + } + + /// Test Setting enum string conversions for ChatHistoryMaxLength + #[test] + fn test_chat_history_max_length_string_conversions() { + // Test AsRef implementation + assert_eq!(Setting::ChatHistoryMaxLength.as_ref(), "chat.history.maxLength"); + + // Test Display implementation + assert_eq!(Setting::ChatHistoryMaxLength.to_string(), "chat.history.maxLength"); + + // Test TryFrom<&str> implementation + assert_eq!( + Setting::try_from("chat.history.maxLength").unwrap(), + Setting::ChatHistoryMaxLength + ); + + // Test invalid string conversion + assert!(Setting::try_from("invalid.setting").is_err()); + } }