-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Fix AI text thread path display bugs on Windows and all platforms #41880
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
base: main
Are you sure you want to change the base?
Conversation
Display full path only for the top-level directory when collecting files; use the bare filename for other entries. Also avoid an unnecessary to_string call when sending the label.
| folded_directory_names = | ||
| folded_directory_names.join(&path_including_worktree_name); | ||
| } else { | ||
| folded_directory_names = |
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.
| is_top_level_directory = false; | ||
| path_including_worktree_name.display(path_style).to_string() | ||
| } else { | ||
| filename | ||
| filename.clone() | ||
| }; | ||
| if is_top_level_directory { | ||
| is_top_level_directory = false; | ||
| } |
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.
I believe we can revert back to the original code here, that way we avoid checking is_top_level_directory twice and avoid the filename.clone() call 👍
| events_tx.unbounded_send(Ok(SlashCommandEvent::Content( | ||
| SlashCommandContent::Text { | ||
| text: label.to_string(), | ||
| // text: path_including_worktree_name.display(path_style).to_string(), |
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.
I believe we can remove this comment? 🤔
| // text: path_including_worktree_name.display(path_style).to_string(), |
Join folded_directory_names with the filename instead of resetting it to empty. Mark is_top_level_directory = false earlier so the filename can be used by value rather than cloned. Remove a redundant commented text line.

🐛 Fix: Files not loading and incorrect directory paths in
/fileslash commandSummary
Fixed two critical bugs in the
/fileslash command that prevented proper file collection and displayed incorrect directory paths when using folders with the AI assistant panel.Fixes: #41242
Problem Statement
Issue 1: Files Not Loading on Windows with
/file [folder]When using
/file [folder]on Windows, files were not appearing under the specified folder. This was due to a path separator mismatch:\\/Result:
This made the feature unusable on Windows for nested folders.
Issue 2: Folded Directory Paths Leak to Siblings
When a directory contains a single-child chain (e.g.,
.github/workflows/), the folded path was not being reset after use. This caused the folded path to leak into subsequent sibling directories, resulting in:Affected structure:
Root Cause
Windows path separator issue: The glob matcher receives backslashes from Windows file paths but expects forward slashes. The path matching fails silently, causing the matcher to skip all files in that path.
Folded path leak: The
folded_directory_namesvariable accumulated folded paths but was never reset after outputting a folded directory. When the next sibling was processed, the checkif folded_directory_names.is_empty()failed because it still contained the previous fold, causing incorrect label generation.Top-level label detection: Using
filename.is_empty()doesn't reliably identify the top-level matched directory. Theis_top_level_directoryflag is more accurate.Changes Made
Change 1: Normalize Windows backslashes to forward slashes in glob inputs
Change 2: Reset folded paths after use
Change 3: Use
is_top_level_directoryflag for correct label detectionChange 4: Simplify top-level folding accumulation logic
Before & After
Before (Windows):
After (Windows & All Platforms):
Checklist
Related Issue: #41242