Fix @mention token parsing in chat composer (#12643)
Fixes #12175 If a user types an npm package name with multiple `@` symbols like `npx -y @foo/bar@latest`, the TUI currently treats this as though it's attempting to invoke the file picker. ### What changed - **Generalized `@` token parsing** - `current_prefixed_token(...)` now treats `@` as a token start **only at a whitespace boundary** (or start-of-line). - If the cursor is on a nested `@` inside an existing whitespace-delimited token (for example `@scope/pkg@latest`), it keeps the surrounding token active instead of starting a new token at the second `@`. - It also avoids misclassifying mid-word usages like `foo@bar` as an `@` file token. - **Enter behavior with file popup** - If the file-search popup is open but has **no selected match**, pressing `Enter` now closes the popup and falls through to normal submit behavior. - This prevents pasted strings containing `@...` from blocking submission just because file-search was active with no actionable selection. ### Testing I manually built and tested the scenarios involved with the bug report and related use of `@` mentions to verify no regressions
This commit is contained in:
parent
3ca0e7673b
commit
74cebceed7
1 changed files with 127 additions and 3 deletions
|
|
@ -1641,7 +1641,11 @@ impl ChatComposer {
|
|||
} => {
|
||||
let Some(sel) = popup.selected_match() else {
|
||||
self.active_popup = ActivePopup::None;
|
||||
return (InputResult::None, true);
|
||||
return if key_event.code == KeyCode::Enter {
|
||||
self.handle_key_event_without_popup(key_event)
|
||||
} else {
|
||||
(InputResult::None, true)
|
||||
};
|
||||
};
|
||||
|
||||
let sel_path = sel.to_string_lossy().to_string();
|
||||
|
|
@ -1692,7 +1696,6 @@ impl ChatComposer {
|
|||
// Non-image: inserting file path.
|
||||
self.insert_selected_path(&sel_path);
|
||||
}
|
||||
// No selection: treat Enter as closing the popup/session.
|
||||
self.active_popup = ActivePopup::None;
|
||||
(InputResult::None, true)
|
||||
}
|
||||
|
|
@ -1912,6 +1915,10 @@ impl ChatComposer {
|
|||
/// - The cursor may be anywhere *inside* the token (including on the
|
||||
/// leading prefix). It does **not** need to be at the end of the line.
|
||||
/// - A token is delimited by ASCII whitespace (space, tab, newline).
|
||||
/// - If the cursor is on `prefix` inside an existing token (for example the
|
||||
/// second `@` in `@scope/pkg@latest`), keep treating the surrounding
|
||||
/// whitespace-delimited token as the active token rather than starting a
|
||||
/// new token at that nested prefix.
|
||||
/// - If the token under the cursor starts with `prefix`, that token is
|
||||
/// returned without the leading prefix. When `allow_empty` is true, a
|
||||
/// lone prefix character yields `Some(String::new())` to surface hints.
|
||||
|
|
@ -2005,7 +2012,15 @@ impl ChatComposer {
|
|||
return left_prefixed;
|
||||
}
|
||||
if after_cursor.starts_with(prefix) {
|
||||
return right_prefixed.or(left_prefixed);
|
||||
let prefix_starts_token = before_cursor
|
||||
.chars()
|
||||
.next_back()
|
||||
.is_none_or(char::is_whitespace);
|
||||
return if prefix_starts_token {
|
||||
right_prefixed.or(left_prefixed)
|
||||
} else {
|
||||
left_prefixed
|
||||
};
|
||||
}
|
||||
left_prefixed.or(right_prefixed)
|
||||
}
|
||||
|
|
@ -5420,6 +5435,115 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_current_at_token_tracks_tokens_with_second_at() {
|
||||
let input = "npx -y @kaeawc/auto-mobile@latest";
|
||||
let token_start = input.find("@kaeawc").expect("scoped npm package present");
|
||||
let version_at = input
|
||||
.rfind("@latest")
|
||||
.expect("version suffix present in scoped npm package");
|
||||
let test_cases = vec![
|
||||
(token_start, "Cursor at leading @"),
|
||||
(token_start + 8, "Cursor inside scoped package name"),
|
||||
(version_at, "Cursor at version @"),
|
||||
(input.len(), "Cursor at end of token"),
|
||||
];
|
||||
|
||||
for (cursor_pos, description) in test_cases {
|
||||
let mut textarea = TextArea::new();
|
||||
textarea.insert_str(input);
|
||||
textarea.set_cursor(cursor_pos);
|
||||
|
||||
let result = ChatComposer::current_at_token(&textarea);
|
||||
assert_eq!(
|
||||
result,
|
||||
Some("kaeawc/auto-mobile@latest".to_string()),
|
||||
"Failed for case: {description} - input: '{input}', cursor: {cursor_pos}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_current_at_token_allows_file_queries_with_second_at() {
|
||||
let input = "@icons/icon@2x.png";
|
||||
let version_at = input
|
||||
.rfind("@2x")
|
||||
.expect("second @ in file token should be present");
|
||||
let test_cases = vec![
|
||||
(0, "Cursor at leading @"),
|
||||
(8, "Cursor before second @"),
|
||||
(version_at, "Cursor at second @"),
|
||||
(input.len(), "Cursor at end of token"),
|
||||
];
|
||||
|
||||
for (cursor_pos, description) in test_cases {
|
||||
let mut textarea = TextArea::new();
|
||||
textarea.insert_str(input);
|
||||
textarea.set_cursor(cursor_pos);
|
||||
|
||||
let result = ChatComposer::current_at_token(&textarea);
|
||||
assert!(
|
||||
result.is_some(),
|
||||
"Failed for case: {description} - input: '{input}', cursor: {cursor_pos}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_current_at_token_ignores_mid_word_at() {
|
||||
let input = "foo@bar";
|
||||
let at_pos = input.find('@').expect("@ present");
|
||||
let test_cases = vec![
|
||||
(at_pos, "Cursor at mid-word @"),
|
||||
(input.len(), "Cursor at end of word containing @"),
|
||||
];
|
||||
|
||||
for (cursor_pos, description) in test_cases {
|
||||
let mut textarea = TextArea::new();
|
||||
textarea.insert_str(input);
|
||||
textarea.set_cursor(cursor_pos);
|
||||
|
||||
let result = ChatComposer::current_at_token(&textarea);
|
||||
assert_eq!(
|
||||
result, None,
|
||||
"Failed for case: {description} - input: '{input}', cursor: {cursor_pos}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn enter_submits_when_file_popup_has_no_selection() {
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyModifiers;
|
||||
|
||||
let (tx, _rx) = unbounded_channel::<AppEvent>();
|
||||
let sender = AppEventSender::new(tx);
|
||||
let mut composer = ChatComposer::new(
|
||||
true,
|
||||
sender,
|
||||
false,
|
||||
"Ask Codex to do anything".to_string(),
|
||||
false,
|
||||
);
|
||||
composer.set_steer_enabled(true);
|
||||
|
||||
let input = "npx -y @kaeawc/auto-mobile@latest";
|
||||
composer.textarea.insert_str(input);
|
||||
composer.textarea.set_cursor(input.len());
|
||||
composer.sync_popups();
|
||||
|
||||
assert!(matches!(composer.active_popup, ActivePopup::File(_)));
|
||||
|
||||
let (result, consumed) =
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
assert!(consumed);
|
||||
match result {
|
||||
InputResult::Submitted { text, .. } => assert_eq!(text, input),
|
||||
_ => panic!("expected Submitted"),
|
||||
}
|
||||
}
|
||||
|
||||
/// Behavior: if the ASCII path has a pending first char (flicker suppression) and a non-ASCII
|
||||
/// char arrives next, the pending ASCII char should still be preserved and the overall input
|
||||
/// should submit normally (i.e. we should not misclassify this as a paste burst).
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue