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

bug: go to definition causes error in Neovim v0.10.x #893

Closed
axzilla opened this issue Aug 21, 2024 · 11 comments · Fixed by #899
Closed

bug: go to definition causes error in Neovim v0.10.x #893

axzilla opened this issue Aug 21, 2024 · 11 comments · Fixed by #899
Labels
bug Something isn't working

Comments

@axzilla
Copy link

axzilla commented Aug 21, 2024

Description

When using the "Go to Definition" feature in Neovim with templ components, an error occurs specifically for the @atoms.Input component. This error doesn't happen for other components or when the file containing the Input component is already open. Importantly, this issue does not occur in Visual Studio Code, suggesting it might be specific to the Neovim integration.

Code Example

WaitlistForm component:

package organisms
import (
	"github.com/axzilla/goilerplate/components/atoms"
	"github.com/axzilla/goilerplate/forms"
)
templ WaitlistForm(values forms.Waitlist, errors map[string]string) {
	<form hx-post="/waitlist" class="flex flex-col gap-2 w-96" hx-swap="outerHTML">
		if errors["success"] != "" {
			<div role="success" class="alert border border-primary">
				<div class="flex flex-col gap-2 justify-center items-center text-center font-bold">
					<p>🚀 You're in! 🚀</p>
					<p>Smart move, future SaaS superstar! We'll buzz your inbox when it's time to launch.</p>
				</div>
			</div>
		}
		@atoms.Input(templ.Attributes{
			"value":       values.Email,
			"name":        "email",
			"placeholder": "Email",
		}, "", errors["email"])
		<button data-loading-disable type="submit" class="btn btn-primary">Let's Go</button>
	</form>
}

Input component:

package atoms
templ Input(attrs templ.Attributes, label, err string) {
	<div class="flex flex-col gap-2">
		if label != "" {
			<p class="">{ label }</p>
		}
		<input { attrs... } class="input input-bordered"/>
		if err != "" {
			<p class="text-red-500">{ err }</p>
		}
	</div>
}

Error Message

When trying to use "Go to Definition" on @atoms.Input, the following error occurs:

Error executing vim.schedule lua callback: ...ar/neovim/0.10.1/share/nvim/runtime/lua/vim/lsp/util.lua:1914: index out of range
stack traceback:
	[C]: in function '_str_byteindex_enc'
	...ar/neovim/0.10.1/share/nvim/runtime/lua/vim/lsp/util.lua:1914: in function 'locations_to_items'
	...eovim/0.10.1/share/nvim/runtime/lua/vim/lsp/handlers.lua:435: in function 'handler'
	.../neovim/0.10.1/share/nvim/runtime/lua/vim/lsp/client.lua:687: in function ''
	vim/_editor.lua: in function <vim/_editor.lua:0>

Environment

  • Neovim version: 0.10.1
  • templ version: v0.2.747
  • Operating System: macOS

Additional Information

  • The error only occurs for the @atoms.Input component in Neovim.
  • "Go to Definition" works for other components in Neovim.
  • The error doesn't occur if the file containing the Input component is already open in Neovim.
  • This issue does not occur in Visual Studio Code, where "Go to Definition" works as expected.
  • A minimal Neovim configuration has been tested, and the issue persists.

Expected Behavior

"Go to Definition" should successfully navigate to the definition of the Input component without throwing an error, as it does in Visual Studio Code.

Questions

  1. Is there an error in the provided code that could be causing this issue?
  2. Are there any known incompatibilities between templ and Neovim 0.10.1?
  3. Could this be related to how the templ language server is interacting with Neovim's LSP client?

Any insights or suggestions for troubleshooting would be greatly appreciated. Thank you for your time and effort in addressing this issue.

@a-h
Copy link
Owner

a-h commented Aug 24, 2024

I'm still using Neovim v0.9.5. I'll have to try out Neovim v0.10.1 and see if I get the same issue.

I spotted some emojis in the text. Does the problem go away if there are no emojis? Emojis are multi-byte characters, so they cause a lot of "fun".

The reason I ask is that LSP positions were recently updated in the LSP spec: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocuments to now support UTF-8 encoding positions.

I did a bit of work on positions in May (#712) around multi-byte characters throwing off LSP positions, so there's some good tests in there around it.

It could be that the LSP code in Neovim is now trying to do some sort of negotiation as to what position to use, and is getting confused about text position, since it works fine in Neovim v0.9.5.

The fix might be to populate the correct parts of the LSP client / server neogitation to tell Neovim to use UTF-8 for positions (I think that's what VS Code does).

If you feel like having a crack at solving the issue...

I'd start by enabling logging in the editor, see https://templ.guide/commands-and-tools/ide-support#enable-lsp-logging - that way, you can see exactly what is being sent to the LSP. There's even a web server which shows you the mapping between the input templ file and the generated code etc.

However, the issue might also be in Neovim's LSP code.

@a-h a-h added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers lsp NeedsInvestigation Issue needs some investigation before being fixed labels Aug 24, 2024
@a-h a-h changed the title Go to Definition Error in Neovim with templ components bug: go to definition causes error in Neovim v0.10.x Aug 24, 2024
@axzilla
Copy link
Author

axzilla commented Aug 25, 2024

I'm still using Neovim v0.9.5. I'll have to try out Neovim v0.10.1 and see if I get the same issue.

I spotted some emojis in the text. Does the problem go away if there are no emojis? Emojis are multi-byte characters, so they cause a lot of "fun".

The reason I ask is that LSP positions were recently updated in the LSP spec: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocuments to now support UTF-8 encoding positions.

I did a bit of work on positions in May (#712) around multi-byte characters throwing off LSP positions, so there's some good tests in there around it.

It could be that the LSP code in Neovim is now trying to do some sort of negotiation as to what position to use, and is getting confused about text position, since it works fine in Neovim v0.9.5.

The fix might be to populate the correct parts of the LSP client / server neogitation to tell Neovim to use UTF-8 for positions (I think that's what VS Code does).

If you feel like having a crack at solving the issue...

I'd start by enabling logging in the editor, see https://templ.guide/commands-and-tools/ide-support#enable-lsp-logging - that way, you can see exactly what is being sent to the LSP. There's even a web server which shows you the mapping between the input templ file and the generated code etc.

However, the issue might also be in Neovim's LSP code.

First of all, thank you for your feedback. Unfortunately, downgrading to 0.9.5 or deleting the emojis has not helped so far.

@a-h
Copy link
Owner

a-h commented Aug 25, 2024

Well, it's good to rule things out. If you are able to create the smallest possible reproduction repo, that would be useful. Then I can try to load the repo on my setup, and see if I can reproduce it.

It could also be an issue in some other plugins that you're running. Here's my setup and scripts in case something jumps out at you:

https://github.com/a-h/dotfiles/blob/master/.config/nixpkgs/nvim.nix
https://github.com/a-h/dotfiles/tree/master/.config/nvim/lua

The issue with Neovim config is that people have loads of stuff running. So, starting from a minimal config and building up can also help with identifying where the issue is coming from.

@axzilla
Copy link
Author

axzilla commented Aug 25, 2024

So, this is what I have right now.

Unfortunately, still the same problem. Honestly, I don't know what else to try!

@tris203
Copy link
Contributor

tris203 commented Aug 28, 2024

I noticed this issue this morning, and have been doing some digging, I think there is an off-by-2 due to:
web server reports the go file to start:

// Code generated by templ - DO NOT EDIT.

package templ

//lint:file-ignore SA4006 This context is only used if a nested component is present.

import "github.com/a-h/templ"

in neovim i can see the go file starts

// Code generated by templ - DO NOT EDIT.

// templ: version: v0.2.747
package templ

//lint:file-ignore SA4006 This context is only used if a nested component is present.

import "github.com/a-h/templ"

we seem to be missing the blank line and the version line (lines 2 and 3) in the lsps understanding
Mapping through the web URI is perfect, but is missing those two lines

EDIT; that said, behaviour persists with include-version=false so, maybe I am wrong

@a-h
Copy link
Owner

a-h commented Aug 28, 2024

Thanks for investigating. If you enable the templ LSP web server, you can look at the mapping between templ files and the Go code that's being used by the LSP.

There's even a side-by-side view of templ code to Go code, with highlighting when you mouseover go code.

@tris203
Copy link
Contributor

tris203 commented Aug 28, 2024

Thanks @a-h

yes, the webview looks perfect, so either its a neovim bug, or the lsp is passing something that neovim isn't expecting
I am looking at the data passed to nvim, and the problem comes from the fact that the "line" that neovim is trying to pass is blank

This comes from this code here:
https://github.com/neovim/neovim/blob/3a61f05dd2d6cb2ac9bca4795467f459595e58dd/runtime/lua/vim/lsp/util.lua#L1797

here is the content of the rows table in the example i am testing with

{ {
    ["end"] = <1>{
      character = 10,
      line = 4
    },
    location = {
      range = {
        ["end"] = <table 1>,
        start = <2>{
          character = 6,
          line = 4
        }
      },
      uri = "file:///home/tris/code/fileclass/templ/Home.templ"
    },
    start = <table 2>
  }, {
    ["end"] = <3>{
      character = 40,
      line = 106
    },
    location = {
      range = {
        ["end"] = <table 3>,
        start = <4>{
          character = 36,
          line = 106
        }
      },
      uri = "file:///home/tris/code/fileclass/templ/Home.templ"
    },
    start = <table 4>
  } }

Line 106 is out of range in Home.templ, and isn't relevant in the generated go file either, so its something to do with the way that neovim is mapping the line/uri references I think. I will take another look after lunch

Additionally from the templ lsp log the mapping looks correct

{"level":"info","ts":"2024-08-28T10:32:38+01:00","caller":"proxy/server.go:71","msg":"updatePosition: found","uri":"file:///home/tris/code/fileclass/templ/Home.templ","fromTempl":"4:8","toGo":"11:7"}

so something is getting messed up in the translation into neovim

@tris203
Copy link
Contributor

tris203 commented Aug 28, 2024

I might be going crazy, but the issue seems to be when the component has the same name as the file, although its not immediately clear why that would matter

scratch that, the issue is if your templ definition is on line 5. which is common with a single import,
if you have a single import in your .templ file in import "package" format, then that's what breaks it for me.

minimal reproduction:

package templ

import "fmt"

templ Hello(name string) {
{ fmt.Sprintf("Hello %s", name) }
}

even wierder, this works

package templ

import "fmt"

//this works
templ Hello(name string) {
{ fmt.Sprintf("Hello %s", name) }
}

even more minimal reproduction

package templ

//comment1
//comment2
templ Hello(name string) {
<h1>Hello, { name }!</h1>
}

why does templ lsp hate line 5?

@a-h
Copy link
Owner

a-h commented Aug 28, 2024

Interesting. Maybe it's a result of the recent change to auto-fixup imports.

Maybe the line is getting removed.

@a-h
Copy link
Owner

a-h commented Sep 9, 2024

We have a good reproduction of a similar issue (maybe even the same?) in #911

@axzilla
Copy link
Author

axzilla commented Sep 13, 2024

The problem disappeared with the nightly version!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants