separate parsing from running steps#217
Merged
teto merged 2 commits intorest-nvim:mainfrom Jul 15, 2023
Merged
Conversation
teto
force-pushed
the
treesitter-engine
branch
2 times, most recently
from
e0edb29 to
884a89a
Compare
teto
force-pushed
the
treesitter-engine
branch
2 times, most recently
from
316b902 to
7de3dcf
Compare
teto
commented
Jul 13, 2023
| return { external = true, filename_tpl = importfile } | ||
| else | ||
| lines = vim.api.nvim_buf_get_lines(bufnr, start_line - 1, stop_line, false) | ||
| lines = vim.api.nvim_buf_get_lines(bufnr, start_line, stop_line, false) |
Collaborator
Author
There was a problem hiding this comment.
I am a bit afraid removing the -1 might create bugs down the line but it was incorrect, i.e, if you look at the main branch today the request in tests/basic_get.http
GET https://reqres.in/api/users?page=5
loads with GET https://reqres.in/api/users?page=5 as body xD
This means we could use golden (also called "snapshot") tests to make sure this is not the case. This PR helps making rest.nvim more "api-like" so I think it's a move in the good direction overall.
Right now we splice variables as we parse the request. This is annoying for several resons: variable needs to exist to create the request, so probably we need to run the previous requests before that.
teto
changed the title
Collaborator
Author
|
I noticed that we dont tests variable splicing in tests but I've done some manual testing and it looked fine. Planning to add them to tests in next PRs. I think I'll merge and if bugs are spotted, I will correct them ^^'' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
the treesitter backend is promising but it is too much work to reach feature parity with our current parser:
#174
what I am proposing here is a first refactoring to delay variable splicing until last minute.
It untangles parsing & running requests. Right now you need variables to exist when parsing which means you have to execute the request when parsing them else you get errors.
This PR replaces (aka "splices") the variables at the last minute so you can parse requests without executing them.
It allowed me to add a
buf_list_requestfunction in the current backend, which was not possible before, so nowrun_fileis backend agnostic (it first lists requests and then run them).In this PR, you can't change backend yet but in a future one, I will add a treesitter backend to parse request.
We will then be able to switch (optional) to a treesitter backend even though it's not complete (One could even imagine a hurl backend) which will make working on it easier.