Allow clients to redefine untitled files protocol during their creation #1903
Conversation
I found a problem - accept lens group appears too quickly (while the file content is still being generated) |
|
|
||
| // For the sake of correctness we only allow untitled files to be created. | ||
| // If we receive a URI with a scheme other than "untitled" it should already exist on the disk | ||
| if (uri.scheme != "untitled") return null |
There was a problem hiding this comment.
I do not like this return statement and the fact we need the comment. If this method assumes the file does not exist we should simply throw an exception and handle it higher.
Or maybe we could just do this check where this method is called - in src/main/kotlin/com/sourcegraph/cody/agent/CodyAgentService.kt?
There was a problem hiding this comment.
Returning null is consistent with other IJ file APIs where if file cannot be created it returns a null (e.g. ScratchRootType::createScratchFile)
As for comment I'm open for discussion.
It is not needed to understand this function, but I think it's a domain knowledge detail to understand that file:// coming from VSC is almost certainly existing file while untitiled:// is not. And as such it might deserve a comment, perhaps on the top of the function to document it.
There was a problem hiding this comment.
if it's consistent with IJ let it be as it is 🆗
pkukielka
force-pushed
the
pkukielka/fix-untitled-files-protocol
branch
from
7695efd
to
2b590f6
Compare
…on (#4841) Closes https://linear.app/sourcegraph/issue/CODY-2004/bug-cody-recreates-the-file-that-was-created-by-generate-unit-tests ## Changes Some IDEs (maybe even most IDEs except VSC) does not have notion of unsaved files. Keeping two different models in IDE and in the agent is problematic and leads to hard to track errors. It is easier to let client be honest and notify agent about real protocol which it is using (most likely `file://` instead of `untitled://`). As a result that change slightly modifies behaviour of the `openTextDocument`. One could now call `openTextDocument` with `untitled://abc` parameter, and as a result get a `TextDocument` which uri points to `file://abc` (or any other path, really). ## Test plan 1. Build [JetBrains branch ](sourcegraph/jetbrains#1903 this PR using CODY_DIR env var 2. Ask cody to generate tests for some file which does not have tests yet 3. Accept the changes 4. Delete that newly created file 5. Perform autocomplete in some other file 6. Make sure test file was not recreated <!-- Required. See https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles. -->
pkukielka
force-pushed
the
pkukielka/fix-untitled-files-protocol
branch
from
2b590f6
to
350704f
Compare
pkukielka
force-pushed
the
pkukielka/fix-untitled-files-protocol
branch
from
350704f
to
0b94299
Compare
Closes https://linear.app/sourcegraph/issue/CODY-2004/bug-cody-recreates-the-file-that-was-created-by-generate-unit-tests
Changes
Some IDEs (maybe even most IDEs except VSC) does not have notion of unsaved files.
Keeping two different models in IDE and in the agent is problematic and leads to hard to track errors.
It is easier to let client be honest and notify agent about real protocol which it is using (most likely
file://instead ofuntitled://).As a result that change slightly modifies behaviour of the
openTextDocument.One could now call
openTextDocumentwithuntitled://abcparameter, and as a result get aTextDocumentwhich uri points tofile://abc(or any other path, really).Test plan