breaks editor features

[claytonrcarter]

This extension seems to be interfering/breaking some editor features that need to work with language settings. For example, editor: toggle comments needs to know what line comments are used for the base language so that they can be toggled on/off. However, when Zed calls snapshot.language_scope_at(...), it resolves to this extension (ie comment) and then, when it later calls language.line_comment_prefixes(), it resolves to an empty slice/vec, because of course there are no line comments defined in this language.

Features I've encountered so far that don't work with this extension installed:

• toggle comments (toggle on works, but then toggle off doesn't)
• rewrap (in a comment)

Repro:

1. ensure this extension is not installed
2. open a file for any language that has the injection installed (eg Rust, but I also tested it with PHP and this patch)
3. run "toggle comment" several times to confirm that it works
4. install this extension
5. run "toggle comment" once → works
6. run "toggle comment" again → 💥

I'm not a Zed expert, but my hunch is that this is a fundamental limitation of how Zed handles languages and it may require changes there (in Zed) to handle stuff like this.

[thedadams]

Thanks for reporting! I will look into this to see if I can figure out what a potential solution is.

[claytonrcarter]

I think I have a potential solution: if we add new fns to Zed to allow filtering the language scopes returned by snapshot.language_scope_at(...), then, for example, toggle comments could do snapshot.language_scope_at(..., |language| !language.line_comment_prefixes().is_empty()) to get the first scope that actually has line_comment_prefixes. I just quickly tested it and it seemed to be working. I will continue to play with it and see if it can also apply to rewrap, etc.

[thedadams]

I haven't looked into this at all (because I am flying today), but my guess is that the Zed team would push back on making a change to Zed for this. Since this plugin is providing the comment language, I would bet that a language server is needed to handle this.

Again, just a guess based on my current understanding of the issue and me trying to predict what the Zed team would say.

[maxdeviant]

I ran into this issue as well.

Was seeing it in a Rust file, which I added comment injection support for: zed-industries/zed#39714

With a line comment like:

// This is a command.

Pressing cmd+/ to run editor: toggle comments does not work. Uninstalling the comment extension fixes the issue.

I thought that maybe we could address it by adding the line_comments to the comment language config, like so:

name = "comment"
grammar = "comment"
line_comments = ["// ", "/// ", "//! "]

This does fix the problem for Rust, however, it then produces problems in languages with different comment syntax (e.g., Python), where using editor: toggle comments repeatedly on a single line will produce output like this:

# // # // # // # // # // # // # // # // # // # // # 

[maxdeviant]

I would bet that a language server is needed to handle this.

A language-server based solution doesn't seem like what we want for this.

[thedadams]

@maxdeviant thanks for coming here to help out!

[claytonrcarter]

I think that this may potentially be larger than just this extension: it may affect any time an "thin" injected language "covers up" a more-relevant syntax language, and then the editor tries to read settings from the language.

@maxdeviant what do you think about making changes in Zed to handle this? I have a change locally that seems to be working for me, though I just did it and would like to try it in use for a little while. FWIW, you can preview my draft change at claytonrcarter/zed@879431e

[maxdeviant]

I think that this may potentially be larger than just this extension: it may affect any time an "thin" injected language "covers up" a more-relevant syntax language, and then the editor tries to read settings from the language.

@maxdeviant what do you think about making changes in Zed to handle this? I have a change locally that seems to be working for me, though I just did it and would like to try it in use for a little while. FWIW, you can preview my draft change at claytonrcarter/zed@879431e

I think if there's a change in Zed we can make to improve this that makes sense, we should definitely do it!

I can't say outright whether your solution is the right one or not, but if you open a PR I can get some other folks on the team involved.

[thedadams]

@claytonrcarter I'm sorry I misled you on the potential solution. I hope you're able to find a solution that works for this and the Zed team.

[claytonrcarter]

🤦 I have a fix in #4: Zed already supports this case, we just have to set hidden = true in the config. No changes needed to core!