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

Determine comment tokens from injection layers #7364

Open
the-mikedavis opened this issue Jun 17, 2023 · 6 comments
Open

Determine comment tokens from injection layers #7364

the-mikedavis opened this issue Jun 17, 2023 · 6 comments
Labels
A-core Area: Helix core improvements A-tree-sitter Area: Tree-sitter C-enhancement Category: Improvements E-medium Call for participation: Experience needed to fix: Medium / intermediate

Comments

@the-mikedavis
Copy link
Member

toggle_comments should take language injections into account. For example in HTML:

<p>
  C-c on this line should use the HTML comment token(s).
</p>
<script type="text/javascript">
  // C-c on this line should use the javascript comment token(s).
  foo();
</script>

To do this we should look at the byte range for each Range in the current Selection and find the LanguageLayer in doc.syntax() (Syntax) with the smallest/deepest range that completely covers the selection range. We will then need a way to find the LanguageConfiguration for a given LanguageLayer and then we can use that LanguageConfiguration's comment_token for that range in the selection.

The changes for this will conflict with #1505 / #4718 so this feature may be better off waiting for those changes to land.

@the-mikedavis the-mikedavis added C-enhancement Category: Improvements A-tree-sitter Area: Tree-sitter E-medium Call for participation: Experience needed to fix: Medium / intermediate A-core Area: Helix core improvements labels Jun 17, 2023
@gabydd
Copy link
Member

gabydd commented Jun 17, 2023

Thanks for making this issue especially the implementation notes, I meant to make it this morning but got caught up with work. This does conflict a tiny bit with my pr but if someone does implement this it shouldn't be too hard to rebase.

I might also just pick this up after #4718

@pascalkuthe
Copy link
Member

One complication will be that we have a language for comments that is injected into comments. We don't want to change the kind of comment tokens inside that obviously.

I think we want to do something similar for indentation (see #7381) so having shared infrastructure for detecting language inside injections would be useful

@the-mikedavis
Copy link
Member Author

the-mikedavis commented Jun 20, 2023

For indentation and textobjects I think we will want to switch to running queries across injections using something like: https://github.com/the-mikedavis/helix/blob/8483de78624be0b52dcd4144bc54a5b918d369cd/helix-core/src/syntax.rs#L1271-L1309 (currently that lives on #2857 but could be ported out). That QueryIter gives the LayerId of the current capture, so maybe the function in Syntax that gets a LayerConfiguration by byte range should actually be two functions: find the LayerId for a given byte range and then find a LanguageConfiguration for a LayerId.

@pascalkuthe
Copy link
Member

Note that I didn't mean indent queries (altough those should also be run across injections) but determining what kind of indent to use (tab, single space, four spaces,...). We could use th layerid from the query for autoindent but not for manual indent with > and < as we don't run a query there afaik

Splitting the functions in two sounds like a good idea tough 👍

@the-mikedavis
Copy link
Member Author

I was looking at this to see if it overlapped with #9320 at all. They look like separate problems unless we want to do a large refactor of how the syntax.rs types are structured (specifically HighlightConfiguration, LanguageConfiguration, Loader and LanguageLayer). I pushed up a branch with a partial solution to this: 17dd102...get-lang-config-by-injection-layer

We need to store some data on LanguageLayer so we can ask Syntax what the LanguageConfiguration is for a given injection layer. And we can use a version of #5176 (comment) acting on the Syntax layers field to get the LayerId for a given byte range.

@noor-tg
Copy link

noor-tg commented Sep 22, 2024

so any thing new about this ? or is there any experimental branch to test ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Helix core improvements A-tree-sitter Area: Tree-sitter C-enhancement Category: Improvements E-medium Call for participation: Experience needed to fix: Medium / intermediate
Projects
None yet
Development

No branches or pull requests

4 participants