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

feat: templ component snippet #714

Merged
merged 6 commits into from
May 8, 2024
Merged

feat: templ component snippet #714

merged 6 commits into from
May 8, 2024

Conversation

valentimarco
Copy link
Contributor

Refer to this PR for more info.

P.S i don't understand how to test this piece of code... I was able to build the command but idk how to connect vscode (in my case) to the lsp server

@joerdav
Copy link
Collaborator

joerdav commented Apr 30, 2024

I don't think that package ${1:}; should be included. I think that the templ snippet should only insert one templ element.

And in terms of connecting to vscode, do you get autocomplete in templ files?

@valentimarco
Copy link
Contributor Author

I don't think that package ${1:}; should be included. I think that the templ snippet should only insert one templ element.

And in terms of connecting to vscode, do you get autocomplete in templ files?

Ok make sense to delete the declaration of the package! For vscode, i dont get the autocomplete bc idk how to connect the lsp server to vscode... I tried with the plugin, but the http param in the settings open the lsp server, instead i want to connect to an existing lsp server

@joerdav
Copy link
Collaborator

joerdav commented Apr 30, 2024

The extension should start the server for you, sometimes it helps to start vscode from the command line so that it has templ in the path code .

@joerdav
Copy link
Collaborator

joerdav commented Apr 30, 2024

I also realize I sent you to the wrong part of the code base, this is for html snippets only (sorry).

In this bit of code https://github.com/a-h/templ/blob/main/cmd/templ/lspcmd/proxy/server.go#L361 it returns completion items, essentially we want to append the completion item to the result of this function.

@valentimarco
Copy link
Contributor Author

valentimarco commented Apr 30, 2024

I also realize I sent you to the wrong part of the code base, this is for html snippets only (sorry).

In this bit of code https://github.com/a-h/templ/blob/main/cmd/templ/lspcmd/proxy/server.go#L361 it returns completion items, essentially we want to append the completion item to the result of this function.

Ok i figure the code and i added this line after line 411:

result.Items = append(result.Items, lsp.CompletionItem{
		Label: "templ",
		InsertText: `templ ${2:}() {
	${0}
}`,
		Kind:             lsp.CompletionItemKind(lsp.CompletionItemKindSnippet),
		InsertTextFormat: lsp.InsertTextFormatSnippet,
	})

And works!
But i have a bug where the templ is not highlight until i write the template name (vscode snippet don't create this problem)

@valentimarco
Copy link
Contributor Author

I put a default name to resolve the problem, do you like it?
Screenshot_20240430_190012

I also create a global list in the snippets.go file

@joerdav
Copy link
Collaborator

joerdav commented May 1, 2024

I'm happy with this, I think its a useful snippet to have. What to you think @a-h ?

@joerdav
Copy link
Collaborator

joerdav commented May 1, 2024

I've also tested locally and it works fine for me.

@joerdav
Copy link
Collaborator

joerdav commented May 1, 2024

One of the tests now fails, I think the expectation needs updating.

@valentimarco
Copy link
Contributor Author

Is something related to the PR? Sorry i have little exp in go and cannot understand the tests...

@joerdav
Copy link
Collaborator

joerdav commented May 3, 2024

Yeah, so there is a test that is expecting no snippets to be returned, but now we have a global snippet that is returned every time.

I'd recommend renaming the test since the name is no longer true. And updating the assertion to expect your new snippet.

You can run the tests with go test ./...

@joerdav
Copy link
Collaborator

joerdav commented May 3, 2024

Let me know if you need a hand, then I can fix it up.

@valentimarco
Copy link
Contributor Author

valentimarco commented May 7, 2024

I got so much confused when trying to create the test, so far:

  1. Deleted the test that create the problem.
  2. trying to copy the test logic without success.
tests_global_snippet := []struct {
		line        int
		replacement string
		cursor      string
		assert      func(t *testing.T, cl *protocol.CompletionList) (msg string, ok bool)
	}{
		{
			line:        18,
			replacement: ` `,
			cursor:      `^`,
			assert: func(t *testing.T, actual *protocol.CompletionList) (msg string, ok bool) {
				// what shoud i do?
				return "", true
			},
		},
		
	}

@joerdav joerdav changed the title Add: templ snippet feat: templ component snippet May 8, 2024
@joerdav
Copy link
Collaborator

joerdav commented May 8, 2024

Thanks for this!

@joerdav joerdav merged commit fd2c772 into a-h:main May 8, 2024
4 checks passed
if diff := lspdiff.CompletionList(nil, actual); diff != "" {
return fmt.Sprintf("unexpected completion: %v", diff), false
if !lspdiff.CompletionListContainsText(actual, "templ") {
return fmt.Sprintf(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would really prefer that this formatting wasn't applied.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll figure out a whitelist, I keep forgetting to toggle it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorted, it wasn't gofumpt after all, I had the golines formatter enabled too.

@valentimarco valentimarco deleted the snippets branch May 8, 2024 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants