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

add primitive document symbol support #848

Merged
merged 9 commits into from
Nov 5, 2024
Merged

Conversation

jackielii
Copy link
Contributor

@jackielii jackielii commented Jul 15, 2024

try to fix #347

Currently it deals with Go symbols + parser.HTMLTemplate

TODOs:

  1. deal with documentSymbol as well
  2. add other symbols (css, script)?
  3. add correct containerName?

@jackielii
Copy link
Contributor Author

jackielii commented Jul 15, 2024

OK, I admit there is a big problem with my approach: I didn't convert the templ URI to the Go URI, yet somehow it worked.

It seems gopls is good at parsing partial results and skip over invalid block, therefore all the non templ elements are all returned. I then parsed and added the templ elements, namely parser.HTMLTemplate, making the result correct....

@joerdav
Copy link
Collaborator

joerdav commented Sep 3, 2024

Is this ready to be reviewed @jackielii ?

@joerdav
Copy link
Collaborator

joerdav commented Sep 3, 2024

I tested this on https://github.com/joerdav/shopping-list/blob/main/app/listsweb/list.templ

And I got the following symbols:

app/listsweb/list.templ|58 col 7| [Function] ListItems(list List, availableItems []Item)
app/listsweb/list.templ|96 col 7| [Function] ListPage(urlpath string, list List, availableRecipes []Recipe, availableItems []Item)
app/listsweb/list.templ|134 col 7| [Function] ListsPage(path string, lists []ListSummary)
app/listsweb/list.templ|18 col 7| [Function] ListRecipes( list List, availableRecipes []Recipe, )

I expected to also see the go functions there recipeInList and itemInList

@joerdav
Copy link
Collaborator

joerdav commented Sep 3, 2024

I spotted the TODO, oops! Thanks for the PR by the way, it's looking to be almost there!

@jackielii
Copy link
Contributor Author

I've been using it, works most of the time. But because it uses gopls to parse the templ files directly. It has quite a few broken cases. I'll redo it using the proper way by converting to go files and find mapping symbols.

@a-h
Copy link
Owner

a-h commented Sep 3, 2024

Legend. 😀

@jackielii
Copy link
Contributor Author

jackielii commented Oct 21, 2024

@a-h @joerdav I have implemented the said approach: use go source to find the symbols and map them back to templ source ranges.

There are two issues I couldn't figure out:

  1. The test I added only returns "SymbolInformation" whereas the actual lsp returns "DocumentSymbols", and also looking at the range for "Page" symbol the range is incorrect. i.e.
						Range: protocol.Range{
							Start: protocol.Position{
								Line:      11,
								Character: 0,
							},
							End: protocol.Position{
								Line:      50,
								Character: 1,
							},
						},

doesn't map to the actual templ source. However, in the actual run, it returns the "DocumentSymbol" and range is correct.

  1. When I used it with nvim-lspconfig, there is always a "A" character inserted when I pull up document symbols. This could be my nvim-lspconfig, but I couldn't figure out why. This is because I have both HTML and templ lsp, disabling html fixes the problem

This is my relevant nvim config just in case it's useful to others:

  {
    "neovim/nvim-lspconfig",
    init = function()
      vim.filetype.add({ extension = { templ = "templ" } })
      vim.api.nvim_create_autocmd("FileType", {
        pattern = "templ",
        callback = function()
          vim.b.autoformat = true
          -- vim.o.commentstring = "// %s"
        end,
      })
    end,
    opts = {
      servers = {
        htmx = {
          filetypes = { "html", "templ" },
        },
        html = {
          filetypes = { "html", "htm", "templ" },
          -- filetypes = { "html", "htm" },
        },
        templ = {
          cmd = { "/Users/jackieli/go/bin/templ", "lsp", "-log", "/tmp/templ.log" },
          -- cmd = { "templ", "lsp" },
        },
        -- emmet_ls = {
        --   filetypes = { "html", "htm", "htmx", "templ" },
        -- },
      },
      setup = {
        html = function(_, opts)
          -- remove html documentSymbol and just use templ's document symbol
          LazyVim.lsp.on_attach(function(client, buffer)
            local ft = vim.api.nvim_get_option_value("filetype", { buf = buffer })
            if ft == "templ" then
              client.server_capabilities.documentSymbolProvider = false
            end
          end, "html")
        end,
      },
    },
  },

@jackielii
Copy link
Contributor Author

@joerdav @a-h the PR is ready as is. Please feel free to review and comment.

cmd/templ/lspcmd/proxy/server.go Outdated Show resolved Hide resolved
cmd/templ/lspcmd/proxy/server.go Show resolved Hide resolved
Copy link
Collaborator

@joerdav joerdav left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!! I've tested this out and it works exactly as I would expect it.

@joerdav joerdav merged commit 4f2ce16 into a-h:main Nov 5, 2024
4 checks passed
@a-h
Copy link
Owner

a-h commented Nov 8, 2024

Thanks a lot for seeing this through to completion. Much appreciated.

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.

lsp: support textDocument/documentSymbol
3 participants