diff --git a/Cargo.toml b/Cargo.toml index d66ab96c..2d7a583d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -442,6 +442,13 @@ debug = "line-tables-only" # location while retaining the debug information. split-debuginfo = "unpacked" +# Reduce duplicated code/unwind records in the monolithic app test binary +# to stay within macOS compact-unwind offsets. Production and dependency +# compilation retain their existing settings. +[profile.test.package.galaxy] +opt-level = 3 +codegen-units = 4 + [profile.dev.package] # Improve the performance of core terminal logic. galaxy_terminal.opt-level = 3 diff --git a/app/assets/bundled/bootstrap/bash_body.sh b/app/assets/bundled/bootstrap/bash_body.sh index c1f4b266..225980be 100644 --- a/app/assets/bundled/bootstrap/bash_body.sh +++ b/app/assets/bundled/bootstrap/bash_body.sh @@ -493,32 +493,39 @@ if [ -z "$WARP_BOOTSTRAPPED" ]; then WARP_INPUT_REPORTING_SUPPORTED=$(warp_input_reporting_supported) fi - # If we haven't already, cache information about supported features. - if [[ -z $WARP_PS1_EXPANSION_SUPPORTED ]]; then - WARP_PS1_EXPANSION_SUPPORTED=$(warp_ps1_expanding_supported) - fi - - if [[ $WARP_PS1_EXPANSION_SUPPORTED == "1" ]]; then - # When evaluating the PS1, we want to ensure that it's aware of the last exit code. - # Since we captured it already and executed multiple other commands, the actual - # last exit code has changed. So before the evaluation, we want to trick the shell - # into returning the correct value for the $? that may be in PS1 - exit_code_hack() { - return $1 - } - exit_code_hack $exit_code - deref_ps1=${WARP_PS1@P} + local honor_ps1 + local deref_ps1="" + local escaped_ps1="" + if [[ "$WARP_HONOR_PS1" == "1" ]]; then + honor_ps1="true" else - # Tricking the shell into rendering the prompt - # Note that in more modern versions of bash we could use ${PS1@P} to achieve the same, - # but MacOS comes by default with a much older version of bash, and we want to be compatible. - deref_ps1=$(echo -e "\n" | PS1="$WARP_PS1" BASH_SILENCE_DEPRECATION_WARNING=1 "$BASH" --norc -i 2>&1 | command -p head -2 | command -p tail -1) - fi + honor_ps1="false" - # Escaped PS1 variable - local escaped_ps1 - if [ "$WARP_IN_MSYS2" = false ]; then - escaped_ps1=$(warp_escape_ps1 "$(echo "$deref_ps1")") + # If we haven't already, cache information about supported features. + if [[ -z $WARP_PS1_EXPANSION_SUPPORTED ]]; then + WARP_PS1_EXPANSION_SUPPORTED=$(warp_ps1_expanding_supported) + fi + + if [[ $WARP_PS1_EXPANSION_SUPPORTED == "1" ]]; then + # When evaluating the PS1, we want to ensure that it's aware of the last exit code. + # Since we captured it already and executed multiple other commands, the actual + # last exit code has changed. So before the evaluation, we want to trick the shell + # into returning the correct value for the $? that may be in PS1 + exit_code_hack() { + return $1 + } + exit_code_hack $exit_code + deref_ps1=${WARP_PS1@P} + else + # Tricking the shell into rendering the prompt + # Note that in more modern versions of bash we could use ${PS1@P} to achieve the same, + # but MacOS comes by default with a much older version of bash, and we want to be compatible. + deref_ps1=$(echo -e "\n" | PS1="$WARP_PS1" BASH_SILENCE_DEPRECATION_WARNING=1 "$BASH" --norc -i 2>&1 | command -p head -2 | command -p tail -1) + fi + + if [ "$WARP_IN_MSYS2" = false ]; then + escaped_ps1=$(warp_escape_ps1 "$(echo "$deref_ps1")") + fi fi # Flush history @@ -661,15 +668,6 @@ if [ -z "$WARP_BOOTSTRAPPED" ]; then # Note WARP_SESSION_ID doesn't need to be escaped since it's a number # We also pass the shell's notion of `honor_ps1` to ensure it's synced correctly on the Warp-side for prompt handling. # This is passed as a "real boolean" via the JSON payload (string interpolated into JSON string below). - local honor_ps1 - if [[ "$WARP_HONOR_PS1" == "1" ]]; then - honor_ps1="true" - # The Warp prompt preview can be rendered using the active prompt in this case (which uses prompt markers). - escaped_ps1="" - deref_ps1="" - else - honor_ps1="false" - fi # We send the escaped PS1, if we are in active Warp prompt mode, for prompt preview rendering (note the shell's PS1 is unset in this case). if [ "$WARP_IN_MSYS2" = true ]; then warp_send_hook_via_kv_pairs_start "Precmd" diff --git a/app/assets/bundled/bootstrap/zsh_body.sh b/app/assets/bundled/bootstrap/zsh_body.sh index 9f8da042..c3a8cac9 100644 --- a/app/assets/bundled/bootstrap/zsh_body.sh +++ b/app/assets/bundled/bootstrap/zsh_body.sh @@ -1390,6 +1390,7 @@ esac # The main logic for generating completions. function warp_main_completer () { + (( ${+functions[_generic]} )) || return 0 # We want all the results listed. compstate[list_max]=-1 @@ -1422,12 +1423,17 @@ esac # Start by reading in the completion buffer. zle warp_read_completion_buffer - compprefuncs=( warp_mark_start_of_completions_for_compadd_override ) - comppostfuncs=( warp_mark_end_of_completions ) - COMPADD_OVERRIDE=true - zle warp_complete_via_compadd_override_internal - BUFFER="" - unset COMPADD_OVERRIDE + # Own the response frame even when compinit is missing or completion fails. + warp_mark_start_of_completions_for_compadd_override + { + local -a compprefuncs=() comppostfuncs=() + COMPADD_OVERRIDE=true + zle warp_complete_via_compadd_override_internal + } always { + unset COMPADD_OVERRIDE + warp_mark_end_of_completions + BUFFER="" + } } function warp_read_completion_buffer() { diff --git a/app/src/ai/blocklist/action_model/risk_review_tests.rs b/app/src/ai/blocklist/action_model/risk_review_tests.rs index 9e5dd634..045999fb 100644 --- a/app/src/ai/blocklist/action_model/risk_review_tests.rs +++ b/app/src/ai/blocklist/action_model/risk_review_tests.rs @@ -40,7 +40,10 @@ fn accepts_a_single_fenced_decision_but_not_empty_output() { let response = format!("{prefix}\n{{\"risk\":1,\"reason\":\"Scoped local edit\"}}\n```"); assert_eq!(Assessment::parse(&response).unwrap().risk, 1); } - assert!(Assessment::parse(" \n").unwrap_err().to_string().contains("no decision text")); + assert!(Assessment::parse(" \n") + .unwrap_err() + .to_string() + .contains("no decision text")); } fn with_pending_review( @@ -406,34 +409,61 @@ fn request_history_does_not_drop_intermediate_revocations() { fn allow_all_skips_model_review_for_unrestricted_actions() { with_pending_review(|model, conversation, action, ctx| { set_fixture_mode(model, PermissionMode::AllowAll, ctx); - assert_eq!(model.review_pending_action(&action, conversation, false, ctx), Some(true)); + assert_eq!( + model.review_pending_action(&action, conversation, false, ctx), + Some(true) + ); assert!(model.action_reviews.is_empty()); }); } #[test] fn explicit_command_approval_applies_in_every_mode_and_denylist_wins() { - for mode in [PermissionMode::ReadOnly, PermissionMode::AskFirst, PermissionMode::GalaxyDecides, PermissionMode::AllowAll] { + for mode in [ + PermissionMode::ReadOnly, + PermissionMode::AskFirst, + PermissionMode::GalaxyDecides, + PermissionMode::AllowAll, + ] { with_pending_review(move |model, conversation, mut action, ctx| { set_fixture_mode(model, mode, ctx); action.action = AIAgentActionType::RequestCommandOutput { - command: "cargo test".to_string(), is_read_only: None, is_risky: None, - rationale: None, uses_pager: None, wait_until_completion: true, citations: vec![], + command: "cargo test".to_string(), + is_read_only: None, + is_risky: None, + rationale: None, + uses_pager: None, + wait_until_completion: true, + citations: vec![], }; - model.pending_actions.insert(conversation, VecDeque::from([action.clone()])); + model + .pending_actions + .insert(conversation, VecDeque::from([action.clone()])); model.action_reviews.clear(); - let predicate = crate::settings::AgentModeCommandExecutionPredicate::new_regex("^cargo test$").unwrap(); + let predicate = + crate::settings::AgentModeCommandExecutionPredicate::new_regex("^cargo test$") + .unwrap(); AIExecutionProfilesModel::handle(ctx).update(ctx, |profiles, ctx| { - let id = *profiles.active_profile(Some(model.terminal_view_id), ctx).id(); + let id = *profiles + .active_profile(Some(model.terminal_view_id), ctx) + .id(); profiles.add_to_command_allowlist(id, &predicate, ctx); }); - assert_eq!(model.review_pending_action(&action, conversation, false, ctx), Some(true)); + assert_eq!( + model.review_pending_action(&action, conversation, false, ctx), + Some(true) + ); AIExecutionProfilesModel::handle(ctx).update(ctx, |profiles, ctx| { - let id = *profiles.active_profile(Some(model.terminal_view_id), ctx).id(); + let id = *profiles + .active_profile(Some(model.terminal_view_id), ctx) + .id(); profiles.add_to_command_denylist(id, &predicate, ctx); }); - assert_eq!(model.is_explicitly_allowlisted_command(&action, ctx), false); - assert_ne!(model.review_pending_action(&action, conversation, false, ctx), Some(true)); + assert!(!model.is_explicitly_allowlisted_command(&action, ctx)); + assert_ne!( + model.review_pending_action(&action, conversation, false, ctx), + Some(true) + ); }); } } diff --git a/app/src/ai/mcp/templatable_manager/native.rs b/app/src/ai/mcp/templatable_manager/native.rs index 2de511d9..c6603c1d 100644 --- a/app/src/ai/mcp/templatable_manager/native.rs +++ b/app/src/ai/mcp/templatable_manager/native.rs @@ -150,6 +150,7 @@ impl TemplatableMCPServerManager { // Pass the state value through as the CSRF token; rmcp will validate it // against the token it stored when generating the authorization URL. csrf_token: state.to_string(), + issuer: query_params.get("iss").map(|issuer| issuer.to_string()), }, None => CallbackResult::Error { error: error.map(|e| e.to_string()), @@ -1887,3 +1888,7 @@ impl TemplatableMCPServerManager { .contains_key(&installation_hash) } } + +#[cfg(test)] +#[path = "native_tests.rs"] +mod tests; diff --git a/app/src/ai/mcp/templatable_manager/native_tests.rs b/app/src/ai/mcp/templatable_manager/native_tests.rs new file mode 100644 index 00000000..9731efe9 --- /dev/null +++ b/app/src/ai/mcp/templatable_manager/native_tests.rs @@ -0,0 +1,37 @@ +use futures_util::stream::AbortHandle; + +use super::*; + +#[test] +fn oauth_callback_preserves_decoded_issuer() { + let mut manager = TemplatableMCPServerManager::default(); + let installation_uuid = Uuid::new_v4(); + let (abort_handle, _) = AbortHandle::new_pair(); + let (oauth_result_tx, oauth_result_rx) = async_channel::unbounded(); + manager.spawned_servers.insert( + installation_uuid, + SpawnedServerInfo { + abort_handle, + oauth_result_tx, + }, + ); + manager + .pending_oauth_csrf + .insert("test-state".to_string(), installation_uuid); + let callback_url = Url::parse( + "galaxydev://mcp/oauth2callback?code=test-code&state=test-state&iss=https%3A%2F%2Fmcp.linear.app", + ).unwrap(); + manager.handle_oauth_callback(&callback_url).unwrap(); + match oauth_result_rx.try_recv().unwrap() { + CallbackResult::Success { + code, + csrf_token, + issuer, + } => { + assert_eq!(code, "test-code"); + assert_eq!(csrf_token, "test-state"); + assert_eq!(issuer.as_deref(), Some("https://mcp.linear.app")); + } + CallbackResult::Error { error } => panic!("unexpected callback error: {error:?}"), + } +} diff --git a/app/src/terminal/input.rs b/app/src/terminal/input.rs index 0210ad51..0ef959b7 100644 --- a/app/src/terminal/input.rs +++ b/app/src/terminal/input.rs @@ -1132,6 +1132,10 @@ pub enum InputAction { CtrlR, CtrlD, Up, + /// Deferred so Up does not update InlineHistoryMenuView while it is already checked out. + SelectPreviousInlineHistoryItem, + /// Deferred so Down does not update InlineHistoryMenuView while it is already checked out. + SelectNextInlineHistoryItem, PageUp, PageDown, ClearScreen, @@ -2741,7 +2745,9 @@ impl Input { render_decorator_elements: Some(Box::new( move |app| -> EditorDecoratorElements { let terminal_model = model_clone.lock(); - let active_block = terminal_model.block_list().active_block(); + let prompt_block = terminal_model + .prompt_block() + .unwrap_or_else(|| terminal_model.block_list().active_block()); let mut editor_decorator_elements = EditorDecoratorElements::default(); @@ -2771,7 +2777,7 @@ impl Input { editor_decorator_elements.left_notch = lprompt_bottom; editor_decorator_elements.right_notch = rprompt; editor_decorator_elements.right_notch_offset_px = Some( - active_block.rprompt_render_offset( + prompt_block.rprompt_render_offset( &input_render_state_model_handle_clone .as_ref(app) .size_info, @@ -4963,6 +4969,9 @@ impl Input { let has_input = !self.editor.as_ref(ctx).buffer_text(ctx).is_empty(); let should_clear_prompt_for_search = has_input && FeatureFlag::RestorePromptOnInlineModelSelectorSearch.is_enabled(); + if !ctx.is_window_open(self.inline_model_selector_view.window_id(ctx)) { + return; + } self.inline_model_selector_view.update(ctx, |view, ctx| { if has_input && !should_clear_prompt_for_search { view.set_filter_results_by_input(false); @@ -8743,6 +8752,44 @@ impl Input { ctx.notify(); } + fn select_previous_inline_history_item(&mut self, ctx: &mut ViewContext) { + if !self + .suggestions_mode_model + .as_ref(ctx) + .is_inline_history_menu() + { + return; + } + + if self.is_cloud_mode_input_v2_composing(ctx) { + if let Some(view) = self.cloud_mode_v2_history_menu_view.clone() { + view.update(ctx, |view, ctx| view.select_up(ctx)); + } + } else { + self.inline_history_menu_view + .update(ctx, |view, ctx| view.select_up(ctx)); + } + } + + fn select_next_inline_history_item(&mut self, ctx: &mut ViewContext) { + if !self + .suggestions_mode_model + .as_ref(ctx) + .is_inline_history_menu() + { + return; + } + + if self.is_cloud_mode_input_v2_composing(ctx) { + if let Some(view) = self.cloud_mode_v2_history_menu_view.clone() { + view.update(ctx, |view, ctx| view.select_down(ctx)); + } + } else { + self.inline_history_menu_view + .update(ctx, |view, ctx| view.select_down(ctx)); + } + } + fn editor_up(&mut self, ctx: &mut ViewContext) { if self.should_show_auth_secret_ftux(ctx) { if let Some(ftux_view) = self.auth_secret_ftux_view().cloned() { @@ -8848,17 +8895,7 @@ impl Input { true } InputSuggestionsMode::InlineHistoryMenu { .. } => { - if self.is_cloud_mode_input_v2_composing(ctx) { - if let Some(view) = self.cloud_mode_v2_history_menu_view.clone() { - view.update(ctx, |view, ctx| { - view.select_up(ctx); - }); - } - } else { - self.inline_history_menu_view.update(ctx, |view, ctx| { - view.select_up(ctx); - }); - } + ctx.dispatch_typed_action_deferred(InputAction::SelectPreviousInlineHistoryItem); true } InputSuggestionsMode::IndexedReposMenu => { @@ -9249,17 +9286,7 @@ impl Input { .as_ref(ctx) .is_inline_history_menu() { - if self.is_cloud_mode_input_v2_composing(ctx) { - if let Some(view) = self.cloud_mode_v2_history_menu_view.clone() { - view.update(ctx, |view, ctx| { - view.select_down(ctx); - }); - } - } else { - self.inline_history_menu_view.update(ctx, |view, ctx| { - view.select_down(ctx); - }); - } + ctx.dispatch_typed_action_deferred(InputAction::SelectNextInlineHistoryItem); return; } @@ -11985,6 +12012,28 @@ impl Input { }), }; + let suggestions = match suggestions { + Some(suggestions) + if suggestions.suggestions.is_empty() + && use_native_shell_completions => + { + completer::suggestions( + before_cursor_text.as_str(), + cursor_position, + session_env_vars.as_ref(), + CompleterOptions { + match_strategy: matcher, + fallback_strategy: CompletionsFallbackStrategy::FilePaths, + suggest_file_path_completions_only: true, + parse_quotes_as_literals: input_type.is_ai(), + }, + &completion_context, + ) + .await + } + suggestions => suggestions, + }; + (suggestions, completions_trigger, editor_snapshot) }, |input, (suggestions, completions_trigger, editor_model), ctx| { @@ -15733,6 +15782,10 @@ impl TypedActionView for Input { match action { InputAction::FocusInputBox => self.focus_input_box(ctx), InputAction::Up => self.editor_up(ctx), + InputAction::SelectPreviousInlineHistoryItem => { + self.select_previous_inline_history_item(ctx) + } + InputAction::SelectNextInlineHistoryItem => self.select_next_inline_history_item(ctx), InputAction::PageUp => self.editor_page_up(ctx), InputAction::PageDown => self.editor_page_down(ctx), InputAction::CtrlD => self.ctrl_d(ctx), diff --git a/app/src/terminal/input_tests.rs b/app/src/terminal/input_tests.rs index dbe2fc27..3ab4b059 100644 --- a/app/src/terminal/input_tests.rs +++ b/app/src/terminal/input_tests.rs @@ -2269,6 +2269,129 @@ fn slash_compact_still_queues_while_in_progress() { }); } +fn selected_inline_history_command( + history_menu: &warpui::ViewHandle, + app: &App, +) -> Option { + history_menu.read(app, |view, ctx| { + view.model() + .as_ref(ctx) + .selected_item() + .and_then(|item| item.buffer_replacement_text().cloned()) + }) +} + +#[test] +fn history_up_does_not_reenter_inline_history_menu_update() { + let _inline_history_menu = FeatureFlag::InlineHistoryMenu.override_enabled(true); + App::test((), |mut app| async move { + initialize_app(&mut app); + + let terminal = add_window_with_bootstrapped_terminal( + &mut app, + Some(vec!["cd ~".to_string(), "ls".to_string()]), + None, + ) + .await; + let input = terminal.read(&app, |view, _| view.input().clone()); + let session_id = input + .read(&app, |input, _| input.active_block_session_id()) + .expect("bootstrapped input should have a session id"); + simulate_directory_for_completion(session_id, &terminal, &mut app, "/tmp"); + input.update(&mut app, |input, ctx| { + input.open_inline_history_menu(ctx); + }); + input.read(&app, |input, ctx| { + assert!(input + .suggestions_mode_model + .as_ref(ctx) + .is_inline_history_menu()); + }); + + let inline_history_menu = + input.read(&app, |input, _| input.inline_history_menu_view.clone()); + let result_count = inline_history_menu.read(&app, |view, ctx| view.result_count(ctx)); + assert!( + result_count >= 2, + "inline history should list both seeded commands, got {result_count}" + ); + let selected_before = selected_inline_history_command(&inline_history_menu, &app); + let buffer_before = input.read(&app, |input, ctx| input.buffer_text(ctx).to_owned()); + assert_eq!(selected_before.as_deref(), Some("ls")); + assert_eq!(buffer_before, "ls"); + + inline_history_menu.update(&mut app, |_, ctx| { + input.update(ctx, |input, ctx| { + input.handle_action(&InputAction::Up, ctx); + }); + }); + input.read(&app, |input, ctx| { + assert!( + input + .suggestions_mode_model + .as_ref(ctx) + .is_inline_history_menu(), + "History/Up must still show inline history after a nested checkout" + ); + assert_eq!(input.buffer_text(ctx), "cd ~"); + }); + assert_eq!( + selected_inline_history_command(&inline_history_menu, &app).as_deref(), + Some("cd ~") + ); + + inline_history_menu.update(&mut app, |_, ctx| { + input.update(ctx, |input, ctx| input.editor_down(ctx)); + }); + input.read(&app, |input, ctx| { + assert!(input + .suggestions_mode_model + .as_ref(ctx) + .is_inline_history_menu()); + assert_eq!(input.buffer_text(ctx), "ls"); + }); + assert_eq!( + selected_inline_history_command(&inline_history_menu, &app).as_deref(), + Some("ls") + ); + }); +} + +#[test] +fn editor_down_does_not_reenter_inline_history_menu_update() { + let _inline_history_menu = FeatureFlag::InlineHistoryMenu.override_enabled(true); + App::test((), |mut app| async move { + initialize_app(&mut app); + + let terminal = add_window_with_bootstrapped_terminal(&mut app, None, None).await; + let input = terminal.read(&app, |view, _| view.input().clone()); + input.update(&mut app, |input, ctx| { + input.open_inline_history_menu(ctx); + }); + input.read(&app, |input, ctx| { + assert!(input + .suggestions_mode_model + .as_ref(ctx) + .is_inline_history_menu()); + }); + + let inline_history_menu = + input.read(&app, |input, _| input.inline_history_menu_view.clone()); + inline_history_menu.update(&mut app, |_, ctx| { + input.update(ctx, |input, ctx| input.editor_down(ctx)); + }); + input.read(&app, |input, ctx| { + assert!( + !input + .suggestions_mode_model + .as_ref(ctx) + .is_inline_history_menu(), + "Down on empty inline history must close the menu after the nested checkout ends" + ); + }); + }); +} + #[test] fn test_history_up_multiline() { App::test((), |mut app| async move { diff --git a/app/src/terminal/model/block.rs b/app/src/terminal/model/block.rs index 9e322a4a..005df9e4 100644 --- a/app/src/terminal/model/block.rs +++ b/app/src/terminal/model/block.rs @@ -2007,10 +2007,16 @@ impl Block { /// Returns the offset, in pixels, at which the rprompt should be rendered /// relative to the prompt. pub fn rprompt_render_offset(&self, size: &SizeInfo) -> Vector2F { + let right_margin = match self.shell_host.as_ref().map(|shell| shell.shell_type) { + Some(ShellType::Fish | ShellType::PowerShell) => 0, + // TODO: Honor ZLE_RPROMPT_INDENT instead of assuming zsh's default. + Some(ShellType::Zsh | ShellType::Bash) | None => 1, + }; let rprompt_width_cells = self.rprompt_grid.grid_storage().max_cursor_point.col; let rprompt_width_px = rprompt_width_cells as f32 * size.cell_width_px.as_f32(); Vector2F::new( - (self.prompt_grid_columns().saturating_sub(1) as f32 * size.cell_width_px().as_f32()) + (self.prompt_grid_columns().saturating_sub(right_margin) as f32 + * size.cell_width_px().as_f32()) - rprompt_width_px, self.prompt_number_of_rows().saturating_sub(1) as f32 * size.cell_height_px().as_f32(), ) diff --git a/app/src/terminal/model/grid/ansi_handler.rs b/app/src/terminal/model/grid/ansi_handler.rs index 521fa80b..5f96fe3a 100644 --- a/app/src/terminal/model/grid/ansi_handler.rs +++ b/app/src/terminal/model/grid/ansi_handler.rs @@ -8,9 +8,9 @@ mod tab_stops; use std::cmp::min; use std::collections::HashMap; -use std::io; use std::ops::Range; use std::sync::Arc; +use std::{io, mem}; use base64::Engine as _; use bounded_vec_deque::BoundedVecDeque; @@ -208,7 +208,21 @@ impl ansi::Handler for GridHandler { col = col.saturating_sub(1); } - self.grid[row][col].push_zerowidth(c, /* log_long_grapheme_warnings */ true); + let old_cell_content_width = match self.grid[row][col].raw_content() { + CharOrStr::Str(s) => s.width(), + CharOrStr::Char(c) => match c.width() { + Some(width) => width, + None => { + return; + } + }, + }; + + // A rejected append cannot change the cell's width, so there is no structural work to do. + if !self.grid[row][col].push_zerowidth(c, /* log_long_grapheme_warnings */ true) { + return; + } + let cell_content_width = match self.grid[row][col].raw_content() { CharOrStr::Str(s) => s.width(), // Note that we should never reach here since we are pushing a zerowidth character, @@ -225,19 +239,29 @@ impl ansi::Handler for GridHandler { // Bash and Fish support emoji variation selectors, but Zsh does not in bracketed paste // mode. Specifically, this references sequences such as \0x2601\0xFE0F (☁️), // which are commonly used in prompts e.g. GCloud prompt chip in Starship. - if cell_content_width == 2 + if old_cell_content_width != cell_content_width + && cell_content_width == 2 && self.ansi_handler_state.supports_emoji_presentation_selector { - // Current cursor cell contains a wide character (double-width). - self.grid[row][col].flags.insert(Flags::WIDE_CHAR); + let mut wide_cell = mem::take(&mut self.grid[row][col]); + if col + 1 == num_cols + && !self.ansi_handler_state.mode.contains(TermMode::LINE_WRAP) + { + // Remove the selector rather than leave a wide grapheme without room for its spacer. + let popped = wide_cell.pop_zerowidth(); + debug_assert_eq!(popped, Some(c)); + self.grid[row][col] = wide_cell; + return; + } + wide_cell.flags.remove( + Flags::WIDE_CHAR_SPACER | Flags::LEADING_WIDE_CHAR_SPACER | Flags::WRAPLINE, + ); + let base_char = wide_cell.c; - // Insert spacer at the next cell. - self.write_at_cursor(cell::DEFAULT_CHAR) - .flags - .insert(Flags::WIDE_CHAR_SPACER); - - // Update cursor appropriately before early-return. - self.advance_cursor_by_one_cell(); + self.update_cursor(|cursor| { + cursor.point.col = col; + }); + self.write_wide_char(base_char, |cell| *cell = wide_cell); } return; } @@ -275,37 +299,10 @@ impl ansi::Handler for GridHandler { if width == 1 { self.write_at_cursor(c); + self.advance_cursor_by_one_cell(); } else { - if self.grid.cursor().point.col + 1 >= num_cols { - if self.ansi_handler_state.mode.contains(TermMode::LINE_WRAP) { - // Insert placeholder before wide char if glyph does not fit in this row. - self.write_at_cursor(cell::DEFAULT_CHAR) - .flags - .insert(Flags::LEADING_WIDE_CHAR_SPACER); - self.wrapline(); - } else { - // Prevent out of bounds crash when linewrapping is disabled. - self.move_cursor_forward(|cursor| { - cursor.input_needs_wrap = true; - }); - return; - } - } - - // Write full width glyph to current cursor cell. - self.write_at_cursor(c).flags.insert(Flags::WIDE_CHAR); - - // Write spacer to cell following the wide glyph. - self.move_cursor_forward(|cursor| { - cursor.point.col += 1; - }); - - self.write_at_cursor(cell::DEFAULT_CHAR) - .flags - .insert(Flags::WIDE_CHAR_SPACER); + self.write_wide_char(c, |_| {}); } - - self.advance_cursor_by_one_cell(); } fn goto(&mut self, row: VisibleRow, column: usize) { @@ -1470,6 +1467,33 @@ impl GridHandler { }); } + fn write_wide_char(&mut self, c: char, update_wide_cell: impl FnOnce(&mut Cell)) { + let num_cols = self.columns(); + if self.grid.cursor().point.col + 1 >= num_cols { + if !self.ansi_handler_state.mode.contains(TermMode::LINE_WRAP) { + self.move_cursor_forward(|cursor| { + cursor.input_needs_wrap = true; + }); + return; + } + + let leading_spacer = self.write_at_cursor(cell::DEFAULT_CHAR); + leading_spacer.flags.insert(Flags::LEADING_WIDE_CHAR_SPACER); + self.wrapline(); + } + + let wide_cell = self.write_at_cursor(c); + update_wide_cell(wide_cell); + wide_cell.flags.insert(Flags::WIDE_CHAR); + + self.move_cursor_forward(|cursor| { + cursor.point.col += 1; + }); + let spacer = self.write_at_cursor(cell::DEFAULT_CHAR); + spacer.flags.insert(Flags::WIDE_CHAR_SPACER); + self.advance_cursor_by_one_cell(); + } + /// Insert a linebreak at the current cursor position. #[inline] pub(crate) fn wrapline(&mut self) { diff --git a/app/src/terminal/model/grid/grid_handler_tests.rs b/app/src/terminal/model/grid/grid_handler_tests.rs index 1500cbc5..4f24bbe0 100644 --- a/app/src/terminal/model/grid/grid_handler_tests.rs +++ b/app/src/terminal/model/grid/grid_handler_tests.rs @@ -2535,3 +2535,76 @@ fn test_full_grid_clear_resize_then_bounds_to_string_does_not_panic() { ); } } + +#[test] +fn emoji_variation_selector_wraps_width_promotion_at_last_column() { + let mut grid = GridHandler::new_for_test(2, 4); + grid.input('a'); + grid.input('b'); + grid.input('c'); + grid.input('\u{26A0}'); + + grid.input('\u{FE0F}'); + + let storage = grid.grid_storage(); + assert_eq!(storage[VisibleRow(0)][3].c, '\0'); + assert!(storage[VisibleRow(0)][3] + .flags + .contains(Flags::LEADING_WIDE_CHAR_SPACER)); + assert!(storage[VisibleRow(0)][3].flags.contains(Flags::WRAPLINE)); + assert!(matches!( + storage[VisibleRow(1)][0].content_for_display(), + CharOrStr::Str("⚠️") + )); + assert!(storage[VisibleRow(1)][0].flags.contains(Flags::WIDE_CHAR)); + assert!(storage[VisibleRow(1)][1] + .flags + .contains(Flags::WIDE_CHAR_SPACER)); + assert_eq!(grid.cursor_point(), Point::new(1, 2)); +} + +#[test] +fn emoji_variation_selector_drops_width_promotion_when_line_wrap_is_disabled() { + let mut grid = GridHandler::new_for_test(2, 4); + grid.unset_mode(ansi::Mode::LineWrap); + grid.input('a'); + grid.input('b'); + grid.input('c'); + grid.input('\u{26A0}'); + + grid.input('\u{FE0F}'); + + let storage = grid.grid_storage(); + assert!(matches!( + storage[VisibleRow(0)][3].content_for_display(), + CharOrStr::Char('⚠') + )); + assert!(!storage[VisibleRow(0)][3] + .flags + .intersects(Flags::WIDE_CHAR | Flags::WIDE_CHAR_SPACER | Flags::LEADING_WIDE_CHAR_SPACER)); + assert_eq!(grid.cursor_point(), Point::new(0, 3)); +} + +#[test] +fn emoji_variation_selector_does_not_advance_already_wide_grapheme() { + let mut grid = GridHandler::new_for_test(2, 6); + grid.input('😀'); + + grid.input('\u{FE0F}'); + grid.input('x'); + + let storage = grid.grid_storage(); + assert!(matches!( + storage[VisibleRow(0)][0].content_for_display(), + CharOrStr::Str("😀️") + )); + assert!(storage[VisibleRow(0)][0].flags.contains(Flags::WIDE_CHAR)); + assert!(storage[VisibleRow(0)][1] + .flags + .contains(Flags::WIDE_CHAR_SPACER)); + assert_eq!(storage[VisibleRow(0)][2].c, 'x'); + assert!(!storage[VisibleRow(0)][2] + .flags + .contains(Flags::WIDE_CHAR_SPACER)); + assert_eq!(grid.cursor_point(), Point::new(0, 3)); +} diff --git a/app/src/undo_close/stack.rs b/app/src/undo_close/stack.rs index d458a429..91ea7440 100644 --- a/app/src/undo_close/stack.rs +++ b/app/src/undo_close/stack.rs @@ -139,7 +139,10 @@ impl ClosedItem { fn clean_up_pane_group(pane_group: &ViewHandle, ctx: &mut AppContext) { let window_id = pane_group.window_id(ctx); - if !ctx.is_window_open(window_id) { + if ctx + .view_with_id::(window_id, pane_group.id()) + .is_none() + { return; } diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index 42e3720f..89fc13b7 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -2765,6 +2765,9 @@ impl Workspace { fn sync_settings_error_state_into_settings_pane(&mut self, ctx: &mut ViewContext) { let error = self.settings_file_error.clone(); let dismissed = self.settings_error_banner_dismissed; + if self.settings_pane.try_as_ref(ctx).is_none() { + return; + } self.settings_pane.update(ctx, |view, ctx| { view.set_settings_error_state(error, dismissed, ctx); }); @@ -9799,6 +9802,14 @@ impl Workspace { self.show_tab_group_right_click_menu = None; self.show_tab_selection_right_click_menu = None; self.hide_move_to_group_sidecar(ctx); + if !self.current_workspace_state.is_tab_being_renamed() + && !self.current_workspace_state.is_any_pane_being_renamed() + && !self + .current_workspace_state + .is_any_tab_group_being_renamed() + { + self.focus_active_tab(ctx); + } ctx.notify(); } MenuEvent::ItemHovered | MenuEvent::ItemSelected => { @@ -11747,6 +11758,7 @@ impl Workspace { } else { let matching = self.vertical_tabs_panel.matching_tab_indices( &self.tabs, + &self.tab_groups, self.active_tab_index, ctx, ); @@ -11771,6 +11783,7 @@ impl Workspace { } else { let matching = self.vertical_tabs_panel.matching_tab_indices( &self.tabs, + &self.tab_groups, self.active_tab_index, ctx, ); diff --git a/app/src/workspace/view/vertical_tabs.rs b/app/src/workspace/view/vertical_tabs.rs index 530bf137..962958a3 100644 --- a/app/src/workspace/view/vertical_tabs.rs +++ b/app/src/workspace/view/vertical_tabs.rs @@ -1,7 +1,7 @@ pub mod telemetry; use std::cell::RefCell; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; @@ -17,9 +17,9 @@ use galaxyui::elements::{ resizable_state_handle, Border, ChildAnchor, Clipped, ClippedScrollStateHandle, ClippedScrollable, ConstrainedBox, Container, CornerRadius, CrossAxisAlignment, DispatchEventResult, DragAxis, DragBarSide, Draggable, DropShadow, DropTarget, Element, Empty, - EventHandler, Expanded, Fill as ElementFill, Flex, Hoverable, MainAxisAlignment, MainAxisSize, - MouseStateHandle, OffsetPositioning, Padding, ParentAnchor, ParentElement, ParentOffsetBounds, - PositionedElementAnchor, PositionedElementOffsetBounds, Radius, Resizable, + EventHandler, Expanded, Fill as ElementFill, Flex, Highlight, Hoverable, MainAxisAlignment, + MainAxisSize, MouseStateHandle, OffsetPositioning, Padding, ParentAnchor, ParentElement, + ParentOffsetBounds, PositionedElementAnchor, PositionedElementOffsetBounds, Radius, Resizable, ResizableStateHandle, SavePosition, ScrollTarget, ScrollToPositionMode, ScrollbarWidth, Shrinkable, Stack, Text, }; @@ -1165,12 +1165,18 @@ impl VerticalTabsPanelState { }); } - /// Returns the indices (in original order) of tab groups that have at least - /// one pane matching the current search query. Returns all indices when the - /// query is empty. + /// Returns the indices (in original order) of tabs that match the current + /// search query, either through their own text or by belonging to a tab + /// group whose displayed name matches. Returns all indices when the query + /// is empty. + /// + /// This drives tab cycling under an active search, so it must admit exactly + /// the tabs `render_groups` renders — otherwise the panel would show tabs + /// the next/previous-tab keybindings refuse to visit. pub(super) fn matching_tab_indices( &self, tabs: &[TabData], + tab_groups: &HashMap, active_tab_index: usize, app: &AppContext, ) -> Vec { @@ -1178,6 +1184,7 @@ impl VerticalTabsPanelState { return (0..tabs.len()).collect(); } let query_lower = self.search_query.to_lowercase(); + let matched_groups = matched_group_ids(tab_groups, &query_lower); let resolved_mode = resolve_vertical_tabs_mode(app); let display_granularity = match resolved_mode { VerticalTabsResolvedMode::Panes => VerticalTabsDisplayGranularity::Panes, @@ -1188,6 +1195,10 @@ impl VerticalTabsPanelState { tabs.iter() .enumerate() .filter(|(tab_index, tab)| { + // A group-name match admits every member, regardless of its own text. + if tab_admitted_by_group_name(tab.group_id, &matched_groups) { + return true; + } let pane_group = tab.pane_group.as_ref(app); let visible_pane_ids = pane_group.visible_pane_ids(); match resolved_mode { @@ -1780,7 +1791,7 @@ fn render_groups( .collect() } else { let query_lower = query.to_lowercase(); - workspace + let own_matches: Vec<(usize, Option>)> = workspace .tabs .iter() .enumerate() @@ -1881,7 +1892,15 @@ fn render_groups( } } }) - .collect() + .collect(); + + // A query matching a group's name reveals every tab under that group, + // even members whose own text does not match. + let matched_groups = matched_group_ids(&workspace.tab_groups, &query_lower); + let tab_group_ids: Vec> = + workspace.tabs.iter().map(|tab| tab.group_id).collect(); + + merge_group_name_matches(&tab_group_ids, &matched_groups, own_matches) }; if visible_tabs.is_empty() { @@ -1929,7 +1948,14 @@ fn render_groups( .get(&gid) .map(|group| (gid, group.clone())) }) { - Some((group_id, group)) => { + Some((group_id, mut group)) => { + // While a search is active, render every surviving group + // expanded so its matches are visible without a click. This + // only mutates the local clone — the stored `collapsed` flag is + // untouched, so clearing the query restores the real state. + if !query.is_empty() { + group.collapsed = false; + } // Members are a contiguous subslice of `visible_tabs`. let run_len = visible_tabs[i..] .iter() @@ -2737,6 +2763,7 @@ fn render_tab_group_header_icon_button( #[allow(clippy::too_many_arguments)] fn render_grouped_tabs_header( group: &TabGroup, + search_query: &str, member_count: usize, mouse_states: &TabGroupMouseStates, is_collapsed: bool, @@ -2745,9 +2772,9 @@ fn render_grouped_tabs_header( is_being_renamed: bool, rename_editor: Option<&ViewHandle>, collapsed_member_kinds: Option<&[SummaryPaneKind]>, - app: &AppContext, + ctx: &AppContext, ) -> Box { - let appearance = Appearance::as_ref(app); + let appearance = Appearance::as_ref(ctx); let theme = appearance.theme(); let font_family = appearance.ui_font_family(); let main_text_color = theme.main_text_color(theme.background()); @@ -2785,15 +2812,17 @@ fn render_grouped_tabs_header( let title_element: Box = if let Some(editor) = rename_editor.filter(|_| is_being_renamed) { - render_inline_tab_rename_editor(editor, appearance, app) + render_inline_tab_rename_editor(editor, appearance, ctx) } else { - let title_text = group - .name - .clone() - .unwrap_or_else(|| "New Group".to_string()); + let title_text = group_display_name(group); + let highlight_indices = group_name_highlight_indices(&title_text, search_query); Text::new_inline(title_text, font_family, 12.) .with_clip(ClipConfig::ellipsis()) .with_color(main_text_color.into()) + .with_single_highlight( + Highlight::new().with_properties(Properties::default().weight(Weight::Bold)), + highlight_indices, + ) .finish() }; let subtitle_text = if member_count == 1 { @@ -3012,6 +3041,7 @@ fn render_grouped_tab_container( is_collapsed.then(|| workspace.compute_group_member_kinds(group.id, app)); let header = render_grouped_tabs_header( &group, + &state.search_query, member_count, &mouse_states, is_collapsed, @@ -3908,6 +3938,102 @@ fn uses_outer_group_container(display_granularity: VerticalTabsDisplayGranularit matches!(display_granularity, VerticalTabsDisplayGranularity::Panes) } +/// Header text for a tab group the user has never named. +const UNTITLED_GROUP_NAME: &str = "New Group"; + +/// The group title as displayed in the panel header, including the fallback +/// used for a group the user has never named. Search matches against this so a +/// query matches what is actually on screen. +fn group_display_name(group: &TabGroup) -> String { + group + .name + .clone() + .unwrap_or_else(|| UNTITLED_GROUP_NAME.to_string()) +} + +fn group_name_highlight_indices(text: &str, query: &str) -> Vec { + if query.is_empty() { + return Vec::new(); + } + + let text_lower = text.to_lowercase(); + let query_lower = query.to_lowercase(); + let matches: Vec<_> = text_lower + .char_indices() + .filter(|(start, _)| text_lower[*start..].starts_with(&query_lower)) + .map(|(start, _)| start..start + query_lower.len()) + .collect(); + let mut indices = Vec::new(); + let mut offset = 0; + // Lowercasing can expand a character, so match bytes must map back to original char indices. + for (index, character) in text.chars().enumerate() { + let end = offset + character.to_lowercase().map(char::len_utf8).sum::(); + if matches + .iter() + .any(|range| range.start < end && offset < range.end) + { + indices.push(index); + } + offset = end; + } + indices +} + +/// Returns the ids of tab groups whose displayed name contains `query_lower`. +fn matched_group_ids( + tab_groups: &HashMap, + query_lower: &str, +) -> HashSet { + tab_groups + .iter() + .filter(|(_, group)| { + group_display_name(group) + .to_lowercase() + .contains(query_lower) + }) + .map(|(group_id, _)| *group_id) + .collect() +} + +/// Whether a tab is admitted by its group's name matching the query, rather +/// than by its own text. Ungrouped tabs are never admitted this way. +fn tab_admitted_by_group_name( + group_id: Option, + matched_groups: &HashSet, +) -> bool { + group_id.is_some_and(|id| matched_groups.contains(&id)) +} + +/// Merges text matches with members of name-matched tab groups while preserving tab order. +/// Members admitted by group name use `None` so the full tab is shown. +fn merge_group_name_matches( + tab_group_ids: &[Option], + matched_groups: &HashSet, + own_matches: Vec<(usize, Option>)>, +) -> Vec<(usize, Option>)> { + if matched_groups.is_empty() { + return own_matches; + } + + let mut merged: Vec<(usize, Option>)> = Vec::with_capacity(own_matches.len()); + let mut own_matches = own_matches.into_iter().peekable(); + + for (tab_index, group_id) in tab_group_ids.iter().enumerate() { + let in_matched_group = tab_admitted_by_group_name(*group_id, matched_groups); + let own_match = own_matches.next_if(|(index, _)| *index == tab_index); + + match (in_matched_group, own_match) { + // The group name matched, so the whole tab is shown regardless of + // whether it also matched on its own text. + (true, _) => merged.push((tab_index, None)), + (false, Some(own_match)) => merged.push(own_match), + (false, None) => {} + } + } + + merged +} + fn search_fragments_contain_query(fragments: &[String], query_lower: &str) -> bool { fragments .iter() diff --git a/app/src/workspace/view/vertical_tabs_tests.rs b/app/src/workspace/view/vertical_tabs_tests.rs index 63d9af7d..a70586ca 100644 --- a/app/src/workspace/view/vertical_tabs_tests.rs +++ b/app/src/workspace/view/vertical_tabs_tests.rs @@ -1,3 +1,4 @@ +use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use galaxyui::elements::PositionedElementOffsetBounds; @@ -8,13 +9,14 @@ use pathfinder_geometry::vector::Vector2F; use super::{ branch_label_display, coalesce_summary_branch_entries, code_detail_kind_label, compact_branch_subtitle_display, detail_sidecar_width_and_bounds, - detail_target_for_hovered_row, non_terminal_search_text_fragments, + detail_target_for_hovered_row, group_display_name, group_name_highlight_indices, + matched_group_ids, merge_group_name_matches, non_terminal_search_text_fragments, pane_ids_for_display_granularity, pane_search_text_fragments, preferred_agent_tab_titles, push_normalized_unique_summary_label, search_fragments_contain_query, select_summary_pane_kind_icons, should_keep_detail_sidecar_visible_for_mouse_position, sort_summary_primary_labels_status_first, summary_overflow_count, - summary_search_text_fragments, terminal_kind_badge_label, terminal_primary_line_data, - terminal_pull_request_badge_label, terminal_search_text_fragments, + summary_search_text_fragments, tab_admitted_by_group_name, terminal_kind_badge_label, + terminal_primary_line_data, terminal_pull_request_badge_label, terminal_search_text_fragments, terminal_title_fallback_font, uses_outer_group_container, visible_pane_ids_for_detail_target, vtab_diff_stats_text, AgentTabTextPreference, SummaryPaneKind, SummaryPaneKindIcons, TerminalAgentText, TerminalPrimaryLineData, TerminalPrimaryLineFont, VerticalTabsDetailTarget, @@ -27,6 +29,7 @@ use crate::pane_group::pane::IPaneType; use crate::pane_group::{PaneId, TerminalPaneId}; use crate::safe_triangle::SafeTriangle; use crate::terminal::CLIAgent; +use crate::workspace::tab_group::{TabGroup, TabGroupId}; use crate::workspace::tab_settings::VerticalTabsDisplayGranularity; fn label(text: &str) -> VerticalTabsSummaryPrimaryLabel { @@ -1151,3 +1154,236 @@ fn summary_search_fragments_include_hidden_overflow_values() { assert!(search_fragments_contain_query(&fragments, "+2")); assert!(search_fragments_contain_query(&fragments, "-3")); } + +fn tab_group(name: Option<&str>) -> TabGroup { + TabGroup { + name: name.map(str::to_string), + ..TabGroup::new() + } +} + +#[test] +fn group_display_name_uses_the_group_name_when_set() { + assert_eq!(group_display_name(&tab_group(Some("backend"))), "backend"); +} + +#[test] +fn group_display_name_falls_back_to_the_untitled_header_text() { + assert_eq!(group_display_name(&tab_group(None)), "New Group"); +} + +#[test] +fn group_name_match_includes_members_that_did_not_match_on_their_own() { + let group = TabGroupId::new(); + let tab_group_ids = vec![Some(group), Some(group), None]; + + let merged = merge_group_name_matches( + &tab_group_ids, + &HashSet::from([group]), + // Only the second tab matched on its own title. + vec![(1, None)], + ); + + assert_eq!(merged, vec![(0, None), (1, None)]); +} + +#[test] +fn group_name_match_keeps_results_ordered_by_tab_index() { + let group = TabGroupId::new(); + // Ungrouped tabs on either side of the group. + let tab_group_ids = vec![None, Some(group), Some(group), None]; + + let merged = merge_group_name_matches( + &tab_group_ids, + &HashSet::from([group]), + vec![(0, None), (3, None)], + ); + + assert_eq!( + merged, + vec![(0, None), (1, None), (2, None), (3, None)], + "group members must be interleaved in tab order, not appended" + ); +} + +#[test] +fn group_name_match_upgrades_a_pane_filtered_member_to_all_panes() { + let group = TabGroupId::new(); + let tab_group_ids = vec![Some(group)]; + + let merged = merge_group_name_matches( + &tab_group_ids, + &HashSet::from([group]), + // The tab matched on one pane row only. + vec![(0, Some(vec![pane_id()]))], + ); + + assert_eq!( + merged, + vec![(0, None)], + "a group-name match shows the whole tab, not a pane-filtered slice" + ); +} + +#[test] +fn group_name_match_for_a_group_with_no_members_changes_nothing() { + let empty_group = TabGroupId::new(); + let tab_group_ids = vec![None, None]; + + let merged = merge_group_name_matches( + &tab_group_ids, + &HashSet::from([empty_group]), + vec![(1, None)], + ); + + assert_eq!(merged, vec![(1, None)]); +} + +#[test] +fn group_name_match_leaves_tabs_outside_the_group_filtered() { + let matched = TabGroupId::new(); + let other = TabGroupId::new(); + let tab_group_ids = vec![Some(matched), Some(other), None]; + + let merged = merge_group_name_matches(&tab_group_ids, &HashSet::from([matched]), vec![]); + + assert_eq!( + merged, + vec![(0, None)], + "only members of the name-matched group are force-included" + ); +} + +#[test] +fn no_group_name_match_leaves_the_filtered_tabs_untouched() { + let group = TabGroupId::new(); + let tab_group_ids = vec![Some(group), None]; + let own_matches = vec![(1, Some(vec![pane_id()]))]; + + let merged = merge_group_name_matches(&tab_group_ids, &HashSet::new(), own_matches.clone()); + + assert_eq!(merged, own_matches); +} + +fn tab_groups_map(groups: Vec) -> HashMap { + groups.into_iter().map(|group| (group.id, group)).collect() +} + +#[test] +fn matched_group_ids_selects_groups_whose_displayed_name_contains_the_query() { + let backend = tab_group(Some("backend")); + let frontend = tab_group(Some("frontend")); + let backend_id = backend.id; + let groups = tab_groups_map(vec![backend, frontend]); + + assert_eq!( + matched_group_ids(&groups, "backend"), + HashSet::from([backend_id]) + ); +} + +#[test] +fn matched_group_ids_is_case_insensitive() { + let group = tab_group(Some("Backend Services")); + let id = group.id; + let groups = tab_groups_map(vec![group]); + + assert_eq!(matched_group_ids(&groups, "backend"), HashSet::from([id])); +} + +#[test] +fn matched_group_ids_matches_the_untitled_group_placeholder() { + let group = tab_group(None); + let id = group.id; + let groups = tab_groups_map(vec![group]); + + assert_eq!(matched_group_ids(&groups, "new group"), HashSet::from([id])); +} + +#[test] +fn matched_group_ids_is_empty_when_nothing_matches() { + let groups = tab_groups_map(vec![tab_group(Some("backend"))]); + + assert!(matched_group_ids(&groups, "nomatch").is_empty()); +} + +#[test] +fn tab_is_admitted_by_group_name_only_when_its_group_matched() { + let matched = TabGroupId::new(); + let other = TabGroupId::new(); + let matched_groups = HashSet::from([matched]); + + assert!(tab_admitted_by_group_name(Some(matched), &matched_groups)); + assert!(!tab_admitted_by_group_name(Some(other), &matched_groups)); + assert!( + !tab_admitted_by_group_name(None, &matched_groups), + "an ungrouped tab is never admitted by a group-name match" + ); +} + +#[test] +fn group_name_highlights_case_insensitive_substrings() { + assert_eq!( + group_name_highlight_indices("My Backend", "BACK"), + vec![3, 4, 5, 6] + ); +} + +#[test] +fn group_name_highlights_repeated_matches() { + assert_eq!( + group_name_highlight_indices("dev / DEV", "dev"), + vec![0, 1, 2, 6, 7, 8] + ); +} + +#[test] +fn group_name_highlights_overlapping_matches() { + assert_eq!( + group_name_highlight_indices("banana", "ana"), + vec![1, 2, 3, 4, 5] + ); +} + +#[test] +fn group_name_highlights_use_character_indices_after_multibyte_text() { + assert_eq!(group_name_highlight_indices("🚀 Équipe", "éq"), vec![2, 3]); +} + +#[test] +fn group_name_highlights_map_expanding_lowercase_to_original_characters() { + assert_eq!( + group_name_highlight_indices("İ Backend", "back"), + vec![2, 3, 4, 5] + ); + assert_eq!(group_name_highlight_indices("İ Backend", "i"), vec![0]); +} + +#[test] +fn group_name_highlights_preserve_contextual_lowercasing() { + assert_eq!(group_name_highlight_indices("ΟΣ", "ος"), vec![0, 1]); +} + +#[test] +fn group_name_highlights_clear_for_empty_query() { + assert_eq!( + group_name_highlight_indices("Backend", ""), + Vec::::new() + ); +} + +#[test] +fn group_name_highlights_do_not_use_fuzzy_matching() { + assert_eq!( + group_name_highlight_indices("Backend", "bkd"), + Vec::::new() + ); +} + +#[test] +fn group_name_highlights_match_untitled_fallback() { + assert_eq!( + group_name_highlight_indices(&group_display_name(&tab_group(None)), "group"), + vec![4, 5, 6, 7, 8] + ); +} diff --git a/app/src/workspace/view_tests.rs b/app/src/workspace/view_tests.rs index d43f82e6..1e00d060 100644 --- a/app/src/workspace/view_tests.rs +++ b/app/src/workspace/view_tests.rs @@ -48,6 +48,7 @@ use crate::cloud_object::model::view::CloudViewModel; use crate::context_chips::prompt::Prompt; use crate::editor::Event; use crate::gpu_state::GPUState; +use crate::menu::MenuAction; use crate::network::NetworkStatus; use crate::notebooks::editor::keys::NotebookKeybindings; use crate::notebooks::notebook::NotebookView; @@ -1638,6 +1639,140 @@ fn test_close_other_tabs_confirmation_dialog() { }); } +#[test] +fn test_closing_tab_context_menu_restores_active_tab_focus() { + App::test((), |mut app| async move { + initialize_app(&mut app); + + let workspace = mock_workspace(&mut app); + let (window_id, menu) = workspace.update(&mut app, |workspace, ctx| { + workspace.toggle_tab_right_click_menu( + 0, + TabContextMenuAnchor::Pointer(Vector2F::zero()), + ctx, + ); + (ctx.window_id(), workspace.tab_right_click_menu.clone()) + }); + assert_eq!(app.focused_view_id(window_id), Some(menu.id())); + + menu.update(&mut app, |menu, ctx| { + menu.handle_action(&MenuAction::Close(false), ctx); + }); + workspace.update(&mut app, |workspace, ctx| { + assert!(workspace.show_tab_right_click_menu.is_none()); + assert!(workspace + .active_tab_pane_group() + .is_self_or_child_focused(ctx)); + }); + }); +} + +#[test] +fn test_selecting_rename_from_tab_context_menu_preserves_editor_focus() { + App::test((), |mut app| async move { + initialize_app(&mut app); + + let workspace = mock_workspace(&mut app); + let menu = workspace.update(&mut app, |workspace, ctx| { + workspace.toggle_tab_right_click_menu( + 0, + TabContextMenuAnchor::Pointer(Vector2F::zero()), + ctx, + ); + workspace.tab_right_click_menu.clone() + }); + + menu.update(&mut app, |menu, ctx| { + assert!(menu.set_selected_by_name("Rename tab", ctx)); + menu.handle_action(&MenuAction::Enter, ctx); + }); + + workspace.read(&app, |workspace, ctx| { + assert!(workspace.show_tab_right_click_menu.is_none()); + assert_eq!( + workspace.current_workspace_state.tab_being_renamed(), + Some(0) + ); + assert!(workspace.tab_rename_editor.is_focused(ctx)); + }); + }); +} + +#[test] +fn test_selecting_rename_from_pane_context_menu_preserves_editor_focus() { + let _vertical_tabs_guard = FeatureFlag::VerticalTabs.override_enabled(true); + + App::test((), |mut app| async move { + initialize_app(&mut app); + + let workspace = mock_workspace(&mut app); + let (menu, locator) = workspace.update(&mut app, |workspace, ctx| { + let pane_group = workspace.active_tab_pane_group(); + let locator = PaneViewLocator { + pane_group_id: pane_group.id(), + pane_id: pane_group.as_ref(ctx).pane_id_from_index(0).unwrap(), + }; + workspace.toggle_vertical_tabs_pane_context_menu( + 0, + VerticalTabsPaneContextMenuTarget::ClickedPane(locator), + Vector2F::zero(), + ctx, + ); + (workspace.tab_right_click_menu.clone(), locator) + }); + + menu.update(&mut app, |menu, ctx| { + assert!(menu.set_selected_by_name("Rename pane", ctx)); + menu.handle_action(&MenuAction::Enter, ctx); + }); + + workspace.read(&app, |workspace, ctx| { + assert!(workspace.show_tab_right_click_menu.is_none()); + assert_eq!( + workspace.current_workspace_state.pane_being_renamed(), + Some(locator) + ); + assert!(workspace.pane_rename_editor.is_focused(ctx)); + }); + }); +} + +#[test] +fn test_selecting_rename_from_tab_group_context_menu_preserves_editor_focus() { + let _grouped_tabs_guard = FeatureFlag::GroupedTabs.override_enabled(true); + + App::test((), |mut app| async move { + initialize_app(&mut app); + + let workspace = mock_workspace(&mut app); + let menu = workspace.update(&mut app, |workspace, ctx| { + let group = TabGroup::new(); + let group_id = group.id; + workspace.tab_groups.insert(group_id, group); + workspace.tabs[0].group_id = Some(group_id); + workspace.toggle_tab_group_right_click_menu( + group_id, + TabContextMenuAnchor::Pointer(Vector2F::zero()), + ctx, + ); + workspace.tab_right_click_menu.clone() + }); + + menu.update(&mut app, |menu, ctx| { + assert!(menu.set_selected_by_name("Rename", ctx)); + menu.handle_action(&MenuAction::Enter, ctx); + }); + + workspace.read(&app, |workspace, ctx| { + assert!(workspace.show_tab_group_right_click_menu.is_none()); + assert!(workspace + .current_workspace_state + .is_any_tab_group_being_renamed()); + assert!(workspace.tab_group_rename_editor.is_focused(ctx)); + }); + }); +} + #[test] fn test_close_tabs_right_confirmation_dialog() { let _guard = FeatureFlag::CreatingSharedSessions.override_enabled(true); diff --git a/crates/cloud_object_models/src/ai_execution_profile_tests.rs b/crates/cloud_object_models/src/ai_execution_profile_tests.rs index 6125e1c1..fb53ee6d 100644 --- a/crates/cloud_object_models/src/ai_execution_profile_tests.rs +++ b/crates/cloud_object_models/src/ai_execution_profile_tests.rs @@ -80,33 +80,46 @@ fn serialized_profiles_only_store_one_permission_mode_and_bounded_approvals() { fn explicit_command_approvals_and_provider_selection_survive_roundtrip_and_mode_changes() { let mut profile = AIExecutionProfile { permission_mode: PermissionMode::AllowAll, - command_allowlist: vec![AgentModeCommandExecutionPredicate::new_regex("^cargo test$").unwrap()], + command_allowlist: vec![ + AgentModeCommandExecutionPredicate::new_regex("^cargo test$").unwrap(), + ], orchestration_providers: Some(vec!["My LiteLLM".to_string()]), ..Default::default() }; - for mode in [PermissionMode::ReadOnly, PermissionMode::AskFirst, PermissionMode::GalaxyDecides, PermissionMode::AllowAll] { + for mode in [ + PermissionMode::ReadOnly, + PermissionMode::AskFirst, + PermissionMode::GalaxyDecides, + PermissionMode::AllowAll, + ] { profile.permission_mode = mode; profile.project_permission_mode(); let saved = serde_json::to_value(&profile).unwrap(); let restored: AIExecutionProfile = serde_json::from_value(saved).unwrap(); assert_eq!(restored.permission_mode, mode); assert_eq!(restored.command_allowlist, profile.command_allowlist); - assert_eq!(restored.includes_orchestration_provider("My LiteLLM"), true); - assert_eq!(restored.includes_orchestration_provider("ChatGPT"), false); + assert!(restored.includes_orchestration_provider("My LiteLLM")); + assert!(!restored.includes_orchestration_provider("ChatGPT")); } } #[test] fn missing_provider_selection_keeps_existing_profiles_unrestricted() { let profile: AIExecutionProfile = serde_json::from_str("{}").unwrap(); - assert_eq!(profile.includes_orchestration_provider("New provider"), true); - let excluded = AIExecutionProfile { orchestration_providers: Some(Vec::new()), ..Default::default() }; - assert_eq!(excluded.includes_orchestration_provider("New provider"), false); + assert!(profile.includes_orchestration_provider("New provider")); + let excluded = AIExecutionProfile { + orchestration_providers: Some(Vec::new()), + ..Default::default() + }; + assert!(!excluded.includes_orchestration_provider("New provider")); } #[test] fn allow_all_projects_every_tool_to_always_allow() { - let mut profile = AIExecutionProfile { permission_mode: PermissionMode::AllowAll, ..Default::default() }; + let mut profile = AIExecutionProfile { + permission_mode: PermissionMode::AllowAll, + ..Default::default() + }; profile.project_permission_mode(); assert_eq!(profile.execute_commands, ActionPermission::AlwaysAllow); assert_eq!(profile.apply_code_diffs, ActionPermission::AlwaysAllow); diff --git a/crates/galaxy_terminal/src/model/grid/cell.rs b/crates/galaxy_terminal/src/model/grid/cell.rs index 4e0c8730..2fc222f0 100644 --- a/crates/galaxy_terminal/src/model/grid/cell.rs +++ b/crates/galaxy_terminal/src/model/grid/cell.rs @@ -208,8 +208,43 @@ impl Cell { /// a row from flat scrollback storage, where the stored content was /// already capped on the way in) should pass `false` to suppress /// that redundant warning. + /// + /// Returns whether the character was appended. A return value of + /// `false` means that the grapheme was already at the size cap and the + /// cell was not changed. #[inline] - pub fn push_zerowidth(&mut self, c: char, log_long_grapheme_warnings: bool) { + pub fn push_zerowidth(&mut self, c: char, log_long_grapheme_warnings: bool) -> bool { + if let Some(zerowidth) = self + .extra + .as_deref_mut() + .and_then(|extra| extra.cell_with_zero_width.as_mut()) + { + let old_len = zerowidth.len(); + let new_len = old_len + c.len_utf8(); + if new_len > MAX_GRAPHEME_BYTES { + // The accumulated grapheme cluster would exceed our + // per-cell cap, which is in turn well below the + // scrollback chunk size. Silently drop additional + // zero-width characters: logging every dropped + // character would produce a flood of spam for + // pathological streams. + return false; + } + zerowidth.push(c); + // Log exactly once, on the push that first takes this cell + // across the soft threshold. This surfaces unusually-large + // graphemes in logs without producing per-character spam. + if log_long_grapheme_warnings + && old_len < WARN_GRAPHEME_BYTES + && new_len >= WARN_GRAPHEME_BYTES + { + log::warn!( + "cell grapheme has accumulated {new_len} bytes of zero-width content (base char {:?}); further zero-width pushes beyond {MAX_GRAPHEME_BYTES} bytes will be dropped", + self.c, + ); + } + return true; + } // If we're adding a zero-width character to this cell, but it has not // had any content set yet, set the content to a space. This preserves // its visual appearance, but clearly marks the cell as having been @@ -219,40 +254,32 @@ impl Cell { } let extra = self.extra.get_or_insert_with(Box::default); - match &mut extra.cell_with_zero_width { - Some(zerowidth) => { - let old_len = zerowidth.len(); - let new_len = old_len + c.len_utf8(); - if new_len > MAX_GRAPHEME_BYTES { - // The accumulated grapheme cluster would exceed our - // per-cell cap, which is in turn well below the - // scrollback chunk size. Silently drop additional - // zero-width characters: logging every dropped - // character would produce a flood of spam for - // pathological streams. - return; - } - zerowidth.push(c); - // Log exactly once, on the push that first takes this cell - // across the soft threshold. This surfaces unusually-large - // graphemes in logs without producing per-character spam. - if log_long_grapheme_warnings - && old_len < WARN_GRAPHEME_BYTES - && new_len >= WARN_GRAPHEME_BYTES - { - log::warn!( - "cell grapheme has accumulated {new_len} bytes of zero-width content (base char {:?}); further zero-width pushes beyond {MAX_GRAPHEME_BYTES} bytes will be dropped", - self.c, - ); - } - } - None => { - // First zero-width push seeds the string with the base - // character. The base character is always a single `char`, - // so it cannot by itself exceed the cap. - extra.cell_with_zero_width = Some(format!("{}{}", self.c, c)); - } + // The first zero-width push seeds the string with the base + // character. The base character is always a single `char`, so it + // cannot by itself exceed the cap. + extra.cell_with_zero_width = Some(format!("{}{}", self.c, c)); + true + } + + /// Removes and returns the most recently appended zero-width character. + #[inline] + pub fn pop_zerowidth(&mut self) -> Option { + let base_char_len = self.c.len_utf8(); + let extra = self.extra.as_deref_mut()?; + let content = extra.cell_with_zero_width.as_mut()?; + if content.len() <= base_char_len { + return None; } + + let popped = content.pop(); + if content.len() == base_char_len { + extra.cell_with_zero_width = None; + } + let extra_is_empty = extra.cell_with_zero_width.is_none() && extra.end_of_prompt.is_none(); + if extra_is_empty { + self.extra = None; + } + popped } /// Returns whether cell is the end of prompt content (contains `EndOfPromptMarker`). diff --git a/crates/galaxy_terminal/src/model/grid/cell_tests.rs b/crates/galaxy_terminal/src/model/grid/cell_tests.rs index ea781f59..650d242f 100644 --- a/crates/galaxy_terminal/src/model/grid/cell_tests.rs +++ b/crates/galaxy_terminal/src/model/grid/cell_tests.rs @@ -58,9 +58,11 @@ fn push_zerowidth_caps_accumulated_grapheme() { let zwj = '\u{200D}'; let zwj_bytes = zwj.len_utf8(); let pushes = (MAX_GRAPHEME_BYTES * 10) / zwj_bytes; + let mut rejected_push = false; for _ in 0..pushes { - cell.push_zerowidth(zwj, /* log_long_grapheme_warnings */ true); + rejected_push |= !cell.push_zerowidth(zwj, /* log_long_grapheme_warnings */ true); } + assert!(rejected_push); let CharOrStr::Str(content) = cell.raw_content() else { panic!("cell should have accumulated zero-width content as a string"); @@ -100,3 +102,22 @@ fn push_zerowidth_seeds_base_char_on_first_push() { cell.push_zerowidth('\u{0301}', /* log_long_grapheme_warnings */ true); assert_eq!(cell.raw_content(), CharOrStr::Str("x\u{0301}")); } + +#[test] +fn pop_zerowidth_restores_previous_cell_content() { + let mut cell = Cell { + c: 'x', + ..Cell::default() + }; + let original_cell = cell.clone(); + + assert!(cell.push_zerowidth('\u{0301}', /* log_long_grapheme_warnings */ true)); + let cell_with_first_zerowidth = cell.clone(); + assert!(cell.push_zerowidth('\u{200D}', /* log_long_grapheme_warnings */ true)); + + assert_eq!(cell.pop_zerowidth(), Some('\u{200D}')); + assert_eq!(cell, cell_with_first_zerowidth); + assert_eq!(cell.pop_zerowidth(), Some('\u{0301}')); + assert_eq!(cell, original_cell); + assert_eq!(cell.pop_zerowidth(), None); +} diff --git a/crates/galaxy_terminal/src/model/grid/flat_storage/row_iterator.rs b/crates/galaxy_terminal/src/model/grid/flat_storage/row_iterator.rs index 4e5942e2..4ec684f9 100644 --- a/crates/galaxy_terminal/src/model/grid/flat_storage/row_iterator.rs +++ b/crates/galaxy_terminal/src/model/grid/flat_storage/row_iterator.rs @@ -109,7 +109,9 @@ impl Iterator for RowIterator<'_> { // first seen on the ANSI-input path, so a warning here would // be redundant and would fire every time a row is // rematerialized (e.g. on scroll or resize). - chars.for_each(|c| cell.push_zerowidth(c, /* log_long_grapheme_warnings */ false)); + chars.for_each(|c| { + cell.push_zerowidth(c, /* log_long_grapheme_warnings */ false); + }); cell.fg = fg; cell.bg = bg; diff --git a/crates/galaxy_terminal/src/model/grid/flat_storage/testing.rs b/crates/galaxy_terminal/src/model/grid/flat_storage/testing.rs index 1118cffc..3c66537f 100644 --- a/crates/galaxy_terminal/src/model/grid/flat_storage/testing.rs +++ b/crates/galaxy_terminal/src/model/grid/flat_storage/testing.rs @@ -122,7 +122,9 @@ impl ToRows for &str { cell.c = chars.next().unwrap(); // Add any remaining chars in the grapheme to the cell as zero-width // characters. - chars.for_each(|c| cell.push_zerowidth(c, /* log_long_grapheme_warnings */ true)); + chars.for_each(|c| { + cell.push_zerowidth(c, /* log_long_grapheme_warnings */ true); + }); // If the grapheme takes up two cells, mark the following cell as // a spacer. diff --git a/crates/galaxyui/src/platform/mac/clipboard.rs b/crates/galaxyui/src/platform/mac/clipboard.rs index 8bdda82e..429ad85a 100644 --- a/crates/galaxyui/src/platform/mac/clipboard.rs +++ b/crates/galaxyui/src/platform/mac/clipboard.rs @@ -136,6 +136,7 @@ impl Clipboard { ns_string!("public.gif"), ns_string!("public.webp"), ns_string!("public.svg-image"), + ns_string!("public.tiff"), ns_string!("com.compuserve.gif"), ]; @@ -154,6 +155,7 @@ impl Clipboard { "public.gif" | "com.compuserve.gif" => "image/gif", "public.webp" => "image/webp", "public.svg-image" => "image/svg+xml", + "public.tiff" => "image/tiff", _ => "image/unknown", }; diff --git a/crates/mcp/src/oauth.rs b/crates/mcp/src/oauth.rs index 3baeaa21..cb1b1e2f 100644 --- a/crates/mcp/src/oauth.rs +++ b/crates/mcp/src/oauth.rs @@ -192,8 +192,14 @@ pub struct AuthContext { /// Result of OAuth callback. #[derive(Debug, Clone)] pub enum CallbackResult { - Success { code: String, csrf_token: String }, - Error { error: Option }, + Success { + code: String, + csrf_token: String, + issuer: Option, + }, + Error { + error: Option, + }, } /// Makes an authenticated client for the given authorization server. @@ -340,8 +346,12 @@ pub async fn make_authenticated_client( .await .map_err(|e| AuthError::InternalError(e.to_string()))?; - let (code, csrf_token) = match &oauth_result { - CallbackResult::Success { code, csrf_token } => (code, csrf_token), + let (code, csrf_token, issuer) = match &oauth_result { + CallbackResult::Success { + code, + csrf_token, + issuer, + } => (code, csrf_token, issuer), CallbackResult::Error { error } => { return Err(AuthError::AuthorizationFailed( error.as_deref().unwrap_or("unknown error").to_string(), @@ -350,7 +360,9 @@ pub async fn make_authenticated_client( }; // Handle the callback with the received authorization code and CSRF token. - oauth_state.handle_callback(code, csrf_token).await?; + oauth_state + .handle_callback_with_issuer(code, csrf_token, issuer.as_deref()) + .await?; let auth_manager = oauth_state.into_authorization_manager().ok_or_else(|| { AuthError::InternalError("Failed to create authorization manager".to_string())