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

Consider setting idle timeout to 0 by default #5054

Closed
matklad opened this issue Dec 7, 2022 · 8 comments
Closed

Consider setting idle timeout to 0 by default #5054

matklad opened this issue Dec 7, 2022 · 8 comments
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements

Comments

@matklad
Copy link
Contributor

matklad commented Dec 7, 2022

Once again test-driving helix today, and one thing I've noticed are somewhat sluggish completions. Luckily, I had enough background knowledge to google for some artificial delay, but I fear that most users would just assume the thing is slow. 400ms seems like the maximally annoying timeout -- it's clearly human perceptible, and yet still fast enough to feel basically ok. I think that maybe just setting this to 100 would be good enough?

@archseer
Copy link
Member

archseer commented Dec 8, 2022

Is it really that slow? At 400 it seems almost instant: it'll autocomplete any time I pause typing. I'll consider lowering it to something like 200-250.

Right now I'd recommend setting idle timeout to at least 1, it's used for some other things like triggering async highlighting in the file picker. If you set it to 0 it'll likely start highlighting lots of documents if you just scroll through the entries.

@matklad
Copy link
Contributor Author

matklad commented Dec 8, 2022

At 400 it seems almost instant

It is, and that’s I think is the problem for the first impression: It definitely is a perceptible, annoying delay. At the same time, it is fast enough to seem to be a performance problem, rather than an artificial delay.

Right now I'd recommend setting idle timeout to at least 1, it's used for some other things like triggering async highlighting in the file picker.

Good tip, thanks. Might put that in the docs alongside the current advice about 0.

If you set it to 0 it'll likely start highlighting lots of documents if you just scroll through the entries.

The ideal way to solve this is to highlight only the prefix of the doc which fits on the screen. That should be O(1) time and fast enough to basically always do.

@the-mikedavis
Copy link
Member

The ideal way to solve this is to highlight only the prefix of the doc which fits on the screen

Tree-sitter may need the full document to parse the syntax tree or for syntax highlighting queries to match correctly. (See #609)

@matklad
Copy link
Contributor Author

matklad commented Dec 8, 2022

FWIW, the way this works in IDEA is that lexing and parsing phases are separate, lexing is super-fast, incremental and robust, and syntax highlighting is done in two passes: lexemes give basic colors, and then syntax (and actual semantic analysis on top) give finer shades of meaning to tokens. So there's an perceptual small change between sync, fast, but approximate results, and precise but async results.

@pascalkuthe
Copy link
Member

FWIW, the way this works in IDEA is that lexing and parsing phases are separate, lexing is super-fast, incremental and robust, and syntax highlighting is done in two passes: lexemes give basic colors, and then syntax (and actual semantic analysis on top) give finer shades of meaning to tokens. So there's an perceptual small change between sync, fast, but approximate results, and precise but async results.

That is an interesting approach. I had been thinking about something similar because tree-sitter highlights slow down the editor for very large files (20MB+) as they are performed synchronously. My idea was to perform them asyncronously so we put them on a timeout and render without syntax highlighting after a certain delay (which would then be added later as you described). The approach you described would be a better version of that by always providing lexical highlighting at least. Sadly treesitter does not expose a way to perform lexing separately from parsing AFAIK so we will probably not be able to implement this.

@kirawi kirawi added C-enhancement Category: Improvements A-helix-term Area: Helix term improvements labels Dec 9, 2022
@vasylnakvasiuk
Copy link

I also feel some discomfort, when I’m using autocomplete with default idle-timeout value.

For instance, I have the next Python code:

from itertools import accumulate

So now I want to use accumulate somewhere, and press ac + Tab (instantly) for that. The same, for example, when I’m using built-in keywords, like import, class, and so on and so forth. I press i (or im) + Tab (instantly) and I get import, as I expect.

If I’m exploring and don’t know exactly, what method/class/etc is called – maybe 400ms is ok. But in other cases, when you know what you want to type, it makes you wait and there is a feeling that either the LSP server slows down a little, or the editor itself.

If we talk about code highlighting on the file picker preview – again at the beginning I thought that it’s an editor’s lag (Telescope.nvim highlight the code instantly, and my expectation was that helix does the same). And maybe would be nice to mention in the documentation, that idle-timeout uses for code highlighting as well.

@Z2Up1UwcaYOyZq
Copy link

It would be nice if Helix could mention idle-timeout (as well as completion-trigger-len) when typing :set auto-completion so I know that there are options to change the behaviour instead of thinking there is only the on/off switch

@pascalkuthe
Copy link
Member

Addressed by #8021. Completikns are now triggered much more reliably (only edits reset the timeout) and the timeout has been reduced to 200ms (and reducing it to low values like 10ms is actually not broken now). We don't want to lower the default any lower since we prefer a slightly longer timeout. This is somewhat su jective so with the option of lowering it further being property supported now I think we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements
Projects
None yet
Development

No branches or pull requests

7 participants