Skip to content

fix/chat: Send workspace root as the expected URI, not a path #2352

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

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

dominiccooney
Copy link

JetBrains was initializing Agent passing a path instead of a workspace root URI. This is fine on UNIX-style systems, because the string /foo/bar is interpreted as a file URI. But on Windows it meant we were passing paths like c:/Users/bob/project (yes... we convert the path separator to a forward slash etc.) and Agent was treating this as /Users/bob/project which is wrong.

The fix is simply to pass a URI instead of a file.

In addition, normalizeUriOrPath was lowercasing drive letters, but in the URI case only if the URI was file://C:/foo; this fixes it to also handle file:///C:/foo (note the extra slash.)

Note, Extension handling often? converts these URLs from file:///c:/foo to file:///c%3a/foo and does prefix checks on URIs as strings or paths, so we should consider canonicalizing colons in file URLs too, but that will take more care and attention. This change is enough to unbreak Prompt Library mentions on Windows.

Test plan

On Windows,

  • Run with prompts as commands enabled (currently set up on s2)
  • Open a file, right click, Cody, Explain Code.
  • Check that the filename has a @file.txt presentation on a mention slab, instead of @C:\full\path\to\file.txt without a mention slab.

Screenshot 2024-09-26 143056

Changelog

  • Agent is now configured with an accurate workspace on root on Windows, like file:///c:/Users/Bob/project not /Users/bob/project.
  • On Windows, at-mentions for explain, document, smell, etc. display as tiles with short names.

@dominiccooney dominiccooney requested review from vovakulikov and a team September 26, 2024 05:45
Copy link
Contributor

@pkukielka pkukielka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
That said, we really need to get rid of those paths-or-uris-as-strings everywhere in the protocol.

@pkukielka pkukielka merged commit 0d3ff42 into main Sep 26, 2024
6 of 7 checks passed
@pkukielka pkukielka deleted the dpc/send-workspace-uri-not-path branch September 26, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants