[not urgent] enable treesitter parsing by default #225
Conversation
|
I want to know @Grueslayer 's take on that. I'm glad that we don't rely on ts at all for parsing, so we can implement features completely independent on our own, but maybe I'm just an old grumpy man 🙈 Also this might break (haven't checked) the ability to run requests in any file, as long as they are in fenced code blocks and well formatted. |
|
As long as @gorillamoe does not take care of a correct and up-to-date treesitter parser matching the Kulala syntax, I think parsing by TS should be opt-in only. Let's have a look at this running (stripped down but looks identically with all request variables used) example:
The URLs are not correctly parsed by treesitter: Kulala will (like vscode-rest-client and others) urlencode query parameters so we can use spaces, ampersands in it's native way. TS http parser at least has a problem with the single quotes here. Maybe in that case it's cosmetic only. |
|
To be honest, when Kulala started in v1, we had full reliance on TS and that's when so many people complained about stuff not working as expected, because TS was error'ing even for basic stuff. That days might be gone and stuff might be fixed, but the biggest downside I see is that when we want to add additional stuff that e.g. jetbrains supports, but isn't part of the http grammar, we can't just implement and ship it, because we have to wait for our PR to get approved. And worst thing would be that the PR gets rejected, which would be totally fair, because http grammar is part of the rest.nvim project, so they can decide what goes in. |
|
Then you might say, okay let's create our own grammar for that. That's something I already thought about, but I'm very contradicted (is this the right word here?) when it comes to this topic, because I want to progress the http grammar and Kulala in a way that it is feature complete with jetbrains (that would be my biggest dream), but I don't really want a competing grammar (even if I think competition is good and sparks a lot of improvment) for http. As you are both maintainers (@Grueslayer and @treywood) you might add your 2 cents on a competing grammar which would be part of the Kulala project. I think either this is the only way to achieve the goal of competing with jetbrains. @boltlessengineer and @NTBBloodbath both are the maintainers of the http grammar and if they decide that something does not belong there, it's fair. When I now think about it, maybe blood and engineer can add their 2 cents to the goal of getting http grammar on par with jetbrains? If this is something they already have planned, we can just work together on that goal and just commit to the http grammar repo. If not, also cool, but then we might have that ugly discussion about thinking about our own grammar. Also I did not look into how we get our own grammar "approved" so it can be installed via |
Hey, thanks for the heads-up here. I'm currently a bit out of rest.nvim and http parser development for personal reasons and botless is maintaining both for now, so take my opinion with a grain of salt. However, a month ago when boltless did the massive parser refactor (see #38), he changed everything to bring the parser into compliance with the Jetbrains http spec (+ some minor extra QoL changes for rest.nvim usage) so I guess if there's something missing that's compliant with Jetbrains' http grammar it could be added without any major issues :) |
|
my only real opinion (as a very new user and contributor to this project) - and the reason why I felt like utilizing treesitter here would be a worthwhile effort - is that rolling your own parsing logic with only the tools available in vimscript/lua seems unpleasant and tree-sitter makes adding/changing parser logic a lot more straightfoward, especially with a lot of the groundwork already done. in my experience contributing to the http parser, @boltlessengineer seems very amenable to aligning with the jetbrains spec and has been quick to get my changes merged so I don't really think there would be too much friction in the future if more changes were needed to add important features to this project. |
|
Okay, let's try this then, but first we need to make sure gql in http in treesitter. I use it on a daily basis and this needs to work for me reliable. When working on rest-nvim/rest.nvim#452, I noticed that at least one of my queries failed in rest.nvim. I haven't checked if it's due to treesitter or just related to rest.nvim. Queries are from work stuff, so I can't exactly share it as it is, but probably supply a repro example. I think it's due to variable replacement. I also noticed rest and kulala are quite different 🤔 which is a good thing on its own, but I struggled to get my login query to work. Maybe I did something wrong, but named requests with response.body.$.token wasn't working for me. Isn't this (yet) supported by rest @NTBBloodbath @boltlessengineer? I'm referring to this here: https://kulala.mwco.app/docs/usage/request-variables |
|
Maybe @Grueslayer can also did some quick tests on his stuff with treesitter enabled. |
Also, you won’t get answer by asking to ntb. He is completely out of rest.nvim development now. |
Ah, thanks for the clarification. I thought that it came from intellij, but maybe it's just a thing form the vscode-rest-client by @Huachao 🤔 |
I've been thinking about how to start enabling tree-sitter parsing by default, and I think this might be a good way to go about it - the
treesitteroption is set to true by default if the user has an http parser installed, and documentation encourages the user to try setting this option to false if they encounter an issue.No rush to merge this, but curious about thoughts on the approach