-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
make inlay hints typing debounce max delay configurable #145377
Comments
So the reasoning here is to make them bounce less while editing the code. But if you make a change then move the caret, the hint for the original line still gets delayed. Also, should that say |
Why would you even want a delay at all for type hints for instance? It’s annoying to have the impression the editor is not responsive like it used to be. |
I don’t feel this is bad, even, this gives the feeling that the editor is really responsive. This is approximately what we had in rust-analyzer before they switched to the native inlay implementation, and I quite liked it. You don’t have to wait to see your changes. It seems not to be the taste of everyone though, so maybe we should need an option for this. However, what would be the default? Do the majority of users prefer reactivity or seeing the hints with a delay? How would the minority even be aware of such parameter to speed-up or slow-down their editor? |
I don't know. It looks a bit bad in that recording, but I used rust-analyzer before and the inlay hints never bothered me. I think you get used to it very quickly. Just so it's clear, I'm not arguing in favour of delaying updates like this, I just linked to the original issue what prompted it.
Yeah, umm... 😅. |
Oh, you’re a rust-analyzer contributor :) I’m not, hence the “they”. Hope it was not understood as a critique: it’s not. It’s a good thing to use the standard APIs, it just has currently some side effects in term of UX. |
This comment was marked as spam.
This comment was marked as spam.
I'm working on a LSP that's not rust-analyzer and I also notice the delay. In my case, the hints are very fast to compute (less than 3ms) and the hints are always shown at the end of a line. E.g.:
Therefore, it wouldn't cause any editor unresponsiveness nor the bouncing layout that's mentioned earlier. On the other hand, the current approach (waiting 1250ms) does cause a jittery layout when inserting a newline. In my example above, the inlay hint floats onto the wrong line while typing for 1250ms:
In my opinion, this behavior is much less user friendly than updating the inlay hints in real time. As a temporary work around, I tried sending a Pragmatically, I see three solutions:
|
Hey, @jrieken - was wondering when you would have a chance to look at this. Happy to update my original PR that implements this feature, or take it in an entirely different direction. Just need some feedback here. Thank you! |
That. I have added the artificial delay because rapid updates as type were perceived as very nervous, flickery and annoying. I believe that's because your cursor jumps (by more than what you type) as you type. This is esp weird when the cursor moves left as you type. The large delay is a workaround and meant to catch typing-pauses. I don't think delaying (at whatever rate) is the best solution. A better solution that comes to my mind is to update inlay hints immediately but exempt the lines at which you have typed. Or apply the new inlay hints but don't change the widths of them for the lines at which you have typed (this only works when no new inlays are added) |
Excempting the line on which you are typing wouldn't be an improvement IMHO as the main annoyance with the delays is that inlay hints are shown at locations where they shouldn't be, like in the middle of an identifier or when deleting text several inlay hints directly next to each other, due to them not being updated yet. This is happens precisely where you changed code. |
There is two things here. synchronous updates and async updates. The former are done by the editor, as you type, and aren't affected by the delay or extension's performance. @bjorn3 The cases you describe, like deleting a block of text, should be covered by that and I consider mis-renderings a bug separate from the/this issue of "badly timed async updates". So, please file a separate issue with a small code sample and edit sequence that shows bad renderings as type. Thanks |
I actually find the delay more annoying than flicker. Inlay hints being more responsive lets me see type information on code I'm typing sooner, which usually means I'm getting it right. I very much preferred the zero-delay, show it as soon as the language server updates, which is why I added it as an option, because imo, it is a preference, and some people might prefer one thing or another. It is notably one of the top complaints from when rust-analyzer switched to the built in LSP implementation. |
Screen.Recording.2023-01-03.at.12.24.14.PM.movHere's an example, where the delay works against you. I'm writing code, and unable to see the types of the arguments until I literally stop typing and have to wait. I do agree, there are those who would find the delay desirable, but there are those who find the delay to be annoying. I do not think that a good tool forces you to slow down and interrupt your thought process/flow to wait on an artificial delay for them to see information that will help them write the rest of their code. I do not think there is a right or wrong here, which is why I think that an option that allows a user to configure the delay to their tastes is the best solution for all. |
Would this be considered a misrendering bug? Screen.Recording.2023-01-03.at.5.07.36.PM.mov |
Fixed via #227377 which isn't a configurable delay but updates as type with fixed lengths. Screen.Recording.2024-09-02.at.15.38.40.mov |
Verify that inlay hints update as you type but make sure the width of those of the current line only changes after a short typing pause |
This works for me when removing characters, but not when adding, example: import * as path from 'path';
import { runTests } from '@vscode/test-electron';
async function main() {
try {
// The folder containing the Extension Manifest package.json
// Passed to `--extensionDevelopmentPath`
const extensionDevelopmentPath = path.resolve(__dirname, '../../');
// The path to test runner
// Passed to --extensionTestsPath
const extensionTestsPath = path.resolve(__dirname, './suite/index');
// Download VS Code, unzip it and run the integration test
await runTests({ extensionDevelopmentPath, extensionTestsPath });
let b = { aaaaaaaaaaaaaaaaaaa: true, b: false };
} catch (err) {
console.error('Failed to run tests');
process.exit(1);
}
}
main(); Keep on typing in the line |
recently, rust-analyzer switched to using vs code's inlay hints provider. however, due to:
vscode/src/vs/editor/contrib/inlayHints/browser/inlayHintsController.ts
Line 214 in 778b74a
However, I think that this actually makes the experience feel much worse if your language server is able to compute inlay hints quite quickly, and indeed users of the extension have noted this fact as well!
rust-lang/rust-analyzer#11733 (comment)
This feature requests proposes to make the hard-coded 1250 be configurable via user settings.
The text was updated successfully, but these errors were encountered: