Skip to content

fix(query): inefficient query of collection_type in highlights.scm#230

Merged
fdncred merged 3 commits intonushell:mainfrom
blindFS:fix_highlight_query
Oct 18, 2025
Merged

fix(query): inefficient query of collection_type in highlights.scm#230
fdncred merged 3 commits intonushell:mainfrom
blindFS:fix_highlight_query

Conversation

@blindFS
Copy link
Contributor

@blindFS blindFS commented Oct 17, 2025

More efficient queries, no more lagging when dealing with verbose type signatures:

def foo []: nothing -> list<record<id: string, title?: string, all_titles: list<record<language: string, title: string>>, description: record, links: list<string>, is_locked: bool, original_language?: string, last_volume?: string, last_chapter?: string, publication_demographic?: string, status?: string, year?: int, content_rating?: string, tags: list<record<id: string, name: string, group: string>>, state?: string, chapter_numbers_reset_on_new_volume: bool, created_at?: datetime, updated_at?: datetime, version?: int, available_translated_languages: list<string>, last_uploaded_chapter?: string, rels: list<record<id: string, type: string, related?: string, attributes?: record>>>> {
  return []
}

Originally reported by @Jan9103 on discord.

@mkatychev I'm not sure this is the best way to do it, would you like to take a look at it?

@mkatychev
Copy link
Contributor

Though this is dealing with the query perf, I figure I'd dump my grammar per notes because this stuff generally isn't in the tree-sitter book:
topiary/topiary#859 (comment)

Some highlight tests would be handy as well for this kind of stuff to get some numbers on regressions, looks better at the surface.

@mkatychev
Copy link
Contributor

These sorts of fields are better distinct as well:
image
so you'd have an easier time performance wise querying something like this:
(collection_type (field key: (_) type: (_)) (field key: (_) type: (_)))

There's likely a performance hit if you have to load many key/type fields at the same depth but that's conjecture without some perf tests.

@blindFS
Copy link
Contributor Author

blindFS commented Oct 18, 2025

Though this is dealing with the query perf, I figure I'd dump my grammar per notes because this stuff generally isn't in the tree-sitter book:

topiary/topiary#859 (comment)

Thanks, I think #185 was designed to track that as well, it gradually evolves to more WASM comp time focused though.

Some highlight tests would be handy as well for this kind of stuff to get some numbers on regressions, looks better at the surface.

Is there any example of how to do that?

@blindFS
Copy link
Contributor Author

blindFS commented Oct 18, 2025

so you'd have an easier time performance wise querying something like this:

(collection_type (field key: (_) type: (_)) (field key: (_) type: (_)))

True, there're other such patterns of anonymous field in other rules (e.g. where clauses IIRC). They seem not worth a breaking change to me (so far), so I haven't touched them yet.

There's likely a performance hit if you have to load many key/type fields at the same depth but that's conjecture without some perf tests.

Well, at least the lagging comes from another issue, which is the same to this. Your example query above shares the same pattern, likely will be laggy as well.

Copy link
Contributor

@mkatychev mkatychev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, you're right my suggestions aren't that useful. I'd be curious where cursor travel distance is heaviest.

@mkatychev
Copy link
Contributor

Though this is dealing with the query perf, I figure I'd dump my grammar per notes because this stuff generally isn't in the tree-sitter book:
tweag/topiary#859 (comment)

Thanks, I think #185 was designed to track that as well, it gradually evolves to more WASM comp time focused though.

Some highlight tests would be handy as well for this kind of stuff to get some numbers on regressions, looks better at the surface.

Is there any example of how to do that?

These are the ones I maintain:
https://github.com/bytecodealliance/tree-sitter-wit/tree/main/test/highlight

@blindFS
Copy link
Contributor Author

blindFS commented Oct 18, 2025

@mkatychev actually the suggestion of adding highlighting perf test is quite useful. I'm trying to figure out how.

@blindFS
Copy link
Contributor Author

blindFS commented Oct 18, 2025

These are the ones I maintain: https://github.com/bytecodealliance/tree-sitter-wit/tree/main/test/highlight

@mkatychev Thanks a lot, it's super helpful.

@fdncred
Copy link
Contributor

fdncred commented Oct 18, 2025

lemme know when this is ready to land

@blindFS
Copy link
Contributor Author

blindFS commented Oct 18, 2025

lemme know when this is ready to land

It's ready, please land it, thanks!

@fdncred fdncred merged commit 4f4ac86 into nushell:main Oct 18, 2025
3 checks passed
@fdncred
Copy link
Contributor

fdncred commented Oct 18, 2025

Thanks

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants