From 07c853f210148c503d72ee983763b89a58735f25 Mon Sep 17 00:00:00 2001 From: swananan Date: Mon, 6 Oct 2025 00:33:38 +0800 Subject: [PATCH 1/9] fix: unify error handling and improve ui response --- ghostscope-compiler/src/lib.rs | 2 +- ghostscope-compiler/src/script/compiler.rs | 67 +++- ghostscope-ui/src/components/app.rs | 326 +++++++----------- .../command_panel/optimized_input.rs | 5 + .../command_panel/response_formatter.rs | 13 + ghostscope-ui/src/events.rs | 10 +- ghostscope/src/runtime/coordinator.rs | 76 ++-- ghostscope/src/runtime/dwarf_loader.rs | 16 +- ghostscope/src/runtime/trace_handlers.rs | 69 ++-- 9 files changed, 298 insertions(+), 286 deletions(-) diff --git a/ghostscope-compiler/src/lib.rs b/ghostscope-compiler/src/lib.rs index ecf914eb..07c812da 100644 --- a/ghostscope-compiler/src/lib.rs +++ b/ghostscope-compiler/src/lib.rs @@ -27,7 +27,7 @@ pub enum CompileError { #[error("LLVM error: {0}")] LLVM(String), - #[error("Error: {0}")] + #[error("{0}")] Other(String), } diff --git a/ghostscope-compiler/src/script/compiler.rs b/ghostscope-compiler/src/script/compiler.rs index fe31c703..fb053af1 100644 --- a/ghostscope-compiler/src/script/compiler.rs +++ b/ghostscope-compiler/src/script/compiler.rs @@ -113,6 +113,7 @@ impl<'a> AstCompiler<'a> { // Continue processing even if some trace points fail let mut successful_trace_points = 0; let mut failed_trace_points = 0; + let mut first_error: Option = None; for (index, stmt) in program.statements.iter().enumerate() { match stmt { @@ -128,11 +129,46 @@ impl<'a> AstCompiler<'a> { } Err(e) => { failed_trace_points += 1; + let error_msg = e.to_string(); error!( "❌ Failed to process trace point {}: {:?} - Error: {}", - index, pattern, e + index, pattern, error_msg ); - // Continue processing other trace points + + // Save first error for detailed error message + if first_error.is_none() { + first_error = Some(error_msg.clone()); + } + + // Check if failed_targets was already populated by process_trace_point + // (e.g., when all addresses failed for a function) + // If not, add a general failed target entry + let has_failed_for_this_pattern = + self.failed_targets.iter().any(|ft| match pattern { + TracePattern::FunctionName(name) => ft.target_name == *name, + TracePattern::SourceLine { + file_path, + line_number, + } => ft.target_name == format!("{}:{}", file_path, line_number), + _ => false, + }); + + if !has_failed_for_this_pattern { + let target_name = match pattern { + TracePattern::FunctionName(name) => name.clone(), + TracePattern::SourceLine { + file_path, + line_number, + } => format!("{}:{}", file_path, line_number), + _ => format!("trace_point_{}", index), + }; + + self.failed_targets.push(FailedTarget { + target_name, + pc_address: 0, + error_message: error_msg, + }); + } } } } @@ -153,12 +189,12 @@ impl<'a> AstCompiler<'a> { "Partial success: {} trace points successful, {} failed", successful_trace_points, failed_trace_points ); - } else { + } else if failed_trace_points > 0 { + // All trace points failed - return error with first failure reason error!("All {} trace points failed to process", failed_trace_points); - return Err(CompileError::Other(format!( - "All {} trace points failed to process", - failed_trace_points - ))); + return Err(CompileError::Other( + first_error.unwrap_or_else(|| "All trace points failed".to_string()), + )); } // Generate target info summary @@ -291,11 +327,11 @@ impl<'a> AstCompiler<'a> { }; if module_addresses.is_empty() { - warn!( - "No addresses resolved for function '{}'; skipping", + // Strict behavior: fail this trace point immediately instead of skipping silently + return Err(CompileError::Other(format!( + "No addresses resolved for function '{}' - function not found in debug symbols", func_name - ); - return Ok(()); + ))); } let total_addresses: usize = module_addresses.len(); @@ -359,19 +395,20 @@ impl<'a> AstCompiler<'a> { "All {} addresses for function '{}' processed successfully", successful_addresses, func_name ); + Ok(()) } else if successful_addresses > 0 && failed_addresses > 0 { warn!( "Partial success for function '{}': {} successful, {} failed addresses", func_name, successful_addresses, failed_addresses ); + Ok(()) } else { - error!( + // All addresses failed to process - this is an error + Err(CompileError::Other(format!( "All {} addresses for function '{}' failed to process", failed_addresses, func_name - ); - // Don't return error here - let the caller decide based on overall results + ))) } - Ok(()) } _ => { unimplemented!(); diff --git a/ghostscope-ui/src/components/app.rs b/ghostscope-ui/src/components/app.rs index e7ade49e..c6c58a2d 100644 --- a/ghostscope-ui/src/components/app.rs +++ b/ghostscope-ui/src/components/app.rs @@ -2493,37 +2493,57 @@ impl App { }; let _ = self.handle_action(action); } - RuntimeStatus::AllTracesEnabled { count } => { + RuntimeStatus::AllTracesEnabled { count, error } => { self.clear_waiting_state(); - // Move all known traces from disabled to enabled - for (file_path, line_num) in self.state.source_panel.trace_locations.values() { - if self.state.source_panel.file_path.as_ref() == Some(file_path) { - self.state.source_panel.disabled_lines.remove(line_num); - self.state.source_panel.traced_lines.insert(*line_num); + if error.is_none() { + // Move all known traces from disabled to enabled + for (file_path, line_num) in self.state.source_panel.trace_locations.values() { + if self.state.source_panel.file_path.as_ref() == Some(file_path) { + self.state.source_panel.disabled_lines.remove(line_num); + self.state.source_panel.traced_lines.insert(*line_num); + } } } let action = Action::AddResponse { - content: format!("✓ All traces enabled ({count} traces)"), - response_type: crate::action::ResponseType::Success, + content: if let Some(ref err) = error { + format!("✗ Failed to enable traces: {err}") + } else { + format!("✓ All traces enabled ({count} traces)") + }, + response_type: if error.is_some() { + crate::action::ResponseType::Error + } else { + crate::action::ResponseType::Success + }, }; let _ = self.handle_action(action); } - RuntimeStatus::AllTracesDisabled { count } => { + RuntimeStatus::AllTracesDisabled { count, error } => { self.clear_waiting_state(); - // Move all known traces from enabled to disabled - for (file_path, line_num) in self.state.source_panel.trace_locations.values() { - if self.state.source_panel.file_path.as_ref() == Some(file_path) { - self.state.source_panel.traced_lines.remove(line_num); - self.state.source_panel.disabled_lines.insert(*line_num); + if error.is_none() { + // Move all known traces from enabled to disabled + for (file_path, line_num) in self.state.source_panel.trace_locations.values() { + if self.state.source_panel.file_path.as_ref() == Some(file_path) { + self.state.source_panel.traced_lines.remove(line_num); + self.state.source_panel.disabled_lines.insert(*line_num); + } } } let action = Action::AddResponse { - content: format!("✓ All traces disabled ({count} traces)"), - response_type: crate::action::ResponseType::Success, + content: if let Some(ref err) = error { + format!("✗ Failed to disable traces: {err}") + } else { + format!("✓ All traces disabled ({count} traces)") + }, + response_type: if error.is_some() { + crate::action::ResponseType::Error + } else { + crate::action::ResponseType::Success + }, }; let _ = self.handle_action(action); } @@ -2562,17 +2582,27 @@ impl App { }; let _ = self.handle_action(action); } - RuntimeStatus::AllTracesDeleted { count } => { + RuntimeStatus::AllTracesDeleted { count, error } => { self.clear_waiting_state(); - // Clear all trace locations and colors - self.state.source_panel.traced_lines.clear(); - self.state.source_panel.disabled_lines.clear(); - self.state.source_panel.trace_locations.clear(); + if error.is_none() { + // Clear all trace locations and colors + self.state.source_panel.traced_lines.clear(); + self.state.source_panel.disabled_lines.clear(); + self.state.source_panel.trace_locations.clear(); + } let action = Action::AddResponse { - content: format!("✓ All traces deleted ({count} traces)"), - response_type: crate::action::ResponseType::Success, + content: if let Some(ref err) = error { + format!("✗ Failed to delete traces: {err}") + } else { + format!("✓ All traces deleted ({count} traces)") + }, + response_type: if error.is_some() { + crate::action::ResponseType::Error + } else { + crate::action::ResponseType::Success + }, }; let _ = self.handle_action(action); } @@ -2664,47 +2694,27 @@ impl App { }; let _ = self.handle_action(action); } - RuntimeStatus::ScriptCompilationFailed { error, target } => { - self.clear_waiting_state(); - // Provide detailed script compilation failure information - let mut formatted_error = - format!("❌ Script compilation failed for target '{target}':\n"); - - // Parse and format the error for better readability - if error.contains("error:") && error.contains("line") { - // Parse compiler-style errors - formatted_error.push_str("\n Compilation Error Details:\n"); - for line in error.lines() { - let trimmed = line.trim(); - if trimmed.starts_with("error:") - || trimmed.starts_with("warning:") - || trimmed.starts_with("note:") - || line.contains("-->") - { - formatted_error.push_str(&format!(" {trimmed}\n")); - } else if !trimmed.is_empty() { - formatted_error.push_str(&format!(" {trimmed}\n")); - } - } - } else { - // Simple error message - formatted_error.push_str(&format!("\n Error: {error}\n")); - } - - formatted_error.push_str("\n Troubleshooting:"); - formatted_error.push_str("\n • Check your script syntax"); - formatted_error.push_str("\n • Verify function/variable names exist"); - formatted_error.push_str("\n • Use 'info ' to check debug information"); - - let action = Action::AddResponse { - content: formatted_error, - response_type: crate::action::ResponseType::Error, - }; - let _ = self.handle_action(action); - } _ => { // Handle other runtime status messages (delegate to command panel or other components) // For now, pass them to command panel for display + + // Check if this is an error status or completed status that should clear waiting state + let should_clear_waiting = matches!( + status, + RuntimeStatus::AllTracesEnabled { .. } + | RuntimeStatus::AllTracesDisabled { .. } + | RuntimeStatus::AllTracesDeleted { .. } + | RuntimeStatus::ScriptCompilationCompleted { .. } + | RuntimeStatus::TraceInfoFailed { .. } + | RuntimeStatus::FileInfoFailed { .. } + | RuntimeStatus::ShareInfoFailed { .. } + | RuntimeStatus::SrcPathFailed { .. } + ); + + if should_clear_waiting { + self.clear_waiting_state(); + } + if let Some(content) = self.format_runtime_status_for_display(&status) { let action = Action::AddResponse { content, @@ -2940,146 +2950,52 @@ impl App { } } } - RuntimeStatus::ScriptCompilationFailed { error, target } => { - // Check if this is part of a batch load operation - if let Some(ref mut batch) = self.state.command_panel.batch_loading { - // Update batch loading state - batch.completed_count += 1; - batch.failed_count += 1; - - // Add failed trace detail - batch.details.push(crate::events::TraceLoadDetail { - target: target.clone(), - trace_id: None, - status: crate::events::LoadStatus::Failed, - error: Some(error.clone()), - }); - - // Check if all traces have been processed - if batch.completed_count >= batch.total_count { - // All traces processed, show summary (same as in ScriptCompilationCompleted) - let filename = batch.filename.clone(); - let total_count = batch.total_count; - let success_count = batch.success_count; - let failed_count = batch.failed_count; - let disabled_count = batch.disabled_count; - let details = batch.details.clone(); - - // Clear batch loading state - self.state.command_panel.batch_loading = None; - - // Clear waiting state - self.clear_waiting_state(); - - // Show summary response - let mut response = format!("📂 Loaded traces from {filename}\n"); - response.push_str(&format!( - " Total: {total_count}, Success: {success_count}, Failed: {failed_count}" - )); - if disabled_count > 0 { - response.push_str(&format!(", Disabled: {disabled_count}")); - } - response.push('\n'); - - // Show details - if !details.is_empty() { - response.push_str("\n📊 Details:\n"); - for detail in &details { - match detail.status { - crate::events::LoadStatus::Created => { - if let Some(id) = detail.trace_id { - response.push_str(&format!( - " ✓ {} → trace #{}\n", - detail.target, id - )); - } else { - response.push_str(&format!(" ✓ {}\n", detail.target)); - } - } - crate::events::LoadStatus::CreatedDisabled => { - if let Some(id) = detail.trace_id { - response.push_str(&format!( - " ⊘ {} → trace #{} (disabled)\n", - detail.target, id - )); - } else { - response.push_str(&format!( - " ⊘ {} (disabled)\n", - detail.target - )); - } - } - crate::events::LoadStatus::Failed => { - if let Some(ref err) = detail.error { - response.push_str(&format!( - " ✗ {}: {}\n", - detail.target, err - )); - } else { - response.push_str(&format!(" ✗ {}\n", detail.target)); - } - } - _ => {} - } - } - } - - let action = Action::AddResponse { - content: response, - response_type: if failed_count > 0 { - crate::action::ResponseType::Warning - } else { - crate::action::ResponseType::Success - }, - }; - let _ = self.handle_action(action); - - // Don't return - suppress individual error display - return None; - } else { - // Still waiting for more traces, suppress individual response - return None; - } + RuntimeStatus::AllTracesEnabled { count, error } => { + if let Some(ref err) = error { + let error_emoji = self + .state + .emoji_config + .get_script_status(crate::ui::emoji::ScriptStatus::Error); + Some(format!("{error_emoji} {err}")) + } else if *count > 0 { + let success_emoji = self + .state + .emoji_config + .get_trace_status(crate::ui::emoji::TraceStatusType::Active); + Some(format!("{success_emoji} Enabled {count} traces")) + } else { + None } - - // Not batch loading, handle normally - // Clear any pending trace line status on failure - if self.state.source_panel.pending_trace_line.is_some() { - self.state.source_panel.pending_trace_line = None; + } + RuntimeStatus::AllTracesDisabled { count, error } => { + if let Some(ref err) = error { + let error_emoji = self + .state + .emoji_config + .get_script_status(crate::ui::emoji::ScriptStatus::Error); + Some(format!("{error_emoji} {err}")) + } else if *count > 0 { + let disabled_emoji = self + .state + .emoji_config + .get_trace_status(crate::ui::emoji::TraceStatusType::Disabled); + Some(format!("{disabled_emoji} Disabled {count} traces")) + } else { + None + } + } + RuntimeStatus::AllTracesDeleted { count, error } => { + if let Some(ref err) = error { + let error_emoji = self + .state + .emoji_config + .get_script_status(crate::ui::emoji::ScriptStatus::Error); + Some(format!("{error_emoji} {err}")) + } else if *count > 0 { + Some(format!("✓ Deleted {count} traces")) + } else { + None } - - // Create detailed error information - let error_details = - crate::components::command_panel::script_editor::TraceErrorDetails { - compilation_errors: None, // Could be enhanced to parse error details - uprobe_error: Some(error.clone()), - suggestion: Some( - "Check function name and ensure binary has debug symbols".to_string(), - ), - }; - - // Get script content for error display - let script_content = self - .state - .command_panel - .script_cache - .as_ref() - .map(|cache| cache.lines.join("\n")); - - Some(crate::components::command_panel::script_editor::ScriptEditor::format_trace_error_response_with_script( - target, - error, - Some(&error_details), - script_content.as_deref(), - &self.state.emoji_config, - )) - } - RuntimeStatus::Error(msg) => { - let error_emoji = self - .state - .emoji_config - .get_script_status(crate::ui::emoji::ScriptStatus::Error); - Some(format!("{error_emoji} Error: {msg}")) } RuntimeStatus::TraceEnabled { trace_id } => { let success_emoji = self @@ -3107,9 +3023,6 @@ impl App { use crate::events::RuntimeStatus; match status { - RuntimeStatus::Error(_) | RuntimeStatus::ScriptCompilationFailed { .. } => { - crate::action::ResponseType::Error - } RuntimeStatus::ScriptCompilationCompleted { details } => { // Check if compilation actually succeeded if details.success_count > 0 { @@ -3118,6 +3031,15 @@ impl App { crate::action::ResponseType::Error } } + RuntimeStatus::AllTracesEnabled { error, .. } + | RuntimeStatus::AllTracesDisabled { error, .. } + | RuntimeStatus::AllTracesDeleted { error, .. } => { + if error.is_some() { + crate::action::ResponseType::Error + } else { + crate::action::ResponseType::Success + } + } _ => crate::action::ResponseType::Info, } } diff --git a/ghostscope-ui/src/components/command_panel/optimized_input.rs b/ghostscope-ui/src/components/command_panel/optimized_input.rs index 151f7979..401e62ea 100644 --- a/ghostscope-ui/src/components/command_panel/optimized_input.rs +++ b/ghostscope-ui/src/components/command_panel/optimized_input.rs @@ -740,6 +740,11 @@ impl OptimizedInputHandler { }; state.command_history.push(item); + tracing::debug!( + "add_command_to_history: Added command '{}', history length now: {}", + command, + state.command_history.len() + ); // Limit history size const MAX_HISTORY: usize = 1000; diff --git a/ghostscope-ui/src/components/command_panel/response_formatter.rs b/ghostscope-ui/src/components/command_panel/response_formatter.rs index fdc11b4d..793f2c2d 100644 --- a/ghostscope-ui/src/components/command_panel/response_formatter.rs +++ b/ghostscope-ui/src/components/command_panel/response_formatter.rs @@ -24,6 +24,19 @@ impl ResponseFormatter { if let Some(last_item) = state.command_history.last_mut() { last_item.response = Some(content); last_item.response_type = Some(response_type); + tracing::debug!( + "add_response: Added response to command '{}'", + last_item.command + ); + } else { + tracing::warn!( + "add_response: No command in history to attach response to! Response content: '{}'", + content + ); + tracing::warn!( + "add_response: command_history length: {}", + state.command_history.len() + ); } // Note: Optimized renderer will handle display updates via cache rebuild } diff --git a/ghostscope-ui/src/events.rs b/ghostscope-ui/src/events.rs index 1e708d65..ffdbc071 100644 --- a/ghostscope-ui/src/events.rs +++ b/ghostscope-ui/src/events.rs @@ -425,11 +425,7 @@ pub enum RuntimeStatus { }, DwarfLoadingFailed(String), ScriptCompilationCompleted { - details: ScriptCompilationDetails, // Now required, contains trace_ids - }, - ScriptCompilationFailed { - error: String, - target: String, // Target instead of trace_id since we don't have trace_ids for failed compilations + details: ScriptCompilationDetails, // Contains trace_ids, success/failed counts and results }, UprobeAttached { function: String, @@ -448,9 +444,11 @@ pub enum RuntimeStatus { }, AllTracesEnabled { count: usize, + error: Option, // Error message if operation completely failed }, AllTracesDisabled { count: usize, + error: Option, // Error message if operation completely failed }, TraceEnableFailed { trace_id: u32, @@ -465,6 +463,7 @@ pub enum RuntimeStatus { }, AllTracesDeleted { count: usize, + error: Option, // Error message if operation completely failed }, TraceDeleteFailed { trace_id: u32, @@ -585,7 +584,6 @@ pub enum RuntimeStatus { SrcPathFailed { error: String, }, - Error(String), } /// Statistics for a loaded module diff --git a/ghostscope/src/runtime/coordinator.rs b/ghostscope/src/runtime/coordinator.rs index 876d8935..d491469c 100644 --- a/ghostscope/src/runtime/coordinator.rs +++ b/ghostscope/src/runtime/coordinator.rs @@ -370,36 +370,58 @@ async fn handle_execute_script( info!("Executing script: {}", script); if let Some(ref mut session) = session { - match crate::script::compile_and_load_script_for_tui(&script, session, compile_options) - .await - { - Ok(details) => { - info!( - "✓ Script compilation completed: {} total, {} success, {} failed", - details.total_count, details.success_count, details.failed_count - ); - let _ = runtime_channels - .status_sender - .send(RuntimeStatus::ScriptCompilationCompleted { details }); - } - Err(e) => { - error!("❌ Script compilation failed: {}", e); - let _ = - runtime_channels - .status_sender - .send(RuntimeStatus::ScriptCompilationFailed { - error: format!("Script compilation failed: {}", e), - target: script.clone(), - }); - } - } + let details = + match crate::script::compile_and_load_script_for_tui(&script, session, compile_options) + .await + { + Ok(details) => { + info!( + "✓ Script compilation completed: {} total, {} success, {} failed", + details.total_count, details.success_count, details.failed_count + ); + details + } + Err(e) => { + error!("❌ Script compilation failed: {}", e); + // Return details with all failures + ghostscope_ui::events::ScriptCompilationDetails { + trace_ids: vec![], + results: vec![ghostscope_ui::events::ScriptExecutionResult { + pc_address: 0, + target_name: script.clone(), + binary_path: String::new(), + status: ghostscope_ui::events::ExecutionStatus::Failed(e.to_string()), + }], + total_count: 1, + success_count: 0, + failed_count: 1, + } + } + }; + + let _ = runtime_channels + .status_sender + .send(RuntimeStatus::ScriptCompilationCompleted { details }); } else { + // No session available - return details with failure + let details = ghostscope_ui::events::ScriptCompilationDetails { + trace_ids: vec![], + results: vec![ghostscope_ui::events::ScriptExecutionResult { + pc_address: 0, + target_name: script.clone(), + binary_path: String::new(), + status: ghostscope_ui::events::ExecutionStatus::Failed( + "No debug session available".to_string(), + ), + }], + total_count: 1, + success_count: 0, + failed_count: 1, + }; + let _ = runtime_channels .status_sender - .send(RuntimeStatus::ScriptCompilationFailed { - error: "No debug session available".to_string(), - target: script, - }); + .send(RuntimeStatus::ScriptCompilationCompleted { details }); } } diff --git a/ghostscope/src/runtime/dwarf_loader.rs b/ghostscope/src/runtime/dwarf_loader.rs index ae3217db..a71ae1a5 100644 --- a/ghostscope/src/runtime/dwarf_loader.rs +++ b/ghostscope/src/runtime/dwarf_loader.rs @@ -106,15 +106,15 @@ pub async fn initialize_dwarf_processing_with_progress( let symbols_count = functions.len(); // Send success status - let _ = - status_sender.send(RuntimeStatus::DwarfLoadingCompleted { symbols_count }); - + // If no debug information was found, treat it as a loading failure if stats.modules_with_debug_info == 0 { - let _ = status_sender.send( - RuntimeStatus::Error( - "No debug information available. Compile with -g for full functionality".to_string() - ) - ); + let _ = status_sender.send(RuntimeStatus::DwarfLoadingFailed( + "No debug information available. Compile with -g for full functionality" + .to_string(), + )); + } else { + let _ = status_sender + .send(RuntimeStatus::DwarfLoadingCompleted { symbols_count }); } // Return the session for use by runtime coordinator diff --git a/ghostscope/src/runtime/trace_handlers.rs b/ghostscope/src/runtime/trace_handlers.rs index 1931a424..62a78b33 100644 --- a/ghostscope/src/runtime/trace_handlers.rs +++ b/ghostscope/src/runtime/trace_handlers.rs @@ -22,7 +22,7 @@ pub async fn handle_disable_trace( .status_sender .send(RuntimeStatus::TraceDisableFailed { trace_id, - error: format!("Failed to disable trace: {}", e), + error: e.to_string(), }); } } @@ -56,7 +56,7 @@ pub async fn handle_enable_trace( .status_sender .send(RuntimeStatus::TraceEnableFailed { trace_id, - error: format!("Failed to enable trace: {}", e), + error: e.to_string(), }); } } @@ -82,22 +82,28 @@ pub async fn handle_disable_all_traces( info!("✓ Disabled all traces (count: {})", trace_count); let _ = runtime_channels .status_sender - .send(RuntimeStatus::AllTracesDisabled { count: trace_count }); + .send(RuntimeStatus::AllTracesDisabled { + count: trace_count, + error: None, + }); } Err(e) => { error!("❌ Failed to disable all traces: {}", e); let _ = runtime_channels .status_sender - .send(RuntimeStatus::Error(format!( - "Failed to disable all traces: {}", - e - ))); + .send(RuntimeStatus::AllTracesDisabled { + count: 0, + error: Some(format!("Failed to disable all traces: {}", e)), + }); } } } else { - let _ = runtime_channels.status_sender.send(RuntimeStatus::Error( - "No debug session available".to_string(), - )); + let _ = runtime_channels + .status_sender + .send(RuntimeStatus::AllTracesDisabled { + count: 0, + error: Some("No debug session available".to_string()), + }); } } @@ -113,22 +119,28 @@ pub async fn handle_enable_all_traces( info!("✓ Enabled all traces (count: {})", trace_count); let _ = runtime_channels .status_sender - .send(RuntimeStatus::AllTracesEnabled { count: trace_count }); + .send(RuntimeStatus::AllTracesEnabled { + count: trace_count, + error: None, + }); } Err(e) => { error!("❌ Failed to enable all traces: {}", e); let _ = runtime_channels .status_sender - .send(RuntimeStatus::Error(format!( - "Failed to enable all traces: {}", - e - ))); + .send(RuntimeStatus::AllTracesEnabled { + count: 0, + error: Some(format!("Failed to enable all traces: {}", e)), + }); } } } else { - let _ = runtime_channels.status_sender.send(RuntimeStatus::Error( - "No debug session available".to_string(), - )); + let _ = runtime_channels + .status_sender + .send(RuntimeStatus::AllTracesEnabled { + count: 0, + error: Some("No debug session available".to_string()), + }); } } @@ -152,7 +164,7 @@ pub async fn handle_delete_trace( .status_sender .send(RuntimeStatus::TraceDeleteFailed { trace_id, - error: format!("Failed to delete trace: {}", e), + error: e.to_string(), }); } } @@ -177,21 +189,24 @@ pub async fn handle_delete_all_traces( info!("✓ Deleted all traces (count: {})", count); let _ = runtime_channels .status_sender - .send(RuntimeStatus::AllTracesDeleted { count }); + .send(RuntimeStatus::AllTracesDeleted { count, error: None }); } Err(e) => { error!("❌ Failed to delete all traces: {}", e); let _ = runtime_channels .status_sender - .send(RuntimeStatus::Error(format!( - "Failed to delete all traces: {}", - e - ))); + .send(RuntimeStatus::AllTracesDeleted { + count: 0, + error: Some(format!("Failed to delete all traces: {}", e)), + }); } } } else { - let _ = runtime_channels.status_sender.send(RuntimeStatus::Error( - "No debug session available".to_string(), - )); + let _ = runtime_channels + .status_sender + .send(RuntimeStatus::AllTracesDeleted { + count: 0, + error: Some("No debug session available".to_string()), + }); } } From 2749a5029cb972c450489ca8209fc519f2a1112a Mon Sep 17 00:00:00 2001 From: swananan Date: Mon, 6 Oct 2025 00:38:36 +0800 Subject: [PATCH 2/9] fix: prevent Ctrl+C from overwriting command history responses --- ghostscope-ui/src/components/app.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ghostscope-ui/src/components/app.rs b/ghostscope-ui/src/components/app.rs index c6c58a2d..216ced2f 100644 --- a/ghostscope-ui/src/components/app.rs +++ b/ghostscope-ui/src/components/app.rs @@ -3335,11 +3335,9 @@ impl App { self.state.command_panel.cursor_position = 4; // Clear auto-suggestion to prevent suggestions after "quit" self.state.command_panel.auto_suggestion.clear(); - vec![Action::AddResponse { - content: "Press Ctrl+C again to quit or modify the command" - .to_string(), - response_type: crate::action::ResponseType::Info, - }] + // Don't add response here - it would attach to previous command in history + // User will see "quit" in input box, which is clear enough + vec![] } _ => { // Other modes - no action needed From 0eabd47a9f896d2bef825d3a329eec5177d62f80 Mon Sep 17 00:00:00 2001 From: swananan Date: Mon, 6 Oct 2025 00:50:45 +0800 Subject: [PATCH 3/9] feat: enhance info trace display --- ghostscope-ui/src/components/app.rs | 6 ++---- ghostscope-ui/src/events.rs | 10 ++-------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/ghostscope-ui/src/components/app.rs b/ghostscope-ui/src/components/app.rs index 216ced2f..6fd8db9b 100644 --- a/ghostscope-ui/src/components/app.rs +++ b/ghostscope-ui/src/components/app.rs @@ -2436,10 +2436,8 @@ impl App { summary.total, summary.active ); for trace in &traces { - response.push_str(&format!( - " #{} - {} ({})\n", - trace.trace_id, trace.target_display, trace.status - )); + // Use format_line() to show detailed info including address and module + response.push_str(&format!(" {}\n", trace.format_line())); } let action = Action::AddResponse { content: response, diff --git a/ghostscope-ui/src/events.rs b/ghostscope-ui/src/events.rs index ffdbc071..3b57cc80 100644 --- a/ghostscope-ui/src/events.rs +++ b/ghostscope-ui/src/events.rs @@ -624,14 +624,8 @@ impl TraceDetailInfo { .unwrap_or(&self.binary_path); format!( - "{} [{}] {}@{}+0x{:x} - {} ({})", - self.status.to_emoji(), - self.trace_id, - self.target_display, - binary_name, - self.pc, - self.status, - self.duration + "#{} | {}+0x{:x} | {} ({}) ", + self.trace_id, binary_name, self.pc, self.target_display, self.status ) } } From 497b362539463942a36a95e466fbfb5c3c115458 Mon Sep 17 00:00:00 2001 From: swananan Date: Mon, 6 Oct 2025 00:54:57 +0800 Subject: [PATCH 4/9] fix: display all trace points when source line maps to multiple addresses --- ghostscope-ui/src/components/app.rs | 57 ++++---------- .../components/command_panel/script_editor.rs | 78 +++++++++++++++++++ 2 files changed, 93 insertions(+), 42 deletions(-) diff --git a/ghostscope-ui/src/components/app.rs b/ghostscope-ui/src/components/app.rs index 6fd8db9b..3d523ef1 100644 --- a/ghostscope-ui/src/components/app.rs +++ b/ghostscope-ui/src/components/app.rs @@ -2872,48 +2872,21 @@ impl App { self.clear_waiting_state(); // Check if compilation actually succeeded - if details.success_count > 0 { - // Find the first successful result - let first_success = details - .results - .iter() - .find(|r| matches!(r.status, crate::events::ExecutionStatus::Success)); - - if let Some(result) = first_success { - // Get the corresponding trace ID from details.trace_ids - let trace_id = if !details.trace_ids.is_empty() { - Some(details.trace_ids[0]) // Use the first trace ID - } else { - None - }; - - let trace_details = - crate::components::command_panel::script_editor::TraceDetails { - trace_id, - binary_path: Some(result.binary_path.clone()), - address: Some(result.pc_address), - source_file: None, // Not available in current structure - line_number: None, // Not available in current structure - function_name: Some(result.target_name.clone()), - }; - - // Get script content from the current cache for better display - let script_content = self - .state - .command_panel - .script_cache - .as_ref() - .map(|cache| cache.lines.join("\n")); - - Some(crate::components::command_panel::script_editor::ScriptEditor::format_trace_success_response_with_script( - &result.target_name, - Some(&trace_details), - script_content.as_deref(), - &self.state.emoji_config, - )) - } else { - None // No successful results found - } + if details.success_count > 0 || details.failed_count > 0 { + // Get script content from the current cache for better display + let script_content = self + .state + .command_panel + .script_cache + .as_ref() + .map(|cache| cache.lines.join("\n")); + + // Use new format_compilation_results to show all traces + Some(crate::components::command_panel::script_editor::ScriptEditor::format_compilation_results( + details, + script_content.as_deref(), + &self.state.emoji_config, + )) } else { // All compilations failed - find the first failed result for error details let first_failed = details.results.first(); diff --git a/ghostscope-ui/src/components/command_panel/script_editor.rs b/ghostscope-ui/src/components/command_panel/script_editor.rs index 12761d26..850f028a 100644 --- a/ghostscope-ui/src/components/command_panel/script_editor.rs +++ b/ghostscope-ui/src/components/command_panel/script_editor.rs @@ -582,6 +582,84 @@ impl ScriptEditor { result.join("\n") } + /// Format compilation results with all successful and failed traces + pub fn format_compilation_results( + compilation_details: &crate::events::ScriptCompilationDetails, + script_content: Option<&str>, + emoji_config: &EmojiConfig, + ) -> String { + let mut result = Vec::new(); + + // 📝 Script section + if let Some(script) = script_content { + let script_lines = Self::format_script_display_section(script, emoji_config); + result.extend(script_lines); + } + + // 🎯 Target line (use first result's target or generic message) + let target_emoji = emoji_config.get_trace_element(crate::ui::emoji::TraceElement::Target); + let target = compilation_details + .results + .first() + .map(|r| r.target_name.clone()) + .unwrap_or_else(|| "unknown".to_string()); + result.push(format!("{target_emoji} Target: {target}")); + + // Empty line for separation + result.push("".to_string()); + + // ✅ Results summary + let success_emoji = emoji_config.get_script_status(crate::ui::emoji::ScriptStatus::Success); + let error_emoji = emoji_config.get_script_status(crate::ui::emoji::ScriptStatus::Error); + + let summary_emoji = if compilation_details.failed_count > 0 { + error_emoji + } else { + success_emoji + }; + + result.push(format!( + "{} Trace Results: {} successful, {} failed", + summary_emoji, compilation_details.success_count, compilation_details.failed_count + )); + + // List all successful traces + let mut trace_idx = 0; + for exec_result in &compilation_details.results { + match &exec_result.status { + crate::events::ExecutionStatus::Success => { + let trace_id = compilation_details.trace_ids.get(trace_idx).copied(); + if let Some(tid) = trace_id { + result.push(format!( + " • {} (0x{:x}) → trace_id: {}", + exec_result.target_name, exec_result.pc_address, tid + )); + trace_idx += 1; + } else { + result.push(format!( + " • {} (0x{:x}) → trace attached", + exec_result.target_name, exec_result.pc_address + )); + } + } + crate::events::ExecutionStatus::Failed(error) => { + result.push(format!( + " ✗ {} (0x{:x}): {}", + exec_result.target_name, exec_result.pc_address, error + )); + } + crate::events::ExecutionStatus::Skipped(reason) => { + result.push(format!( + " ⊘ {} (0x{:x}): {}", + exec_result.target_name, exec_result.pc_address, reason + )); + } + } + } + + result.join("\n") + } + /// Format trace error response with detailed information pub fn format_trace_error_response( target: &str, From 924e2fbe6e47eea9338b12aacd651d8bfe792249 Mon Sep 17 00:00:00 2001 From: swananan Date: Mon, 6 Oct 2025 01:25:38 +0800 Subject: [PATCH 5/9] fix: remove duplicate trace command display in command panel --- ghostscope-ui/src/components/app.rs | 30 ++--------------- .../command_panel/optimized_input.rs | 1 + .../command_panel/optimized_renderer.rs | 7 +--- .../command_panel/response_formatter.rs | 32 ++----------------- 4 files changed, 8 insertions(+), 62 deletions(-) diff --git a/ghostscope-ui/src/components/app.rs b/ghostscope-ui/src/components/app.rs index 3d523ef1..e65a94ee 100644 --- a/ghostscope-ui/src/components/app.rs +++ b/ghostscope-ui/src/components/app.rs @@ -1597,11 +1597,10 @@ impl App { // Focus command panel self.state.ui.focus.current_panel = PanelType::InteractiveCommand; - // Add command to both history managers + // Add command to history manager (for Ctrl+R search) self.state .command_panel - .command_history_manager - .add_command(&trace_command); + .add_command_to_history(&trace_command); // Add to command_history for proper response handling self.state.command_panel.command_history.push( @@ -1614,30 +1613,7 @@ impl App { }, ); - // Add the command to static lines for display - self.state.command_panel.static_lines.push( - crate::model::panel_state::StaticTextLine { - content: format!( - "{} {}", - crate::ui::strings::UIStrings::GHOSTSCOPE_PROMPT, - trace_command - ), - line_type: crate::model::panel_state::LineType::Command, - history_index: Some( - self.state - .command_panel - .command_history - .len() - .saturating_sub(1), - ), - response_type: None, - styled_content: None, - }, - ); - - // Don't store trace line here - will be determined from trace info response - - // Clear input and directly enter script mode + // Clear input self.state.command_panel.input_text.clear(); self.state.command_panel.cursor_position = 0; diff --git a/ghostscope-ui/src/components/command_panel/optimized_input.rs b/ghostscope-ui/src/components/command_panel/optimized_input.rs index 401e62ea..c35394b6 100644 --- a/ghostscope-ui/src/components/command_panel/optimized_input.rs +++ b/ghostscope-ui/src/components/command_panel/optimized_input.rs @@ -751,6 +751,7 @@ impl OptimizedInputHandler { if state.command_history.len() > MAX_HISTORY { state.command_history.remove(0); } + // Note: Renderer will display command_history directly } } diff --git a/ghostscope-ui/src/components/command_panel/optimized_renderer.rs b/ghostscope-ui/src/components/command_panel/optimized_renderer.rs index e655f466..693a6a15 100644 --- a/ghostscope-ui/src/components/command_panel/optimized_renderer.rs +++ b/ghostscope-ui/src/components/command_panel/optimized_renderer.rs @@ -166,7 +166,7 @@ impl OptimizedRenderer { // Response lines if let Some(ref response) = item.response { - let response_lines = self.split_response_lines(response); + let response_lines: Vec = response.lines().map(String::from).collect(); for response_line in response_lines { let wrapped_responses = self.wrap_text(&response_line, width); for wrapped_response in wrapped_responses { @@ -1328,11 +1328,6 @@ impl OptimizedRenderer { result_lines } - /// Split response into lines - fn split_response_lines(&self, response: &str) -> Vec { - response.lines().map(String::from).collect() - } - /// Scroll methods for API compatibility pub fn scroll_up(&mut self) { if self.scroll_offset > 0 { diff --git a/ghostscope-ui/src/components/command_panel/response_formatter.rs b/ghostscope-ui/src/components/command_panel/response_formatter.rs index 793f2c2d..de566283 100644 --- a/ghostscope-ui/src/components/command_panel/response_formatter.rs +++ b/ghostscope-ui/src/components/command_panel/response_formatter.rs @@ -38,7 +38,7 @@ impl ResponseFormatter { state.command_history.len() ); } - // Note: Optimized renderer will handle display updates via cache rebuild + // Note: Renderer will display command_history directly } // Removed add_welcome_message - now using direct styled approach @@ -83,18 +83,8 @@ impl ResponseFormatter { } } - // Add current input line if should show prompt - if Self::should_show_input_prompt(state) { - let prompt = Self::get_prompt(state); - let input_line = format!("{prompt}{input}", input = state.input_text); - state.static_lines.push(StaticTextLine { - content: input_line, - line_type: LineType::CurrentInput, - history_index: None, - response_type: None, - styled_content: None, - }); - } + // Note: Current input line is rendered separately by the renderer (render_normal_input) + // Don't add it to static_lines to avoid duplication } /// Split response into individual lines for display @@ -327,22 +317,6 @@ impl ResponseFormatter { } } - /// Check if input prompt should be shown - fn should_show_input_prompt(state: &CommandPanelState) -> bool { - matches!( - state.input_state, - crate::model::panel_state::InputState::Ready - ) - } - - /// Get the current prompt string - fn get_prompt(state: &CommandPanelState) -> String { - if !Self::should_show_input_prompt(state) { - return String::new(); - } - UIStrings::GHOSTSCOPE_PROMPT.to_string() - } - /// Format file information display pub fn format_file_info(groups: &[crate::events::SourceFileGroup], use_ascii: bool) -> String { const MAX_FILES_DETAILED: usize = 1000; From 3c709e7e2ba4f60bbd30f1f14a82f02b38ffbde2 Mon Sep 17 00:00:00 2001 From: swananan Date: Mon, 6 Oct 2025 01:35:26 +0800 Subject: [PATCH 6/9] fix: prevent Ctrl+C in history search from clearing previous response --- ghostscope-ui/src/components/app.rs | 8 ++++---- .../src/components/command_panel/input_handler.rs | 12 ++++-------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/ghostscope-ui/src/components/app.rs b/ghostscope-ui/src/components/app.rs index e65a94ee..0de29105 100644 --- a/ghostscope-ui/src/components/app.rs +++ b/ghostscope-ui/src/components/app.rs @@ -3264,10 +3264,10 @@ impl App { if self.state.command_panel.is_in_history_search() { // In history search mode - exit search directly self.state.command_panel.exit_history_search(); - vec![Action::AddResponse { - content: String::new(), - response_type: crate::action::ResponseType::Info, - }] + self.state.command_panel.input_text.clear(); + self.state.command_panel.cursor_position = 0; + // Don't add empty response - would overwrite previous command's response + vec![] } else { match self.state.command_panel.mode { crate::model::panel_state::InteractionMode::ScriptEditor => { diff --git a/ghostscope-ui/src/components/command_panel/input_handler.rs b/ghostscope-ui/src/components/command_panel/input_handler.rs index 441cc3cc..53638740 100644 --- a/ghostscope-ui/src/components/command_panel/input_handler.rs +++ b/ghostscope-ui/src/components/command_panel/input_handler.rs @@ -140,20 +140,16 @@ impl InputHandler { }; state.exit_history_search_with_selection(&selected_command); - vec![Action::AddResponse { - content: String::new(), - response_type: crate::action::ResponseType::Info, - }] + // Don't add empty response - would overwrite previous command's response + vec![] } // Ctrl+C: Exit search mode and clear input (KeyCode::Char('c'), KeyModifiers::CONTROL) => { state.exit_history_search(); state.input_text.clear(); state.cursor_position = 0; - vec![Action::AddResponse { - content: String::new(), - response_type: crate::action::ResponseType::Info, - }] + // Don't add empty response - would overwrite previous command's response + vec![] } // Enter: Execute the current search result (KeyCode::Enter, _) => { From 03e4a10146d24a940cf07a9eb999debcb19900e5 Mon Sep 17 00:00:00 2001 From: swananan Date: Mon, 6 Oct 2025 01:45:27 +0800 Subject: [PATCH 7/9] feat: show empty command line in history for user feedback --- .../command_panel/optimized_input.rs | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/ghostscope-ui/src/components/command_panel/optimized_input.rs b/ghostscope-ui/src/components/command_panel/optimized_input.rs index c35394b6..b924f68d 100644 --- a/ghostscope-ui/src/components/command_panel/optimized_input.rs +++ b/ghostscope-ui/src/components/command_panel/optimized_input.rs @@ -556,30 +556,24 @@ impl OptimizedInputHandler { InteractionMode::Input => { // Submit the current input as a command let command = state.input_text.clone(); - if !command.trim().is_empty() { - // First add the command to history - self.add_command_to_history(state, &command); - // Reset history navigation to start from newest command next time - state.history_index = None; + // Always add command to history (even if empty) to show user feedback + self.add_command_to_history(state, &command); - // Then parse and execute the command - use crate::components::command_panel::CommandParser; - let actions = CommandParser::parse_command(state, &command); + // Reset history navigation to start from newest command next time + state.history_index = None; - // Clear input after command submission - state.input_text.clear(); - state.cursor_position = 0; - - // Clear auto-suggestion since input is cleared - state.auto_suggestion.clear(); + // Clear input after command submission + state.input_text.clear(); + state.cursor_position = 0; + state.auto_suggestion.clear(); - actions + if !command.trim().is_empty() { + // Parse and execute non-empty commands + use crate::components::command_panel::CommandParser; + CommandParser::parse_command(state, &command) } else { - // Even for empty commands, clear the input line - state.input_text.clear(); - state.cursor_position = 0; - state.auto_suggestion.clear(); + // Empty command - no action needed, just shows empty line in history Vec::new() } } From c0540a1542b7e15b6b776e135b79838a18f6424c Mon Sep 17 00:00:00 2001 From: swananan Date: Mon, 6 Oct 2025 01:49:48 +0800 Subject: [PATCH 8/9] feat: increase jk escape sequence timeout from 100ms to 150ms --- docs/tui-reference.md | 2 +- docs/zh/tui-reference.md | 2 +- ghostscope-ui/src/components/command_panel/optimized_input.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/tui-reference.md b/docs/tui-reference.md index 8b93a415..06686bb2 100644 --- a/docs/tui-reference.md +++ b/docs/tui-reference.md @@ -225,7 +225,7 @@ After pressing `Ctrl+R`: | Shortcut | Function | |----------|----------| -| `Esc` or `jk` | Enter Command Mode (jk must be pressed within 100ms) | +| `Esc` or `jk` | Enter Command Mode (jk must be pressed within 150ms) | ### Mode: Command Mode diff --git a/docs/zh/tui-reference.md b/docs/zh/tui-reference.md index 5f03a614..cfa02b80 100644 --- a/docs/zh/tui-reference.md +++ b/docs/zh/tui-reference.md @@ -225,7 +225,7 @@ GhostScope TUI 界面由三个面板组成，每个面板具有不同的功能 | 快捷键 | 功能 | |--------|------| -| `Esc` 或 `jk` | 进入命令模式（jk 需在 100ms 内按下）| +| `Esc` 或 `jk` | 进入命令模式（jk 需在 150ms 内按下）| ### 模式：命令模式 diff --git a/ghostscope-ui/src/components/command_panel/optimized_input.rs b/ghostscope-ui/src/components/command_panel/optimized_input.rs index b924f68d..f87eeebb 100644 --- a/ghostscope-ui/src/components/command_panel/optimized_input.rs +++ b/ghostscope-ui/src/components/command_panel/optimized_input.rs @@ -16,7 +16,7 @@ pub struct OptimizedInputHandler { impl OptimizedInputHandler { pub fn new() -> Self { Self { - jk_timeout_ms: 100, + jk_timeout_ms: 150, last_input_time: Instant::now(), } } From f0eaa0ebf31edba048dd4e928a399269afa7c1b0 Mon Sep 17 00:00:00 2001 From: swananan Date: Mon, 6 Oct 2025 01:59:54 +0800 Subject: [PATCH 9/9] fix: correct test assertion for history search escape behavior --- ghostscope-ui/tests/additional_ui_coverage_test.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ghostscope-ui/tests/additional_ui_coverage_test.rs b/ghostscope-ui/tests/additional_ui_coverage_test.rs index 7511c5ca..7ceff182 100644 --- a/ghostscope-ui/tests/additional_ui_coverage_test.rs +++ b/ghostscope-ui/tests/additional_ui_coverage_test.rs @@ -101,7 +101,8 @@ mod history_search_tests { let actions = InputHandler::handle_key_event(&mut state, esc_key); assert!(!state.is_in_history_search()); - assert!(actions + // Should NOT add empty response - would overwrite previous command's response + assert!(!actions .iter() .any(|a| matches!(a, Action::AddResponse { .. }))); }