Conversation
src/infoManagers/endpointManager.ts
Outdated
| public async encodeLooseFileEndpoint(location: string, file?: vscode.Uri): Promise<string> { | ||
| let fullParent = await PathUtil.GetParentDir(location); | ||
| const child = await PathUtil.GetFileName(location, true); | ||
| const child = file?.scheme === 'vscode-chat-code-block' ? |
There was a problem hiding this comment.
The code block URIs have a path like /response_1/0. GetFileName would just return 0 in that case and we should keep more context.
This is a hack but the path handling for non-file URIs is a little confusing. Maybe we can come up with something better.
There was a problem hiding this comment.
Technically this still isn't enough- the full URI is like "vscode-chat-code-block://1a22dd3d-ff95-4b4b-bb77-ca04942e2280/response_0/0" and the GUI identifies the chat session. The GUID should be part of the url too, otherwise you could have collisions between codeblocks in multiple chat sessions.
There was a problem hiding this comment.
would there be multiple codeblocks that return 0? I guess it should be fine as long as they don't need the old preview, but we want to prevent server path duplications. IIRC, this is generating the path that the server uses to access the file.
There was a problem hiding this comment.
also, I'm a bit confused: if you used file.fsPath.slice(1) and the path was /response_1/0, wouldn't the string return response_1/0? Or is what you're trying to say is that you want to retain more than just 0?
There was a problem hiding this comment.
Oh okay, I re-read and it does seem that you want to retain up to response_1/0. I don't mind the GUID being part of the URL too. Is that part of the authority?
There was a problem hiding this comment.
Yeah, the guid is part of the authority.
One way to think about it would be that we could have a nice path for Untitled files where it produces a readable url, but for other files that are not Untitled, not file: scheme, we just put the whole URI into the generated path, and it wouldn't be pretty but then it will work for all other types of URIs.
Are there other types of URIs that you can preview that aren't Untitled editors but have some other scheme? Like from custom FS providers? I wasn't sure
There was a problem hiding this comment.
I don't think that we support any other types of schemes that require special treatment, so this would be a first. I'm fine with this handling for now.
andreamah
left a comment
andreamah
left a comment
There was a problem hiding this comment.
These changes make sense to me. How exactly does it get registered as a 'previewer'? I would expect that there is some way to register this extension as a chat previewer via the package.json
andreamah
left a comment
andreamah
left a comment
There was a problem hiding this comment.
For the record- these changes look fine to me.
|
This is just to enable the normal editor context menu action for codeblocks, so I can right click an HTML codeblock and click "Preview" |
Ah ok, I didn't think to right click. It doesn't seem to work for me with just these changes. Does it work on your side? |
|
It did last time I tried it, what happens for you? |
|
It seemed like it went to the wrong URL for me. Maybe it was a windows thing. |
Co-authored-by: Andrea Mah <31675041+andreamah@users.noreply.github.com>
| */ | ||
| public async encodeLooseFileEndpoint(location: string): Promise<string> { | ||
| public async encodeLooseFileEndpoint(file: string | vscode.Uri): Promise<string> { | ||
| if (typeof file !== 'string' && file.scheme === 'vscode-chat-code-block') { |
There was a problem hiding this comment.
In theory we could try to handle all types of URIs that are not loose files on disk or untitled files, instead of just handling vscode-chat-code-block here but I didn't have enough confidence in what this method has to handle
|
|
||
| while (i < workspaceDocuments.length) { | ||
| if (PathUtil.PathEquals(readPath, workspaceDocuments[i].fileName)) { | ||
| if (PathUtil.PathEquals(readPath, workspaceDocuments[i].fileName) || (workspaceDocuments[i].uri.scheme === 'vscode-chat-code-block' && workspaceDocuments[i].uri.with({ fragment: '' }).toString() === readPath)) { |
There was a problem hiding this comment.
Could put this thing .uri.with({ fragment: '' }).toString() in some utility, where's a good spot?
|
Tried to clean this up a bit, but I'm still not 100% sure I'm doing the right thing. No rush, we can look at it later during debt week. |
3 participants
Needs microsoft/vscode#224662 for the editor language context key
What do you think of this @andreamah? I think it's a cool scenario but I had to do a couple hacks in the code.
Fixes #673