search

zed-industries / zed

Code at the speed of thought – Zed is a high-performance, multiplayer code editor from the creators of Atom and Tree-sitter.
https://zed.dev
Other
39.33k stars 2.04k forks source link

Space being added automatically to respective closing html tags. #13551

Open diwasrimal opened 6 days ago

diwasrimal commented 6 days ago

Check for existing issues

Describe the bug / provide steps to reproduce it

This should be a bug related to auto renaming to html tags. When I want to add class="..." to a html tag, I first have to hit space before >. Doing this, the corresponding closing tag will have a space added at the end.

Environment

Zed: v0.140.5 (Zed) OS: macOS 14.5.0 Memory: 8 GiB Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

https://github.com/zed-industries/zed/assets/84910758/706b529d-55dc-4b40-93a3-5bf28b7e49c0

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

https://pastebin.com/XM8NRU07

kriskw1999 commented 2 days ago

Hi @osiewicz, I am tagging you since you've implemented the feature #12769 here.

To solve this kind of issue we could add another layer that is language specific that based on some language specific conditions (like for html jsx and tsx if is space ignore the change in auto-rename) could invalidate the auto-renaming.

Do you think it is possible and is a good solution? If so I can try implement a proposal for it. If you have any suggestion or advice let me know!

P.S. In this way we could fix somehow also this one here (I know that this is Typescript fault but I think it is a huge rock to move since it involves how the ts chunking works on a broken tree).

osiewicz commented 2 days ago

Hey @kriskw1999, I believe we may actually get away with not having any language-specific logic. Here's why: Here's a trace of communication with HTML language server in a scenario where a space is added to the opening tag of <html></html>:

// Send:
{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///Users/hiro/Projects/throwaways/glob-benchmark/file.html","version":9},"contentChanges":[{"range":{"start":{"line":0,"character":5},"end":{"line":0,"character":5}},"text":" "},{"range":{"start":{"line":0,"character":13},"end":{"line":0,"character":13}},"text":" "}]}}
// Send:
{"jsonrpc":"2.0","id":39,"method":"textDocument/linkedEditingRange","params":{"textDocument":{"uri":"file:///Users/hiro/Projects/throwaways/glob-benchmark/file.html"},"position":{"line":0,"character":6}}}
// Receive:
{"jsonrpc":"2.0","id":39,"result":null}

It goes as follows:

  1. We make edits to all associated ranges (note how a space is added to the ending tag).
  2. We refresh linked editing ranges for a file.
  3. Whoops, we notice that the range is no longer linked/valid.

Thus, we essentially allow through an edit that breaks the linked range.

Thankfully, smart folks over at Microsoft predicted this issue and supplied us with word patterns that describe contents of a valid range. The problem with spaces and whatnot is that we allow these breaking edits without any regard for language word patterns.

There's another problem though; the word pattern is an optional return value and language server does not have to return it. However, per spec, when it is not provided, we should use whatever we deem as word characters as a way to verify whether the edit is breaking or not. Thus, I'd suggest the following heuristic: if an edit contains non-word-characters, do not apply it for linked ranges. Let's also try to respect a word_pattern value if possible (though here it may be a bit tricky, as you'd have to implement some kind of Regex caching on Zed's side). Most of the time these regexes will be the same ig.

Overall, I'd say that your suggestion is fine and I've just tried to rephrase it. We don't really need a separate mechanism for language to override the treatment of linked ranges so much as we need to improve our spec compliance.

If needed, I'd be down to pair on it.

As for #13495 I'd really love to get it fixed upstream rather than work around it on our side if possible.

kriskw1999 commented 1 day ago

I completely agree with you, this solution is much more solid and clean, I've started working on it and I will reach you when I will have a draft of the solution to refine it if needed.

Btw thank you so much for the advice!

osiewicz commented 1 day ago

I'm glad to hear that. Btw. I forgot to mention that we have a char_kind func for detecting character kind in a way that respects the language configuration.