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.

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

Fix extracting path from URI #427

Merged
merged 3 commits into from
Apr 26, 2021
Merged

Conversation

rgrinberg
Copy link
Member

see if this works @mnxn

Signed-off-by: Rudi Grinberg <[email protected]>
@rgrinberg rgrinberg force-pushed the fix-uri-path-extract branch from 006f6d4 to 527d931 Compare April 26, 2021 19:37
@rgrinberg rgrinberg requested a review from mnxn April 26, 2021 19:37
Copy link
Collaborator

@mnxn mnxn left a comment

Choose a reason for hiding this comment

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

I can confirm that this fixes the interface switching issue under Windows.

@rgrinberg
Copy link
Member Author

I made the URI parsing a little better and removed the code that caused this bug altogether.

@rgrinberg rgrinberg force-pushed the fix-uri-path-extract branch from e377bd5 to 5ba613e Compare April 26, 2021 20:39
Signed-off-by: Rudi Grinberg <[email protected]>
@rgrinberg rgrinberg force-pushed the fix-uri-path-extract branch from 5ba613e to 23e1708 Compare April 26, 2021 20:43
@mnxn
Copy link
Collaborator

mnxn commented Apr 26, 2021

I think there are too many slashes at the beginning now:

[Trace - 1:46:20 PM] Sending request 'ocamllsp/switchImplIntf - (5)'.
Params: [
    "file:///c:/Users/Max/Projects/vscode-ocaml-platform/src/ocaml_lsp.mli"
]


[Trace - 1:46:20 PM] Received response 'ocamllsp/switchImplIntf - (5)' in 1ms.
Result: [
    "file:////c%3A/Users/Max/Projects/vscode-ocaml-platform/src/ocaml_lsp.ml"
]

Developer console:

[UriError]: If a URI does not contain an authority component, then the path cannot begin with two slash characters ("//")

@rgrinberg rgrinberg merged commit d6a0672 into ocaml:master Apr 26, 2021
@rgrinberg rgrinberg deleted the fix-uri-path-extract branch April 26, 2021 22:37
@mnxn
Copy link
Collaborator

mnxn commented Apr 26, 2021

@rgrinberg: I think there might be a problem with this patch too.

My projects are filled with "Unbound module" errors and there are no errors/exception in the log or developer console.
I originally thought it was a problem with my projects, but release 1.5 doesn't have this problem.

The first commit of this PR (527d931) had neither problem.

rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request May 1, 2021
CHANGES:

## Features

- Code action to annotate a value with its type (ocaml/ocaml-lsp#397)

## Fixes

- Fix interface/implementation switching on Windows (ocaml/ocaml-lsp#427)

- Correctly parse project paths with spaces and other special characters that
  must be escaped.

- Print types with `-short-paths` even if the project wasn't built yet
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