-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix directory link handling in markdown #3690
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
Enhance the openFile function to better handle directory links in markdown: - Add support for resolving ./SimpleName paths to home directory if not found in workspace - Improve path resolution by checking multiple potential locations - Ensure directories are properly revealed in the Explorer view - Attempt to expand directories after revealing them Fixes: cline#3686 Signed-off-by: Eric Wheeler <[email protected]>
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.
This looks goods to me, just a minor point that might be worth paying atenttion.
It makes sense to also handle directory paths when the model links them.
|
||
const uri = vscode.Uri.file(fullPath) | ||
if (filePath.startsWith("./")) { | ||
const relativePart = filePath.slice(2) |
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.
Hey @KJ7LNW Would it be a good idea to prevent directory traversal?
Something like this could work:
if (relativePart.includes('..')) {
throw new Error('Path contains directory traversal pattern');
}
Or maybe something more sophisticated would make sense.
I just tested your implementation and I see some weird behavior when the model outputs a path like ..
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.
Did the directory that it referenced using ../ exist, or was it just missing?
Can you provide a specific example so I can understand better?
I think we need to expect the model will sometimes render things in a way that is not expected; worst case the link is broken, but I do not think that is a problem.
Render-output handling is always best effort: the models are pretty good, but you never know what you are going to get.
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.
Here's how it behaves for me, it's probably a non-issue though, that's why I left this PR for review.
2025-05-29.17-17-13.mp4
Apologies for the awful quality.
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.
that is strange behavior. if anything, ..
should mean ${workspace}/..
but you're right, non-issue no one will need to click on ".."
Context
When clicking on directory links in markdown like
[some/directory/](some/directory/)
, the application does not know how to open the link in the explorer tree. Gemini 2.5 references directory sometimes, so this feature makes it possible to click on those links.Clicking
test/
takes you here:Implementation
Enhanced the
openFile
function to better handle directory links in markdown:How to Test
[Desktop](Desktop)
Fixes #3686
Important
Enhance
openFile
inopen-file.ts
to handle directory links in markdown by resolving paths to home directory, checking multiple locations, and revealing directories in Explorer.openFile
function inopen-file.ts
to handle directory links in markdown../SimpleName
paths to home directory if not found in workspace.This description was created by
for db482aa. You can customize this summary. It will automatically update as commits are pushed.