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

Incomplete highlights for complex queries (tree-sitter match limit not large enough?). #22042

Open
1 task done
JaagupAverin opened this issue Dec 15, 2024 · 2 comments
Open
1 task done
Labels
bug [core label] reproducible Verified steps to reproduce included tree-sitter Syntax highlighting and tree-sitter

Comments

@JaagupAverin
Copy link
Contributor

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

Currently the match limit for Tree-Sitter is assigned at 64 in zed/crates/language/src/syntax_map.rs:1820:

impl QueryCursorHandle {
    pub fn new() -> Self {
        let mut cursor = QUERY_CURSORS.lock().pop().unwrap_or_default();
        cursor.set_match_limit(64);
        QueryCursorHandle(Some(cursor))
    }
}

This limit appears to cause issues quite easily in Python, for example this code:

class Foo:
    def __init__(self) -> None:
        self.a = 1
        """Docstring A"""
        self.b = 2
        """Docstring B"""
        self.c = 3
        """Docstring C"""
        self.d = 4
        """Docstring D"""
        self.e = 5
        """Docstring E"""

is highlighted as follows:
image
(i.e. after a certain token, the highlighting only semi-works - the attributes and strings are captured as expected, but the string.doc capture stops being applied)
By doubling the match limit to 128, the highlighting works better, but ofc I'm only sweeping the problem under the carpet:
image

I don't know if this limit can be increased easily without performance impact (probably would need to increase it to at least 1024 or so to cover even the most complicated situations?), or in fact the issue lies somewhere else entirely, so I won't be attempting a PR myself but I'm happy if this gets a solution soon :)

Environment

Zed Preview 0.166.1 546b49c

If applicable, add screenshots or screencasts of the incorrect state / behavior

No response

If applicable, attach your Zed.log file to this issue.

No response

@JaagupAverin JaagupAverin added admin read Pending admin review bug [core label] triage Maintainer needs to classify the issue labels Dec 15, 2024
@notpeter notpeter added tree-sitter Syntax highlighting and tree-sitter reproducible Verified steps to reproduce included and removed triage Maintainer needs to classify the issue admin read Pending admin review labels Dec 16, 2024
@0x2CA
Copy link
Contributor

0x2CA commented Dec 16, 2024

Simply increasing this value will lead to performance problems, which needs to be balanced between effect and performance, different languages have different balance points, I think it may need to be modified freely according to the different languages will be better, so that most languages keep the value low to have good performance is more important, some languages allow a little sacrifice of performance for the sake of better effect. In a more exaggerated case, if the user wants to be able to configure a larger value.

Of course the time concepts of the other comments make more sense in lieu of unclear values, and it's better to let the user configure the maximum accepted delay time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [core label] reproducible Verified steps to reproduce included tree-sitter Syntax highlighting and tree-sitter
Projects
None yet
Development

No branches or pull requests

3 participants