Fix Generate Unit Test on Windows
#1478
Conversation
| @@ -187,8 +187,10 @@ abstract class FixupSession( | |||
| val future = group.show(range) | |||
| // Make sure the lens is visible. | |||
| ApplicationManager.getApplication().invokeLater { | |||
| val logicalPosition = LogicalPosition(range.start.line, range.start.character) | |||
| editor.scrollingModel.scrollTo(logicalPosition, ScrollType.CENTER) | |||
| if (!editor.isDisposed) { | |||
There was a problem hiding this comment.
unrel fix
The error appears when closing the file that contains the content being generated. This fixes it.
| fun uriFor(file: VirtualFile): String { | ||
| return Rfc3986UriEncoder.encode(file.url) | ||
| private fun uriFor(file: VirtualFile): String { | ||
| val uriString = FileSystems.getDefault().getPath(file.path).toUri().toString() |
There was a problem hiding this comment.
thanks to this we have
file:///c:/Users/mikolaj/blah-blah/blah
instead of
file://c:/Users/mikolaj/blah-blah/blah
(note additional / before the disk name)
That fixes the paths in the agent. This one is related directly to the issue with:
Caused by: java.nio.file.InvalidPathException: Illegal char <:> at index 46: c:/Users/Jay Gohil/IdeaProjects/jaysVSDemo/\\c:\Users\Jay Gohil\IdeaProjects\jaysVSDemo\BubbleSortTest.java
(#1436)
There was a problem hiding this comment.
Cool! Can we removed Rfc3986UriEncoder then?
There was a problem hiding this comment.
it looks like we can indeed!
| val uri = URI.create(params.uri).withScheme("file") | ||
| if (CodyEditorUtil.createFileIfNeeded(project, uri, params.content) == null) | ||
| val uri = URI.create(params.uri) | ||
| if (!uri.path.contains("/")) { |
There was a problem hiding this comment.
Is it worth commenting why? IIUC it is because we produce VirtualFiles, and below uriFor ensures they start with /?
With that being the case, can you harden this assertion that uri.path[0] is '/'? Should we log here? Could we throw here instead?
There was a problem hiding this comment.
Or even better startsWith("/") to avoid crashes in case of empty uri string.
I would also like to see detailed comment why that code looks that way.
I suppose that might be due to a fact that we are not covering all possible file creation requests.
VSCode sometimes creates in-memory files for a temp usage (which have UUID names) and we do not support/care about those.
It's a leftover from my recent changes, perhaps we should improve it on the agent side.
| if (!uri.path.contains("/")) { | ||
| return@Function false | ||
| } | ||
| val fileUri = uri.withScheme("file") |
There was a problem hiding this comment.
I know this is not new, but this seems really dodgy to me. When do we get URIs that aren't file:? When would it be OK to take on of those and say randomly, boom, now you're a file path!
There was a problem hiding this comment.
In fact we always get a files which does not start with file:// there.
They start with untitled:// which is VSCode-specific way to say that files does not exist on the disk yet.
I should have added a comment. @mkondratek can you fix my omission and add it now?
mkondratek
force-pushed
the
mkondratek/windows-uri-fix
branch
3 times, most recently
from
9596cd8
to
71362f0
Compare
… be performed on the agent side
mkondratek
force-pushed
the
mkondratek/windows-uri-fix
branch
from
07542cc
to
00a9e40
Compare
Fixes #1436.
Btw it fixes an issue with an error when closing a file that's content is being generated.
Test plan
Generate Unit Teston both Windows and macOs