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

Implement codeLens/resolve for some code lenses #3536

Closed
michaelpj opened this issue Mar 21, 2023 · 8 comments
Closed

Implement codeLens/resolve for some code lenses #3536

michaelpj opened this issue Mar 21, 2023 · 8 comments

Comments

@michaelpj
Copy link
Collaborator

Same as #3534 but for code lenses. Again, ideally this would come with generic support to make it easy to add to new code lens providers.

@michaelpj
Copy link
Collaborator Author

It turns out that this is especially important for code lenses because of some subtlety about using stale results.

HLS lets you request either accurate or possibly-stale information. The advantage of getting the stale information is that it's much faster. Using possibly-stale information when possible improves responsiveness, and this is especially important for code lenses as they appear in the buffer, and so if they take too long to compute we can observe flickering.

However, for actually applying the edit from a code lens we ideally want fully up-to-date information. So the ideal thing to do is:

  • Use useWithStale to compute the initial codeLens response
  • Use use to compute the edit in codeLens/resolve

@July541
Copy link
Collaborator

July541 commented Apr 16, 2023

Use use to compute the edit in codeLens/resolve

We should useWithStale in resolve also, because useWithStale will return the fresh result if available, and fall back to staled if unavailable.

Particularly, I think we should use useWithStale in the whole pipeline of code lens generation.

@michaelpj
Copy link
Collaborator Author

We should useWithStale in resolve also, because useWithStale will return the fresh result if available, and fall back to staled if unavailable.

I think we actively don't want stale results though, right? Suppose I have e.g. a hotkey to trigger the type signature code lens, and I quickly edit a function to add a new argument and then hit the hotkey. I really don't want to get the type signature without the new argument - I'd much rather wait.

@wz1000
Copy link
Collaborator

wz1000 commented Apr 16, 2023

There might be a bit of confusion here, which is understandable given that there are three rather poorly named functions here

  • use/use_: these will either give you an up to date result or fail.
  • useWithStale: this will give you an up to date result, or if that fails, give you stale info. However, it will always trigger rebuilds of values and block until that operation completes.
  • useWithStaleFast: this will never block and give you the latest stale value.

@July541
Copy link
Collaborator

July541 commented Apr 16, 2023

Suppose I have e.g. a hotkey to trigger the type signature code lens, and I quickly edit a function to add a new argument and then hit the hotkey. I really don't want to get the type signature without the new argument - I'd much rather wait.

In this situation, you'll get a correct edit if your code parsed succeed, or an invalid one if parsing failed.

In some situations, the staled result still makes sense.

image

This staled lens is correct even if my code can't parse temporarily.

That means we have to trade off the value of the staled result.


As for useWithStaleFast, I think we never want this in lens?

@michaelpj
Copy link
Collaborator Author

Ah I see. Then I guess useWithStale almost certainly is right. And then maybe we should be using useWithStaleFast more often?

@July541
Copy link
Collaborator

July541 commented Apr 16, 2023

And then maybe we should be using useWithStaleFast more often?

No, useWithStaleFast should only be used with IdeAction.
https://github.com/haskell/haskell-language-server/blob/master/ghcide/src/Development/IDE/Core/Shake.hs#L1008-L1012

Most plugins are running with Action, which is returned by use and useWithStale.
https://github.com/haskell/haskell-language-server/blob/master/ghcide/src/Development/IDE/Core/Shake.hs#L955-L963

@michaelpj
Copy link
Collaborator Author

We did this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants