-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix: allow folder drill-down in context picker #8290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Modified handleMentionSelect to handle folder selection specially - When selecting a folder, ensure path ends with "/" - Insert folder mention without trailing space to keep menu open - Trigger immediate search for folder contents - Fixes #8076 where Tab/Enter would close picker preventing drill-down
- Added tests for folder path handling without trailing spaces - Added tests for shouldShowContextMenu with folder paths - Added tests for slash command behavior - Tests verify menu stays open for folder drill-down
| } | ||
| } | ||
|
|
||
| // Special handling for folder selection with concrete value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good specialized handling for folder selection. Consider adding a clarifying comment that explains why we bypass insertMention (to avoid the extra trailing space) and whether the folder path needs to be escaped here (or if the value is already escaped upstream).
| // Special handling for folder selection with concrete value | |
| // Special handling for folder selection with concrete value: bypass insertMention to avoid extra trailing space. Folder path is assumed to be already escaped upstream. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-review: Auditing my own diff like a linter staring into a mirror.
Review Findings:
P1 - Escaping for folder paths with spaces:
- In webview-ui/src/components/chat/ChatTextArea.tsx: the folder special-case bypasses insertMention() and inserts the raw folderPath. If the folder contains spaces, the next keystroke will cause shouldShowContextMenu() to see unescaped whitespace and close the picker, breaking drill-down.
- Recommendation:
- Ensure path ends with "/" then escape for display using escapeSpaces().
- Use the escaped display value in the textarea (no trailing space).
- Unescape only for search messages.
- Example:
import { escapeSpaces } from '@src/utils/path-mentions'
let folderPath = value; if (!folderPath.endsWith('/')) folderPath += '/';
const displayPath = escapeSpaces(folderPath);
// Use displayPath in the inserted value and caret math.
P2 - Consistent search query handling:
- The immediate search after insertion posts query as-is. Elsewhere (handleInputChange) the code calls unescapeSpaces(query) before searchFiles. If the input uses escaped spaces (recommended above), mirror that behavior when posting the immediate search to avoid mismatches.
- Recommendation:
vscode.postMessage({ type: 'searchFiles', query: unescapeSpaces(query), requestId: reqId })
P3 - Robustness of menu state:
- Relying only on not calling setShowContextMenu(false) can be fragile if prior state closed it. Explicitly setting setShowContextMenu(true) after insertion makes the behavior resilient.
P4 - Missing component-level test:
- The new folder-drilldown branch in handleMentionSelect() is not exercised by a UI test. Add a test (e.g., webview-ui/src/components/chat/tests/ChatTextArea.folder-drilldown.spec.tsx) that simulates selecting a folder with spaces and asserts:
- Input becomes '@/my\ documents/folder/' (no trailing space), caret after '/'.
- Menu remains open.
- Immediate searchFiles is posted with unescapeSpaces-ed query.
|
Closing in favor of #8289 |
Summary
This PR fixes issue #8076 where selecting a folder in the context picker would close the menu instead of allowing users to drill down into the folder contents.
Problem
When users pressed Tab/Enter on a folder in the context picker:
Solution
Modified the folder selection behavior in
handleMentionSelect:Changes
ChatTextArea.tsxto add special handling for folder selectionTesting
Fixes #8076
Implementation Details
As suggested by @hannesrudolph, the implementation:
setShowContextMenu(false)to keep menu openThis allows users to seamlessly drill down into folders using keyboard navigation.
Important
Fixes folder drill-down in context picker by keeping menu open and formatting paths correctly in
ChatTextArea.tsx.ChatTextArea.tsxto allow drill-down without closing the menu.context-mentions.spec.tsfor folder drill-down behavior.handleMentionSelectinChatTextArea.tsxto handle folder paths specifically.This description was created by
for bd8c101. You can customize this summary. It will automatically update as commits are pushed.