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

Code lenses with unresolved commands show up as !!MISSING: command!! #197643

Closed
michaelpj opened this issue Nov 7, 2023 · 5 comments
Closed

Code lenses with unresolved commands show up as !!MISSING: command!! #197643

michaelpj opened this issue Nov 7, 2023 · 5 comments
Assignees
Labels
code-lens under-discussion Issue is under discussion for relevance, priority, approach

Comments

@michaelpj
Copy link

Does this issue occur when all extensions are disabled?: Yes

A code lens with no command shows up as !!MISSING: command!!.

This is clearly deliberate, see the implementation and the test for the behaviour.

However, this interacts badly with codeLens/resolve. If the resolve request fails (for example, if the document has been changed such that the old lens is now stale), then the code lens will still appear with !!!MISSING: command!!, even though this is "normal" operation.

The behaviour I would expect is for the code lens not to be displayed in this case. The current behaviour makes a lot of sense for simple code lens providers that don't rely on codeLens/resolve, since in that case not providing a command is questionable and it's useful for it to show up obviously. But in the case where we've resolved the lens and got a failure, I think it would make much more sense to just not display the lens.

This was discovered by a user of the Haskell Language Server after we implemented codeLens/resolve: https://github.com/haskell/haskell-language-server/issues/3827b

  • VS Code Version: I don't know what the user was using, but the behaviour is clearly present in the source code on master
  • OS Version: ditto

Steps to Reproduce:

  1. Write a code lens resolve handler that fails
  2. Observe ugly lenses
@jrieken
Copy link
Member

jrieken commented Dec 4, 2023

The behaviour I would expect is for the code lens not to be displayed in this case. The current behaviour makes a lot of sense for simple code lens providers that don't rely on codeLens/resolve, since in that case not providing a command is questionable and it's useful for it to show up obviously. But in the case where we've resolved the lens and got a failure, I think it would make much more sense to just not display the lens.

We don't remove these lenses to not introduce flicker (all lines would shift up by a little). I agree that today's fallback isn't great, a dash might be better, but it should also be clear that the extension should always try to resolve successfully and that failure is pretty bad here (because the UI space for code lense commands is already reserved)

@michaelpj
Copy link
Author

I can see that it's awkward. Surely indeed the point of having two phases for code lenses is to reserve the UI space. It would be nice if the unresolved code lens could give some placeholder text or something 🤔

@jrieken
Copy link
Member

jrieken commented Dec 4, 2023

Decided to go with the (existing) "no commands" rendering but we will also start to log extension errors for this

Screenshot 2023-12-04 at 16 23 29

@michaelpj
Copy link
Author

Just one thought: what about simply blank space? So we'd have the space in the UI, which would look a little odd, but we wouldn't be forcing what is ultimately a diagnostic message into the user's face unless they look in the log.

@jrieken
Copy link
Member

jrieken commented Dec 4, 2023

I experimented with that but kinda looks like a rendering glitch and you keep staring at it waiting for an update

@github-actions github-actions bot locked and limited conversation to collaborators Jan 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
code-lens under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

2 participants