Svelte duplicated auto-imports on tab

[galearez]

Summary

SvelteKit auto-generates types, so I had to make a few rounds over the options in the popup with tab, and it duplicated the import as many times as I tabbed.

See video below:
Screencast from 2023-03-10 .webm

Screencast.from.2023-03-10.18-32-07.webm

Reproduction Steps

1. Run npm create svelte@latest then select the following options:
   Which Svelte app template?
   Skeleton project
   Add type checking with Typescript?
   Yes, using Typescript syntax
   Make sure to install the dependencies AND start the dev server.

2. Create src/routes/+page.ts whit the following content:

import type { PageLoad } from "./$types";

export const load = (() => {
	return {};
}) satisfies PageLoad;

3. Go to src/routes/+page.svelte insert <script lang="ts"></script>

4. While still in src/routes/+page.svelte, inside the script tag write export let data: ActionData and here cycle, with TAB, through auto-completion options a couple times, making sure you let the docs to popup and hit ENTER

Helix log

~/.cache/helix/helix.log

please provide a copy of `~/.cache/helix/helix.log` here if possible, you may need to redact some of the lines

Platform

Fedora Linux 37 (Workstation Edition)

Terminal Emulator

gnome-terminal

Helix Version

helix 22.12

[pascalkuthe]

Could you try out if you can reproduce this with the latest master? There have been a few completion related fixes recently

[galearez]

I just build from master and the glitch is still there, but I found something interesting, apparently the glitch only happens if I let the docs to pop up, btw I am only "circling" around with TAB nothing else

With pop up:

Screencast.from.2023-03-131.webm

Without popup:

Screencast.from.2023-03-132.webm

[galearez]

I've also tried to replicate the glitch in plain TS, but there nothing happens, only using Svelte (SvelteKit exactly), so it can be something related to the Svelte LSP, maybe?

[pascalkuthe]

Could you run helix with hx -vv and post the resulting logfile here? Then I can get a better picture what is being sent back and fourth between the LSP and helix

[galearez]

I ran hx -vv <my-file> then look for the log at ~/.cache/helix/helix.log, but I don't know what the important parts are, the log file is around ~4MB and it has ~340 lines, I don't know if this is normal.

helix.log

[galearez]

Hey @pascalkuthe, I just trested your PR #6511 and the glitch is still there, I don't know if I am doing something wrong with my buildings, but if you want me to help you find a more detail description of the glitch, let me know and I will do exactly what you say me to find the problem.

[galearez]

The editor has, also, been panicking while trying move (while deleting or auto-importing) the line right bellow the <script> opening tag, the error is the following:

thread 'main' panicked at 'byte_slice(): Byte range does not align with char boundaries: range 46..51', /home/josep/.cargo/registry/src/github.aaakk.us.kg-.../ropey-1.6.0/src/slice.rs:703:23
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This bug was already described here #3283, maybe these two bugs are related?

[pascalkuthe]

Could you retry with #6594? I can not reproduce this but I never used (or heard of) sevelte before so I might be missing some setup (nothing is imported for me). I tested on TS directly (were more or less the same bug occurs) and there it's fixed now (I previously missed something). If the problem persists please post a minimal reproducible example so I can test this locally.

The crash seems to be #3283 and related to TS highlighting (probably a language specific issue). Not something to do with completions (probably just a coincidence you ran into this at the same time)

[galearez]

@pascalkuthe I tried the new one and is still the same, so I updated the reproduction steps (at the top of the issue) trying to be as clear as possible, and I am not gona lie, I am kind of worried there is something wrong with my machine and this only happens to me, but we will see, hopefully you can reproduce it.

[galearez]

@pascalkuthe I don't know if you got to try it, but I made a mistake with the reproduction steps, but it should be fixed now.

[pascalkuthe]

I was reproduce now after a while it took me a few times turns out I need to tab quite a bit.

The reason for this bug seems to be that the svelte language servers adds an additional auto-import each time we resolve the completion. VSCode only resolves completions once but the LSP standard does not specify that clients can not resolve a completion multiple times.

It makes sense to me tough to be more careful here since the point of resolving completions is to avoid expensive calculations so repeating them isn't exactly desirable. I added a commit to #6594 that makes helix remember which completions it requested (and avoid rerequesting)

[galearez]

Yeah, I only ran into it once while coding casually and I discovered it because I got distracted and kept tabbing.

I just tried your commit and it seems fixed, thanks for your contributions, you all are making an amazing tool.

BTW, how did you find out? I know a little Rust, but once I saw the error log I knew I was not ready to solve the issue myself, maybe in the future I can start making small contributions.

[pascalkuthe]

BTW, how did you find out? I know a little Rust, but once I saw the error log I knew I was not ready to solve the issue myself, maybe in the future I can start making small contributions.

I have done a lot of work on the completions recently (and fixed other similar bugs in #6594) so I had a rough idea what it was related to. Once I had a reproduction case I simply looked at the logs and found that the problem occurred if completion resolving (what happens when you however over a completion item) happened multiple times and each time one more additionalTextEdit (what generates the import statement) was generated. So I looked at how vscode does things (vscode is basically the reference implementation for a LSP client) and noticed a comment saying that they are careful to only resolve completions once which confirmed my suspicions. Implementing the fix was almost trivial: just keep an additional resolved: bool around for each completion item.

For any LSP related issue it's basically always good to look at the log and check if the request helix sends/recives line up with the LSP standard/the way VSCode does things (since the standard is kind of incomplete sometimes)

[galearez]

Very interesting, next time I will try to explore the problem a little more, and hopefully I can fix the problem or at least give a more detailed explanation of the error, thanks.