pfeiferj / tree-sitter-hurl

tree-sitter parser for the hurl file format
Apache License 2.0
20 stars 1 forks source link

Support fenced blocks (e.g. graphql) #10

Closed lervag closed 3 months ago

lervag commented 7 months ago

The current version does not support fenced blocks that are e.g. used for GraphQL queries (https://hurl.dev/blog/2023/01/25/hurl-2.0.0-the-graphql-edition.html#graphql-query-support), e.g.:

POST https://api.starwars.com/graphql
```graphql
query HeroNameAndFriends($episode: Episode) {
  hero(episode: $episode) {
    name
    friends {
      name
    }
  }
}

variables {
  "episode": "JEDI"
}

HTTP 200



Notice that this syntax is also an optional way to include json bodies, see https://hurl.dev/docs/request.html#body.

I would be glad to help here, but I don't really know how to write tree-sitter parsers yet. If you give me some hints I may be able to open a pull request.
pfeiferj commented 7 months ago

Should be working with latest version v1.4.1. Here's a screenshot with both the hurl and graphql tree-sitter grammars installed in neovim. Do you have a screenshot of it not working?

Screenshot 2024-03-29 at 8 40 33 PM
lervag commented 7 months ago

This is how it looks for me:

image

I've installer tree-sitter-hurl through nvim-treesitter. To be honest, I don't know how to check which version I'm using. I've made sure to update nvim-treesitter and to run :TSUpdate.

pfeiferj commented 6 months ago

This is how it looks for me:

image

I've installer tree-sitter-hurl through nvim-treesitter. To be honest, I don't know how to check which version I'm using. I've made sure to update nvim-treesitter and to run :TSUpdate.

Make sure you also install the graphql grammar. The hurl grammar uses language injection to use the graphql grammar inside of the fenced section. So basically if you have the graphql grammar installed your editor knows that section is graphql and loads up the graphql grammar for only that section of the file

lervag commented 6 months ago

I have installed all grammars, including the graphql one. Here's how the included graphql code looks like if I open it as a .graphql file:

image

pfeiferj commented 6 months ago

@lervag ok, so i was able to reproduce on latest neovim master. The issue doesn't appear to happen in latest stable neovim 0.9.5 though. I'll have to look into what's changed with treesitter in the neovim nightlies

lervag commented 6 months ago

Great, thanks. Sorry that I never mentioned I'm on the nightly builds. I didn't expect this to be caused by a change in neovim here. In any case, I'm very glad to hear you've reproduced this and that you are looking into it!

pfeiferj commented 6 months ago

well, this is quite interesting... It seems the neovim commit that causes the issue is https://github.com/neovim/neovim/commit/3d948a4dc4b2cd3c8d3ac497caf3dfe25adfb90d

The interesting thing about that is the only thing there that sounds like it might cause issues is that it adds in hurl filetype detection. Which makes me think that there's some kind of behavioral difference when the filetype detection is built into vim vs when it's not

pfeiferj commented 6 months ago

yep, it appears that is the case. If i go to that commit, remove hurl from runtime/lua/vim/filetype.lua, build neovim, and then manually call :set ft=hurl the language injection works

lervag commented 6 months ago

Yes, that's very strange indeed. To make it easier to debug this, I've made the a minimal config (based on lazy.nvim; attached below). To reproduce the problem:

  1. Create some test directory and save the contents of the below listing to TEST/init.lua.
  2. From within the directory, run nvim --clean -u init.lua.
  3. :e test.hurl
  4. :InspectTree

Now we see something like this:

image

-- init.lua
local root = vim.fn.fnamemodify("./.lazy", ":p")

for _, name in ipairs({ "config", "data", "state", "cache" }) do
  vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end

local lazypath = root .. "/plugins/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
  vim.fn.system({
    "git",
    "clone",
    "--filter=blob:none",
    "https://github.com/folke/lazy.nvim.git",
    lazypath,
  })
end
vim.opt.runtimepath:prepend(lazypath)

require("lazy").setup({
  {
    "nvim-treesitter/nvim-treesitter",
    build = ":TSUpdate",
    config = function()
      require("nvim-treesitter.configs").setup({
        ensure_installed = { "hurl", "graphql", "query" },
        highlight = { enable = true },
      })
    end,
  },
}, {
  root = root .. "/plugins",
})
lervag commented 6 months ago

I also tested to add vim.g.did_load_filetypes = 1 to the init.lua, which prevents neovim from applying any filetypes, and then using set ft=hurl. But the problem persists.

lervag commented 6 months ago

I tried compiling neovim and going back to the commit you mentioned, but I'm still seeing errors. Not as many as before, though. I did this:

  1. Put the test files mentioned before under ~/workdir/test-hurl
  2. Clone neovim under ~/workdir/neovim
  3. Compiled neovim with the specified commit (3e3eddc, i.e. the parent to the commit you mentioned)
  4. Started neovim with VIMRUNTIME=../neovim/runtime ../neovim/build/bin/nvim --clean -u init.lua

As you see below, there are still "ERROR"s.

image

lervag commented 6 months ago

I get the same errors if I checkout the commit that introduced hurl into filetype.lua, so I don't think this is the one that introduces the errors.

lervag commented 6 months ago

And if I build neovim v0.9.5 I still get the same :InspectTree output, i.e. with the ERROR statements from line 4 and on.

pfeiferj commented 6 months ago

@lervag make sure to clean your builds and double check that you're running the correct neovim. I noticed some weird behavior if i just used make clean, so delete the entire build folder

pfeiferj commented 6 months ago

Some more interesting stuff... It looks like if i build master then i see the exact same thing you see above. However, if i remove hurl from filetype.lua and rebuild and set ft=hurl then I get no error but the injection doesn't work at all, it sees the section as a multiline text block. I did see in the commit history there was some changes to the treesitter queries for injections, so that might explain why without the file detection it doesn't inject at all, but it's very strange that the behavior is different with hurl listed in filetype.lua

lervag commented 6 months ago

Yes, I've also noticed that there have been updates to the injection stuff. But I've not built any real experience with implementing any treesitter grammars and I don't fully understand how everything works.

Looking at the nvim-treesitter repo, it seems like:

  1. nvim-treesitter will install hurl from this repo due to this information.
  2. The revision is stored in the lockfile. Indeed, the revision there corresponds to the tag v1.4.1.
  3. From the install info, it seems nvim-treesitter will generate it's information directly from src/parser.c. So, does this mean that the only relevant file in this repo, from nvim-treesitter point of view, is src/parser.c?
lervag commented 6 months ago

@lervag make sure to clean your builds and double check that you're running the correct neovim. I noticed some weird behavior if i just used make clean, so delete the entire build folder

I did this:

cd ~/workdir/neovim
git checkout 3d948a4~
make clean
rm -rf build
make

cd ~/workdir/test-hurl
VIMRUNTIME=../neovim/runtime ../neovim/build/bin/nvim --clean -u init.lua

Then :edit test.hurl and set ft=hurl. init.lua and test.hurl are the same as provided earlier. I still get the same result where there are errors in the :InspectTree window.

clason commented 3 months ago

Tip: you can use I on the tree inspector to toggle language injection display:

(hurl_file ; [0, 0] - [16, 0] hurl
  (entry ; [0, 0] - [16, 0] hurl
    (request ; [0, 0] - [15, 0] hurl
      (method) ; [0, 0] - [0, 4] hurl
      (value_string ; [0, 4] - [0, 37] hurl
        (value_string_content ; [0, 4] - [0, 37] hurl
          (value_string_text))) ; [0, 4] - [0, 37] hurl
      (body ; [1, 0] - [15, 0] hurl
        (bytes ; [1, 0] - [14, 3] hurl
          (multiline_string ; [1, 0] - [14, 3] hurl
            (multiline_string_type) ; [1, 3] - [1, 10] hurl
            (multiline_string_content ; [2, 0] - [2, 45] hurl
              (ERROR ; [2, 0] - [2, 45] graphql
                (operation_type) ; [2, 0] - [2, 5] graphql
                (name) ; [2, 6] - [2, 24] graphql
                (variable_definitions ; [2, 24] - [2, 43] graphql
                  (variable_definition ; [2, 25] - [2, 42] graphql
                    (variable ; [2, 25] - [2, 33] graphql
                      (name)) ; [2, 26] - [2, 33] graphql
                    (type ; [2, 35] - [2, 42] graphql
                      (named_type ; [2, 35] - [2, 42] graphql
                        (name))))))) ; [2, 35] - [2, 42] graphql
            (multiline_string_content ; [2, 45] - [3, 27] hurl
              (ERROR ; [3, 2] - [3, 27] graphql
                (ERROR ; [3, 2] - [3, 25] graphql
                  (ERROR) ; [3, 2] - [3, 6] graphql
                  (ERROR) ; [3, 7] - [3, 14] graphql
                  (ERROR)))) ; [3, 17] - [3, 24] graphql
            (multiline_string_content ; [3, 27] - [4, 8] hurl
              (ERROR ; [4, 4] - [4, 8] graphql
                (ERROR))) ; [4, 4] - [4, 8] graphql
            (multiline_string_content ; [4, 8] - [5, 13] hurl
              (ERROR ; [5, 4] - [5, 13] graphql
                (ERROR ; [5, 4] - [5, 11] graphql
                  (ERROR)))) ; [5, 4] - [5, 11] graphql
            (multiline_string_content ; [5, 13] - [6, 10] hurl
              (ERROR ; [6, 6] - [6, 10] graphql
                (ERROR))) ; [6, 6] - [6, 10] graphql
            (multiline_string_content ; [6, 10] - [7, 5] hurl
              (ERROR)) ; [7, 4] - [7, 5] graphql
            (multiline_string_content ; [7, 5] - [8, 3] hurl
              (ERROR)) ; [8, 2] - [8, 3] graphql
            (multiline_string_content) ; [8, 3] - [9, 0] hurl
            (multiline_string_content ; [9, 0] - [9, 1] hurl
              (ERROR)) ; [9, 0] - [9, 1] graphql
            (multiline_string_content) ; [9, 1] - [11, 0] hurl
            (multiline_string_content ; [11, 0] - [11, 11] hurl
              (ERROR ; [11, 0] - [11, 11] graphql
                (ERROR ; [11, 0] - [11, 9] graphql
                  (ERROR)))) ; [11, 0] - [11, 9] graphql
            (multiline_string_content ; [11, 11] - [12, 19] hurl
              (ERROR ; [12, 2] - [12, 19] graphql
                (ERROR ; [12, 2] - [12, 12] graphql
                  (description ; [12, 2] - [12, 11] graphql
                    (string_value))) ; [12, 2] - [12, 11] graphql
                (description ; [12, 13] - [12, 19] graphql
                  (string_value)))) ; [12, 13] - [12, 19] graphql
            (multiline_string_content) ; [12, 19] - [13, 0] hurl
            (multiline_string_content ; [13, 0] - [13, 1] hurl
              (ERROR)))))) ; [13, 0] - [13, 1] graphql
    (response ; [15, 0] - [16, 0] hurl
      (version) ; [15, 0] - [15, 4] hurl
      (status)))) ; [15, 5] - [15, 8] hurl

This means injections are working (on latest commit of Neovim and nvim-treesitter), but the graphql parser fails to parse the embedded code. Probably the injection query needs to set injection.combined. @lervag mind to open an issue (or, even better, PR) on nvim-treesitter?

lervag commented 3 months ago

This means injections are working (on latest commit of Neovim and nvim-treesitter), but the graphql parser fails to parse the embedded code. Probably the injection query needs to set injection.combined. @lervag mind to open an issue (or, even better, PR) on nvim-treesitter?

I was not aware of the I key, thanks for the tip. I wouldn't mind making a PR, but I don't quite know how to solve this. I assume you are talking about something that needs to be changed here? Do you have some reference or similar that might help me to understand what I need to do to make this work?

clason commented 3 months ago

Ah, I see, this is one of those weird info-string injections. (Things are simpler to deal with on the main branch.) I'd just open an issue about it, then.

lervag commented 3 months ago

But does this mean that you believe the problem is related to nvim-treesitter and not the tree-sitter-hurl implemnetation itself? Is there a way to test that in a more direct manner?

clason commented 3 months ago

I am firmly convinced that the issue is solely on the nvim-treesitter (and possibly on the tree-sitter-graphql) side. If the contents are not parsed by the hurl parser (as the tree view shows), it cannot be due to this parser.

lervag commented 3 months ago

Cool, thanks. I've opened an issue on nvim-treesitter, so I'll close this one as a consequence.