Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Make test RlsHandle transport-agnostic #1317

Merged
merged 2 commits into from
Feb 24, 2019

Conversation

Xanewok
Copy link
Member

@Xanewok Xanewok commented Feb 19, 2019

Started working towards more clean and abstract LSP client (hopefully server later on?) - first change separates the underlying transport used for the LSP client.

Another convenient change is implementing Drop for the RlsHandle so we don't need to call shutdown manually everywhere.

Next, I'd like to separate and not own directly Runtime but probably work with tokio Executors.

r? @alexheretic

@Xanewok Xanewok requested a review from alexheretic February 19, 2019 17:31
@lijinpei
Copy link
Contributor

lijinpei commented Feb 20, 2019

If needed, I think I can help with making RLS listening on multiple endpoints(uds/tcp/udp). I don't know if this functionality is needed, or have already been start to work on by others.

@bors
Copy link
Contributor

bors commented Feb 24, 2019

☔ The latest upstream changes (presumably #1320) made this pull request unmergeable. Please resolve the merge conflicts.

@Xanewok Xanewok force-pushed the wip-async-lsp-client branch from e2a5f55 to 735212f Compare February 24, 2019 22:20
@Xanewok
Copy link
Member Author

Xanewok commented Feb 24, 2019

Thanks for offering your help!
With this being done in this way, anything that implements AsyncWrite + AsyncRead should Just Work^TM, although this is only done for the RlsHandle, which mostly acts as our integration test helper.

However, I'd love to separate that into a more principled and transport-agnostic LSP client+server on which we can build this RlsHandle, use it for our main LSP server or even make it an easy crate that other LSP developers can build upon.

Do you have experience with Tokio? I'd love to know how best to create a simple and reusable abstraction for a type-safe LSP server (sends Notifications and handles incoming Requests) and client (sends Requests and handles incoming notifications).

@Xanewok
Copy link
Member Author

Xanewok commented Feb 24, 2019

Meanwhile, this PR sort of makes RlsHandle something in the middle of being a somewhat reusable LSP client and specifically our integration test harness but hopefully we'll hash it out in the follow up PRs.

@bors r+

@bors
Copy link
Contributor

bors commented Feb 24, 2019

📌 Commit 735212f has been approved by Xanewok

@bors
Copy link
Contributor

bors commented Feb 24, 2019

⌛ Testing commit 735212f with merge b5a02e0...

bors added a commit that referenced this pull request Feb 24, 2019
Make test `RlsHandle` transport-agnostic

Started working towards more clean and abstract LSP client (hopefully server later on?) - first change separates the underlying transport used for the LSP client.

Another convenient change is implementing `Drop` for the `RlsHandle` so we don't need to call shutdown manually everywhere.

Next, I'd like to separate and not own directly `Runtime` but probably work with tokio Executors.

r? @alexheretic
@bors
Copy link
Contributor

bors commented Feb 24, 2019

☀️ Test successful - checks-travis
Approved by: Xanewok
Pushing b5a02e0 to master...

@bors bors merged commit 735212f into rust-lang:master Feb 24, 2019
@lijinpei
Copy link
Contributor

I am not familiar with tokio, but I think problems that needs to be considered include:

  1. tokio's actor may spawn new threads. This is in fact not big deal but conceptually that thread is out of RLS's control.
  2. tokio may convert to futures 0.3 in the future, and that will bring a breaking change
    Tracking issue for async / await tokio-rs/tokio#804
    https://areweasyncyet.rs/

bors added a commit to rust-lang/rust that referenced this pull request Mar 4, 2019
Update cargo, rls

## cargo

6 commits in 5c6aa46e6f28661270979696e7b4c2f0dff8628f..716b02cb4c7b75ce435eb06defa25bc2d725909c
2019-02-22 19:32:35 +0000 to 2019-03-02 14:23:51 +0000
- Some test/bench-related tweaks (rust-lang/cargo#6707)
- Fix links to the permanent home of the edition guide. (rust-lang/cargo#6703)
- HTTPS all the things (rust-lang/cargo#6614)
- Cargo test quicker by not building untested examples when filtered (rust-lang/cargo#6683)
- Link from ARCHITECTURE.md to docs.rs and github (rust-lang/cargo#6695)
- Update how to install rustfmt (rust-lang/cargo#6696)

## rls

9 commits in 0d6f53e1a4adbaf7d83cdc0cb54720203fcb522e..6a1b5a9cfda2ae19372e0613e76ebefba36edcf5
2019-02-14 07:52:15 +0000 to 2019-03-04 20:24:45 +0000
- Update cargo and clippy. (rust-lang/rls#1388)
- catch up rust-lang/rust PR#58321 (rust-lang/rls#1384)
- Apply Clippy fixes (rust-lang/rls#1327)
- Various cosmetic improvements (rust-lang/rls#1326)
- Make test `RlsHandle` transport-agnostic (rust-lang/rls#1317)
- Translate remaining tests (rust-lang/rls#1320)
- Remove unnecessary #![feature]s (rust-lang/rls#1323)
- Update Clippy (rust-lang/rls#1319)
- Update Clippy (rust-lang/rls#1315)

cc @Xanewok
bors added a commit to rust-lang/rust that referenced this pull request Mar 9, 2019
Update cargo, rls, books

## cargo

10 commits in 5c6aa46e6f28661270979696e7b4c2f0dff8628f..95b45eca19ac785263fed98ecefe540bb47337ac
2019-02-22 19:32:35 +0000 to 2019-03-06 19:24:30 +0000
- Relax some rustdoc tests. (rust-lang/cargo#6721)
- Include build script execution in the fingerprint. (rust-lang/cargo#6720)
- part of the infrastructure for public & private dependencies in the resolver (rust-lang/cargo#6653)
- Bump to 0.36.0 (rust-lang/cargo#6718)
- Some test/bench-related tweaks (rust-lang/cargo#6707)
- Fix links to the permanent home of the edition guide. (rust-lang/cargo#6703)
- HTTPS all the things (rust-lang/cargo#6614)
- Cargo test quicker by not building untested examples when filtered (rust-lang/cargo#6683)
- Link from ARCHITECTURE.md to docs.rs and github (rust-lang/cargo#6695)
- Update how to install rustfmt (rust-lang/cargo#6696)

## rls

9 commits in 0d6f53e1a4adbaf7d83cdc0cb54720203fcb522e..6a1b5a9cfda2ae19372e0613e76ebefba36edcf5
2019-02-14 07:52:15 +0000 to 2019-03-04 20:24:45 +0000
- Update cargo and clippy. (rust-lang/rls#1388)
- catch up rust-lang/rust PR#58321 (rust-lang/rls#1384)
- Apply Clippy fixes (rust-lang/rls#1327)
- Various cosmetic improvements (rust-lang/rls#1326)
- Make test `RlsHandle` transport-agnostic (rust-lang/rls#1317)
- Translate remaining tests (rust-lang/rls#1320)
- Remove unnecessary #![feature]s (rust-lang/rls#1323)
- Update Clippy (rust-lang/rls#1319)
- Update Clippy (rust-lang/rls#1315)

cc @Xanewok

## Books
See #58936 for details.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants