Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Version 0.6.3 fails under Windows under VS Code: Error file:///c%3A//... could not be converted back to a URL #62

Closed
hauserx opened this issue Nov 28, 2024 · 5 comments
Closed

Version 0.6.3 fails under Windows under VS Code: Error file:///c%3A//... could not be converted back to a URL #62

hauserx opened this issue Nov 28, 2024 · 5 comments

Comments

@hauserx
Copy link
Contributor

hauserx commented Nov 28, 2024 •
edited
Loading

From vscode logs:

[Trace - 12:41:10 PM] Sending notification 'textDocument/didOpen'.
Params: {
    "textDocument": {
        "uri": "file:///c%3A/work/bazel_plain/sample.bzl",
        "languageId": "starlark",
        "version": 1,
        "text": "x = 7;\n\ny = hello();\n"
    }
}


Error: `file:///c%3A/work/bazel_plain/sample.bzl` could not be converted back to a URL
[Info  - 12:41:10 PM] Connection to server got closed. Server will restart.

(one may enable logs using: bazel-contrib/vscode-bazel#425)

Similar, slightly different error under neovim:

[DEBUG][2024-11-29 16:44:21] .../vim/lsp/rpc.lua:286	"rpc.send"	{
  jsonrpc = "2.0",
  method = "textDocument/didOpen",
  params = {
    textDocument = {
      languageId = "bzl",
      text = "x = 7;\n\ny = hello();\n",
      uri = "file:///C:/work/bazel_plain/sample.bzl",
      version = 0
    }
  }
}
[ERROR][2024-11-29 16:44:21] .../vim/lsp/rpc.lua:770	"rpc"	"bazel-lsp"	"stderr"	
"Error: `file:///C:/work/bazel_plain/sample.bzl` could not be converted back to a URL\n"

Here is what neovim sends under Linux where it works:

[DEBUG][2024-11-29 16:56:25] .../vim/lsp/rpc.lua:286	"rpc.send"	{
  jsonrpc = "2.0",
  method = "textDocument/didOpen",
  params = {
    textDocument = {
      languageId = "bzl",
      text = "x = 7;\n\ny = hello();\n",
      uri = "file:///tmp/bazel_plain/sample.bzl",
      version = 0
    }
  }
}
@cameron-martin
Copy link
Owner

Looks like this error comes from here: https://github.com/facebook/starlark-rust/blob/7dd526ad8f2f0e275aa1575a845038544e24bfee/starlark_lsp/src/server.rs#L237

hauserx pushed a commit to hauserx/starlark-rust that referenced this issue Dec 2, 2024
Fix Windows by handling Url paths correctly.
b6a7e51 
Previous implementation was mixing Url paths with file paths.

It caused errors like:

"Error: `file:///C:/work/bazel_plain/sample.bzl` could not be converted back to a URL\n"

More: cameron-martin/bazel-lsp#62

The Url.path() is usually `"/" + path to file` which works fine as file
path under unix-like systems, but breaks under Windows, where it can be
e.g. "/c:/some/file". See
https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#syntax

Switched to using Url.to_file_path() which
represents actual file path.

In the process:
  - Enabled back Windows tests that at least for my machine started
    passing;
  - In validate() function not converting from Url to LspUrl and back,
    just returning to client Url passed in the request.
hauserx pushed a commit to hauserx/starlark-rust that referenced this issue Dec 2, 2024
Fix Windows by handling Url paths correctly.
6250d74 
Previous implementation was mixing Url paths with file paths.

It caused errors like:

"Error: `file:///C:/work/bazel_plain/sample.bzl` could not be converted back to a URL\n"

More: cameron-martin/bazel-lsp#62

The Url.path() is usually `"/" + path to file` which works fine as file
path under unix-like systems, but breaks under Windows, where it can be
e.g. "/c:/some/file". See
https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#syntax

Switched to using Url.to_file_path() which
represents actual file path.

In the process:
  - Enabled back Windows tests that at least for my machine started
    passing;
  - In validate() function not converting from Url to LspUrl and back,
    just returning to client Url passed in the request.
hauserx added a commit to hauserx/starlark-rust that referenced this issue Dec 2, 2024
@hauserx
Fix Windows by handling Url paths correctly.
477d87e 
Previous implementation was mixing Url paths with file paths.

It caused errors like:

"Error: `file:///C:/work/bazel_plain/sample.bzl` could not be converted back to a URL\n"

More: cameron-martin/bazel-lsp#62

The Url.path() is usually `"/" + path to file` which works fine as file
path under unix-like systems, but breaks under Windows, where it can be
e.g. "/c:/some/file". See
https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#syntax

Switched to using Url.to_file_path() which
represents actual file path.

In the process:
  - Enabled back Windows tests that at least for my machine started
    passing;
  - In validate() function not converting from Url to LspUrl and back,
    just returning to client Url passed in the request.
@hauserx hauserx mentioned this issue Dec 2, 2024
Closed
facebook-github-bot pushed a commit to facebook/buck2 that referenced this issue Dec 3, 2024
@hauserx @facebook-github-bot
Fix Windows by handling Url paths correctly.
c565220 
Summary:
Previous implementation was using `Url.path()` where it should use `Url.to_file_path()`

It caused errors like:

```
"Error: `file:///C:/work/bazel_plain/sample.bzl` could not be converted back to a URL\n"
```

More: cameron-martin/bazel-lsp#62

The `Url.path()` is usually `"/" + path to file` which works fine as file path under unix-like systems, but breaks under Windows, where it can be e.g. `"/c:/some/file"`. See
https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#syntax

Switched to using `Url.to_file_path()` which represents actual file path.

In the process:
  - Enabled back Windows tests that at least for my machine started passing;
  - In validate() function not converting from Url to LspUrl and back, just returning to client Url passed in the request.

X-link: facebook/starlark-rust#136

Reviewed By: KapJI

Differential Revision: D66655476

Pulled By: ndmitchell

fbshipit-source-id: b4590fb27348019424e27ee1d12ffe27d86c6f9f
facebook-github-bot pushed a commit to facebook/starlark-rust that referenced this issue Dec 3, 2024
@hauserx @facebook-github-bot
Fix Windows by handling Url paths correctly. (#136)
760df74 
Summary:
Previous implementation was using `Url.path()` where it should use `Url.to_file_path()`

It caused errors like:

```
"Error: `file:///C:/work/bazel_plain/sample.bzl` could not be converted back to a URL\n"
```

More: cameron-martin/bazel-lsp#62

The `Url.path()` is usually `"/" + path to file` which works fine as file path under unix-like systems, but breaks under Windows, where it can be e.g. `"/c:/some/file"`. See
https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#syntax

Switched to using `Url.to_file_path()` which represents actual file path.

In the process:
  - Enabled back Windows tests that at least for my machine started passing;
  - In validate() function not converting from Url to LspUrl and back, just returning to client Url passed in the request.

Pull Request resolved: #136

Reviewed By: KapJI

Differential Revision: D66655476

Pulled By: ndmitchell

fbshipit-source-id: b4590fb27348019424e27ee1d12ffe27d86c6f9f
@hauserx hauserx mentioned this issue Dec 3, 2024
Merged
cameron-martin pushed a commit that referenced this issue Dec 9, 2024
@hauserx
Upgrade starlark-rust to current HEAD (#64)
3b6992c 
This PR upgrades stalark-rust to the current HEAD and contains required
fixes to bazel-lsp. Previous version was from Sep 3, and there were a
lot of changes in the mean time.

This fixes e.g. #62

# Changes

## remove get_registered_starlark_docs

This follows change from
facebook/starlark-rust@9a7b00e
which removed it in starlark_bin

Basically get_registered_starlark_docs returned only some random symbols
built in into the binary itself, namely: list, StackFrame, dict, string


## adapt to starlark-rust rewrite DocParams

Rewrite happened in
facebook/starlark-rust@723a272
@cameron-martin
Copy link
Owner

Can this be closed now after merging #64?

@hauserx
Copy link
Contributor Author

hauserx commented Dec 13, 2024

Yes, double checked right now that it works under Windows.

For some reason was not able to build bazel-lsp on current HEAD though, running git-bisect shows this PR as culprit: #69

I am getting an errors like:

error[E0432]: unresolved import `winapi::shared::winerror`
   --> ../rules_rust~~_crate~~~crate~crates__dirs-sys-next-0.1.2/src/lib.rs:131:9
    |
131 |     use winapi::shared::winerror;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^ no `winerror` in `shared`
    |

@hauserx hauserx closed this as completed Dec 13, 2024
@cameron-martin
Copy link
Owner

Hmm, I wonder why it works on CI but not your machine. I'll check it out on my windows machine later today.

@hauserx
Copy link
Contributor Author

hauserx commented Dec 13, 2024

I have downloaded also bazel-lsp.exe from release PR job (https://github.com/cameron-martin/bazel-lsp/actions/runs/12307052626/job/34349973144?pr=80) and it works as expected.

@hauserx hauserx mentioned this issue Dec 14, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@cameron-martin @hauserx