-
Notifications
You must be signed in to change notification settings - Fork 249
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
Only search relative paths when @-mentioning files in chat #2241
Conversation
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.
Looks good!
|
||
// We should only match the relative visible path, not parts of the full path outside of the workspace. | ||
// Eg. searching for "source" should not find all files if the project is inside `C:\Source`. | ||
await chatInput.fill('@fixtures') // fixture is in the test project folder name, but in the relative paths. |
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.
Once #2235 lands, you can make this more robust by adding workspaceDirectory
alongside page, sidebar
and get the path to the workspace directory. That will let you assert that fixtures really is in that path and remains that way.
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.
Oh yeah, good point. I've added a TODO about that. If you're planning to land that soon I can wait and fix it up here, or if you think I should land this as-is and fix later, let me know.
I've rebased this on main after merging #2208 so I think this should all be green now.
eb27c16
to
c44f623
Compare
Hmm, there's something odd going on now. With this change, fail.mp4This is reliably reproducible for me, and feels like two bugs (non-matched items in the results, and the duplicated text). I might not get chance to look into this before Friday now, so if this is important and anyone else has time, feel free to take a look! |
lol, it very much as a |
I've filed #2243 about the bug. @dominiccooney heading off, but if you're happy with this as-is and the last bots go green feel free to merge :) |
[This depends on a function added in https://github.com//pull/2215 so this will fail bots until that lands and I rebase/merge]
Fixes #2208
Test plan
pnpm test:e2e at-file
to run the updated test, or@
but type part of the file path outside of the folder opened in VS Code (eg. if you haveC:\Source\cody\ open, type
source`)source
exists in their full fspaths