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

Move vfs-racer impls to RLS #964

Merged
merged 1 commit into from
Jul 29, 2018
Merged

Move vfs-racer impls to RLS #964

merged 1 commit into from
Jul 29, 2018

Conversation

matklad
Copy link
Member

@matklad matklad commented Jul 27, 2018

That way, we don't need to maintain racer version in VFS.

Right now, for example, I can't say racer = { path = "../racer" } in rls/Cargo.toml, b/c that won't be the same racer that rls_vfs usues. But given that the amount of code in question is tiny, it seems operationally more effective to just write a newtype wrapper inside rls.

That way, we don't need to maintain racer version in VFS.
@kngwyu
Copy link
Contributor

kngwyu commented Jul 28, 2018

I'm for this PR.
It makes it much easier to write a PR for both rls and racer.

@nrc nrc merged commit a528e2f into rust-lang:master Jul 29, 2018
@nrc
Copy link
Member

nrc commented Jul 29, 2018

Thanks!

Copy link
Member

@Xanewok Xanewok left a comment

Choose a reason for hiding this comment

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

LGTM! Affected scope is small and it keeping it in one place vs distributed across crates seems like a win in this case.

@@ -321,7 +323,7 @@ impl RequestAction for Completion {
let vfs = ctx.vfs;
let file_path = parse_file_path!(&params.text_document.uri, "complete")?;

let cache = racer::FileCache::new(vfs);
let cache = racer_cache(vfs);
Copy link
Member

Choose a reason for hiding this comment

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

I realize this is minor and a thing of preference, but personally I'd find it a lot easier to read with racer::FileCache::new(RacerVfs(vfs)) as this limits what I'd expect to need to know - here it'd just be a impl racer::FileLoader for RacerVfs/*(Arc<Vfs>)*/ that is hiding somewhere 😉

Super minor though!

@Xanewok
Copy link
Member

Xanewok commented Jul 29, 2018

I just wanted to ping you @nrc bit it seems you were faster!

@matklad matklad deleted the racer-vfs branch July 29, 2018 21:11
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.

4 participants