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

Conversation

@p-avital
Copy link

Hello There!

I wanted to have Lenses to show the types of variables in the editor much like CLion does in VSCode. Here is my first go at it.
I'm pretty sure it won't be merged as is, but input on how to get it up to your standards (and maybe some pointers on how to actually code it) are welcome.

For now, it only shows types for variables that are being declared alone with a let (mut) var, and refreshes slightly too early (you need to type a bit to wake it up after just writing a new declaration).

It also does no typename compacting, so the fact that I haven't made the lenses inline is a plus for now in my opinion, but it's a little invasive, so it should probably become optional if it is to stay that way.

Here's a little example of how it looks.
image

I had to update clippy-lints to get it to compile, in case you wonder why I changed the ref in Cargo.toml.

Have a nice day :)

For now, it only shows types for variables that are being declared alone with a `let (mut) var`, and refreshes slightly too early (you need to type a bit to wake it up after just writing a new declaration.
It also does no typename compacting, so the fact that I haven't made the lenses inline is a plus for now.
@p-avital
Copy link
Author

Travis CI says it doesn't compile, maybe its version of rustc is lower than the 1.36 I'm using ?

@Xanewok
Copy link
Contributor

Xanewok commented Apr 30, 2019

Hi! Sorry for the delayed response; this looks awesome!
Right now I’m away do I’ll have time to properly review this next week, I’m afraid.
I’d also be wary of doing a lot of manual pretty printing and type manipulation ourselves, it’d be good to somehow keep this in rustc itself (glanced over the last commit).
Sorry and hope you don’t mind to wait a bit!

@p-avital
Copy link
Author

Hi,

I don't mind waiting a bit, I wasn't expecting this to be merged as is anyway.
Yeah, the latest commit has some doubtful code in it, it's mostly an attempt at name shortening, and is completely a work in progress for now. If you have ideas to get the name shortening from rustc, I'm listening. The printing is only there for testing purposes, I'll remove it soon.

My main concern is the update timing of the lenses though: as of today, lenses are refreshed on save and on editor change. Due to this, indexing is not finished by the time lenses are asked, so you have to do something manually after finishing your edit to a declaration statement in order to refresh the lenses.
I've tried providing a CodeLensResolver implementation and sending unresolved lenses (with the command field set to None), but I just end up with VSCode displaying !!MISSING COMMAND!! instead of attempting to resolve them later.
Would you have an idea in order to trigger CodeLens resolution (or maybe requesting) at the end of indexing ?

Finally, if you know how to get my lenses nicely inlined with their declaration, I could start trying to place them inside other patterns too (closures, pattern matching...)

Hopefully we can get this feature up and running soon, it's the only thing I really miss from CLion (well, this and match arms completion in the code fixes)

See you.

@bors
Copy link
Contributor

bors commented May 1, 2019

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

@p-avital
Copy link
Author

p-avital commented May 3, 2019

Just a little a parte on typename shortening: although I agree we should leave this to rustc, my lack of knowledge of it made implementing this more efficient as a PoC.

The implementation I give there is just a way to get the imports and named imports from the current file and use them in the lenses, so that when I do find a way to display them inline, we don't all suddenly need 21/9 monitors 😃

I've seen cases where let x: Type = ... will have a lens showing some::namespace::Type, so obviously it's not an exact mapping to what rustc might give, but I find the benefit in it that names shown by the lenses are easy to locate, so I might like this implementation better on that point.

@p-avital
Copy link
Author

p-avital commented May 6, 2019

After eating some Swiss chocolate, I think I found why my lenses won't go inline: they simply can't, GitLens uses decorations for the inline stuff.
Welp, time to try some edits to the client side then I guess.

@p-avital
Copy link
Author

p-avital commented May 7, 2019

Ok, since I can't inline them yet (see previous comment), I've gone with plan B: the lenses now show the name of the variable they give a type for.
And now, basic support for tuple unpacking, closure parameters and match arms has arrived. Gaze at its beauty!

image

Anyway, some cleanup is probably necessary, and that many lenses with the same old refresh not happening at end of indexing issue is probably too intrusive to not be optional, but I'm gonna start using it as is.

Note that I had to add a show_name(span) method to rls-analysis::AnalysisHost to get the names of my spans, so rls-analysis is now out of sync with cargo by 3 lines.

p-avital and others added 6 commits May 8, 2019 21:12
clippy revision dependency has caused me many headaches with rustc versions, hopefully this makes it a bit less painful
Lens range moved to end of work for tuple and parameter unpacking as prelude to Decoration based type hints that I plan for the VSCode client
@p-avital
Copy link
Author

p-avital commented May 9, 2019

STOP
I've tried working with Decorations on the VSCode side, and that works so much better!
Closing this PR, since it's redundant in purpose with the other one, but far inferior in functionality.
Have a nice day :)

@p-avital p-avital closed this May 9, 2019
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