Allow attaching to URI schemes other than the 'file' scheme #1758

rwols · 2021-06-20T12:49:55Z

This allows servers to attach to schemes like:

buffer: in-memory buffers
res: ST resource files in .sublime-packages
other foreign schemes, like deno, json-schema, jdt, etc

The way this works is that we have a new client configuration setting
called "schemes" which is a list of allowable URI schemes.

If "schemes" is not specified, then it's by default ["file"].

Resolves #1739

I've tried this with LSP-json, LSP-pyright and these language servers
are working well with non-file URIs. clangd on the other hand refuses
to work with in-memory buffers.

This allows servers to attach to schemes like: - `buffer`: in-memory buffers - `res`: ST resource files in .sublime-packages - other foreign schemes, like `deno`, `json-schema`, `jdt`, etc The way this works is that we have a new client configuration setting called `"schemes"` which is a list of allowable URI schemes. If `"schemes"` is not specified, then it's by default `["file"]`.

plugin/core/sessions.py

rchl · 2021-06-20T21:50:10Z

plugin/documents.py

+    def _on_settings_object_changed(self) -> None:
+        new_syntax = self.view.settings().get("syntax")
+        if new_syntax != self._current_syntax:
+            self._current_syntax = new_syntax
+            self._cleanup()
+            self._setup()
+


Don't we have to unregister the listener and then register it back and trigger "on_activated" to trigger didOpen?

No need: container of SessionViews is cleared, _is_registered is set to False, and then on_activated_async is called by ST.

I haven't checked but I've assumed that on_activated_async won't necessarily be called. Not unless the user changes the focus of the files manually.

fixed in 5294c95

rchl · 2021-06-21T10:18:25Z

Don't have time to investigate now but noticed that languageId is empty for LSP-eslint (at least in a Vue file).

rwols · 2021-06-25T20:29:24Z

@rchl in what circumstances is languageid empty? do you have an example?

rchl · 2021-06-25T21:06:07Z

We don't seem to be handling file renames and are sending the old file name to the server even after rename.

rchl · 2021-06-25T21:46:48Z

Also found some bad issue where LSP gets confused with what file (URI) was closed and reports one for a different file.

I'm not entirely sure how this happens but have sort of solid repro so can look closer when I have more time.

EDIT: It's probably related to backward-compatibility problem with files that were already open before those changes were introduced.

rwols · 2021-06-26T11:53:55Z

We don't seem to be handling file renames and are sending the old file name to the server even after rename.

Works fine for me in pyright:

::  -> LSP-pyright textDocument/didClose: {'textDocument': {'uri': 'buffer://sublime/156'}}
::  -> LSP-pyright textDocument/didOpen: {'textDocument': {'version': 15, 'text': 'import functools\n\n', 'uri': 'file:///Users/raoulwols/Desktop/test2.py', 'languageId': 'python'}}

pyright does have problems with publishing diagnostics though. It publishes diagnostics for file:///156 instead of buffer://sublime/156. I think that's a bug in pyright.

rwols · 2021-06-26T11:59:07Z

Saving existing files on disk to a different filename also works:

::  -> LSP-pyright textDocument/didClose: {'textDocument': {'uri': 'file:///Users/raoulwols/Desktop/test2.py'}}
::  -> LSP-pyright textDocument/didOpen: {'textDocument': {'languageId': 'python', 'version': 1, 'uri': 'file:///Users/raoulwols/Desktop/test3.py', 'text': 'import functools\n'}}

rchl · 2021-06-26T11:59:51Z

I don't mean saving files. I mean open a normal file and use rename command on it.

rwols · 2021-06-26T12:08:25Z

Seems to be missing callbacks in the ST API.

rwols · 2021-06-26T12:10:48Z

made an issue for it sublimehq/sublime_text#4540

rchl · 2021-06-26T12:40:28Z

Instead of waiting for a fix in ST, maybe we should consider only falling back to a saved URI when the file has no filename so that we in most cases always access native view.file_name(). Otherwise it's easy for regressions and corner cases to bite us (besides the already mentioned one).

rwols · 2021-06-26T14:17:30Z

maybe we should consider only falling back to a saved URI when the file has no filename

Sorry, could you expand more on what you mean here?

rchl · 2021-06-26T15:12:08Z

Basically what I'm thinking is to only set lsp_uri in the view if the view has no file name. Then the get_uri method would get the lsp_uri from view, if set, otherwise calculate the URI from view.file_name() each time.

I'm not sure how much of a performance impact it would have but it would ensure that we always ask the real source of truth for the file name instead of relying on our cached value and having to worry about invalidating it in all cases.

This reverts commit 8040cb5.

This reverts commit b723eb6.

rwols · 2021-06-26T17:30:50Z

Trying to figure out why the CI fails

This reverts commit 8f8c394.

rwols · 2021-06-26T18:40:58Z

Something in 3a495bc makes the CI fail spectacularly :D

rchl · 2021-06-27T22:01:34Z

What's your plan regarding this issue: #1758 (comment) ?

Integrate with that flaw, wait or have my suggested workaround?

While previously we didn't detect the renames either (I guess those would ideally trigger didClose and didOpen), now renames cause various errors with various servers (I guess because we notify about file that is no longer on disk).

rwols · 2021-06-27T22:24:52Z

I would go ahead with this in spite of not handling renames as it doesn’t work on main as well.

plugin/session_buffer.py

rchl · 2021-06-28T08:44:27Z

plugin/core/url.py

 def uri_to_filename(uri: str) -> str:
+    """
+    DEPRECATED: An URI associated to a view does not necessarily have a "file:" scheme.
+    Use urllib.parse.urlparse to determine the scheme and go from there.
+    Use urllib.parse.unquote to unquote the path.
+    """
    parsed = urlparse(uri)
    assert parsed.scheme == "file"
    if os.name == 'nt':
        # url2pathname does not understand %3A (VS Code's encoding forced on all servers :/)
        return url2pathname(parsed.path).strip('\\')
    else:
        return url2pathname(parsed.path)
+
+


Can we have an alternative helper function for getting filename from URI? I'd need to use something in eslint.

Having to play with urllib doesn't sound like fun and also I wonder if such helper needs to apply some path transformations to stay consistent.

I imagine such helper could return a tuple with scheme and file path (if file scheme at least).

Let's also do that in another PR?

rchl · 2021-06-28T08:51:13Z

I would go ahead with this in spite of not handling renames as it doesn’t work on main as well.

But I could argue that the situation is worse now. Previously at least the server would more or less happily work after rename. Now there are errors like:

LSP-eslint: ENOENT: no such file or directory, realpath '/xxx/app/pages/notifications.vue'

Though, won't hold it against this change as previous behavior wasn't perfect either.

…sp#1758) This allows servers to attach to schemes like: - `buffer`: in-memory buffers - `res`: ST resource files in .sublime-packages - other foreign schemes, like `deno`, `json-schema`, `jdt`, etc The way this works is that we have a new client configuration setting called `"schemes"` which is a list of allowable URI schemes. If `"schemes"` is not specified, then it's by default `["file"]`.

rwols requested a review from rchl June 20, 2021 12:49

rwols commented Jun 20, 2021

View reviewed changes

plugin/core/sessions.py Show resolved Hide resolved

rwols added 2 commits June 20, 2021 15:40

tests prevented a bug

da181b9

flake8

bd77113

rwols mentioned this pull request Jun 20, 2021

Set up URI schemes sublimelsp/LSP-json#96

Merged

does this fix it?

2bca433

rwols mentioned this pull request Jun 20, 2021

Enable uri schemes sublimelsp/LSP-jdtls#15

Merged

rchl reviewed Jun 20, 2021

View reviewed changes

rwols added 4 commits June 21, 2021 22:07

Fix windows-specific code for URIs

7635781

revert last commit

f120b1c

no need for a global map!

63c3668

Merge branch 'main' into feat/attach-to-non-file-uris

8856f73

rwols mentioned this pull request Jun 23, 2021

fix: work around ST bug sublimelsp/LSP-yaml#36

Merged

rwols added 4 commits June 23, 2021 23:22

Fix code action params

6aa3ad7

remove unused import

40fbb99

Keep compatibility with LSP-eslint

b723eb6

I guess we already had compatibility?

8040cb5

rwols mentioned this pull request Jun 25, 2021

feat: enable pyright in raw buffers and ST resource files sublimelsp/LSP-pyright#77

Merged

rwols added 2 commits June 26, 2021 16:34

call self.on_activated_async explicitly after resetting

5294c95

Merge branch 'main' into feat/attach-to-non-file-uris

26b60ff

rwols added 3 commits June 26, 2021 18:58

Merge branch 'main' into feat/attach-to-non-file-uris

2fa323d

Revert "I guess we already had compatibility?"

8f8c394

This reverts commit 8040cb5.

Revert "Keep compatibility with LSP-eslint"

c94d210

This reverts commit b723eb6.

Revert "Revert "I guess we already had compatibility?""

3a495bc

This reverts commit 8f8c394.

rwols added 3 commits June 27, 2021 01:52

Load language ID earlier

ce180b7

Tests prevented a bug

0043286

Merge branch 'main' into feat/attach-to-non-file-uris

f61246a

rwols requested a review from rchl June 27, 2021 21:54

rchl reviewed Jun 28, 2021

View reviewed changes

rwols added 2 commits June 29, 2021 22:02

rename to last_known_uri

438f700

Merge branch 'main' into feat/attach-to-non-file-uris

1b5e241

rwols merged commit 7673446 into main Jun 29, 2021

rwols deleted the feat/attach-to-non-file-uris branch June 29, 2021 20:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow attaching to URI schemes other than the 'file' scheme #1758

Allow attaching to URI schemes other than the 'file' scheme #1758

rwols commented Jun 20, 2021

rchl Jun 20, 2021

rwols Jun 23, 2021

rchl Jun 25, 2021

rwols Jun 26, 2021

rchl commented Jun 21, 2021

rwols commented Jun 25, 2021

rchl commented Jun 25, 2021

rchl commented Jun 25, 2021 •

edited

Loading

rwols commented Jun 26, 2021

rwols commented Jun 26, 2021

rchl commented Jun 26, 2021

rwols commented Jun 26, 2021

rwols commented Jun 26, 2021

rchl commented Jun 26, 2021 •

edited

Loading

rwols commented Jun 26, 2021

rchl commented Jun 26, 2021 •

edited

Loading

rwols commented Jun 26, 2021

rwols commented Jun 26, 2021

rchl commented Jun 27, 2021 •

edited

Loading

rwols commented Jun 27, 2021

rchl Jun 28, 2021

rwols Jun 29, 2021

rchl commented Jun 28, 2021

Allow attaching to URI schemes other than the 'file' scheme #1758

Allow attaching to URI schemes other than the 'file' scheme #1758

Conversation

rwols commented Jun 20, 2021

rchl Jun 20, 2021

Choose a reason for hiding this comment

rwols Jun 23, 2021

Choose a reason for hiding this comment

rchl Jun 25, 2021

Choose a reason for hiding this comment

rwols Jun 26, 2021

Choose a reason for hiding this comment

rchl commented Jun 21, 2021

rwols commented Jun 25, 2021

rchl commented Jun 25, 2021

rchl commented Jun 25, 2021 • edited Loading

rwols commented Jun 26, 2021

rwols commented Jun 26, 2021

rchl commented Jun 26, 2021

rwols commented Jun 26, 2021

rwols commented Jun 26, 2021

rchl commented Jun 26, 2021 • edited Loading

rwols commented Jun 26, 2021

rchl commented Jun 26, 2021 • edited Loading

rwols commented Jun 26, 2021

rwols commented Jun 26, 2021

rchl commented Jun 27, 2021 • edited Loading

rwols commented Jun 27, 2021

rchl Jun 28, 2021

Choose a reason for hiding this comment

rwols Jun 29, 2021

Choose a reason for hiding this comment

rchl commented Jun 28, 2021

rchl commented Jun 25, 2021 •

edited

Loading

rchl commented Jun 26, 2021 •

edited

Loading

rchl commented Jun 26, 2021 •

edited

Loading

rchl commented Jun 27, 2021 •

edited

Loading