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

Use treesitter highlights on successful invocations of Hurl #7

Open
PriceHiller opened this issue Jul 4, 2023 · 14 comments
Open

Use treesitter highlights on successful invocations of Hurl #7

PriceHiller opened this issue Jul 4, 2023 · 14 comments
Labels
enhancement New feature or request

Comments

@PriceHiller
Copy link
Contributor

This issue is mostly to pitch an idea.

When Hurl successfully executes it returns the content from whatever API/webpage it was called against it returns the raw content. We can extract the Content-Type from Hurl by running it with the -i flag.

With the Content-Type in hand it should be possible to set the buffer's filetype to the output Content-Type. As a note Hurl returns the last http request's content and headers only, so if you had multiple GET or POST requests in a single file to different URLs, only the last request's content is returned.

If Hurl runs into an error then we can return the content in stderr as usual without any filetype settings.

@pfeiferj
Copy link
Owner

pfeiferj commented Jul 4, 2023

Very interesting idea. Currently I set the buffer to be a terminal to gain highlighting of the hurl verbose output. I suspect that we can't really combine the two, so it might be that we need to switch to not using a terminal for the output to implement this. Another thought is that we could try to get treesitter highlighting merged into upstream hurl, but even if we did that it would probably still be valuable to be able to do it in neovim for the sake of using your nvim color settings

PriceHiller added a commit to PriceHiller/nvim-hurl that referenced this issue Jul 4, 2023
A severe limitation is the headers at the top of these files, as I don't
know how to write treesitter injections at this point in time to give
the headers highlights separate from the body. Another issue is that
this forces the users to opt-in to seeing headers which may not be
desired. But for a PoC? Good enough.

Related to pfeiferj#7
@PriceHiller
Copy link
Contributor Author

@pfeiferj

I have accomplished this feat. Is it any good? Probably not considering I've been up for close to 40 hours.

BUT, it does, in fact, work. Take a look here: https://github.com/treatybreaker/nvim-hurl/tree/set-filetype

I, unfortunately, don't know how to write Treesitter queries of any note so the headers are looking a bit drab. Ideally we'd do injections for the headers to highlight them separately from the body.

For now though, this serves as a solid enough PoC.

Screenshot of that branch in action:

image

PriceHiller added a commit to PriceHiller/nvim-hurl that referenced this issue Jul 4, 2023
A severe limitation is the headers at the top of these files, as I don't
know how to write treesitter injections at this point in time to give
the headers highlights separate from the body. Another issue is that
this forces the users to opt-in to seeing headers which may not be
desired. But for a PoC? Good enough.

Related to pfeiferj#7
@pfeiferj
Copy link
Owner

pfeiferj commented Jul 4, 2023

@pfeiferj

I have accomplished this feat. Is it any good? Probably not considering I've been up for close to 40 hours.

Hmm, looks like a good start. Have some thoughts though:
1.) Get some sleep man.
2.) It likely would be better to just pass no-color to hurl rather than try to remove the term codes.
3.) Looking at the output just now I realized that it looks like the hurl grammar might actually be able to parse the output and provide highlighting, in which case we actually might get the injections for free since the grammar's queries support some injections already. That would also allow moving away from using a term window for the output altogether because there wouldn't really be a reason to highlight based off of term codes

@pfeiferj
Copy link
Owner

pfeiferj commented Jul 4, 2023

3.) Looking at the output just now I realized that it looks like the hurl grammar might actually be able to parse the output and provide highlighting, in which case we actually might get the injections for free since the grammar's queries support some injections already. That would also allow moving away from using a term window for the output altogether because there wouldn't really be a reason to highlight based off of term codes

Nevermind, it would require a whole new grammar to support this

@PriceHiller
Copy link
Contributor Author

PriceHiller commented Jul 4, 2023

1.) Infinite Energy.
2.) If you pass no-color to hurl we lose color for anything that crops up on stderr unfortunately as I can't set a filetype to color the hurl errors. That's why I do the whole term code removal dance.

If you know of a better way then do say so, cutting out term codes is kinda hacky

@pfeiferj
Copy link
Owner

pfeiferj commented Jul 4, 2023

If you know of a better way then do say so, cutting out term codes is kinda hacky

So another idea is to actually separate stdout and stderr into separate windows. I believe the verbose hurl output actually outputs on stderr and then the final request outputs its data to stdout. Then stderr we could set term on to get its highlighting and then we could set a filetype for the stdout

@PriceHiller
Copy link
Contributor Author

Then stderr we could set term on to get its highlighting and then we could set a filetype for the stdout

But the issue is that if you call hurl with --no-color it never inserts those term codes for colors at all. We'd have to issue two calls to hurl, one to get no color stdout so we don't have to parse term codes and another for colored output to handle the stderr case.

@pfeiferj
Copy link
Owner

pfeiferj commented Jul 4, 2023

Then stderr we could set term on to get its highlighting and then we could set a filetype for the stdout

But the issue is that if you call hurl with --no-color it never inserts those term codes for colors at all. We'd have to issue two calls to hurl, one to get no color stdout so we don't have to parse term codes and another for colored output to handle the stderr case.

Not exactly, we just wouldn't use --no-color. The stderr is the only part that will have the term codes, so if we send that to its own window and set that window to term it will color it and hide the term codes correctly, then stdout we just send to a normal window that we set the filetype in. But, this is all assuming i'm right about the verbose header information being sent to stderr and not stdout

@PriceHiller
Copy link
Contributor Author

Oh ok, I see what you're saying. AFAIK the Header info is populated into stdout by the --include flag with a single newline separating the Header information from the body/content. Where I'm getting this from is from the stdout section over here. When I was messing about with it earlier the only way I could get Header info was with --include which populated stdin with the Headers.

@pfeiferj
Copy link
Owner

pfeiferj commented Jul 4, 2023

Oh ok, I see what you're saying. AFAIK the Header info is populated into stdout by the --include flag with a single newline separating the Header information from the body/content. Where I'm getting this from is from the stdout section over here. When I was messing about with it earlier the only way I could get Header info was with --include which populated stdin with the Headers.

ah, gotcha, so if we were to take the two window approach we would drop the --include so that we would have a separation of stderr=hurl colored term output, stdout=the last request's response data

@pfeiferj
Copy link
Owner

pfeiferj commented Jul 4, 2023

Oh ok, I see what you're saying. AFAIK the Header info is populated into stdout by the --include flag with a single newline separating the Header information from the body/content. Where I'm getting this from is from the stdout section over here. When I was messing about with it earlier the only way I could get Header info was with --include which populated stdin with the Headers.

ah, gotcha, so if we were to take the two window approach we would drop the --include so that we would have a separation of stderr=hurl colored term output, stdout=the last request's response data

wait, just realized what you meant, I've only ever really used the verbose option to get header info, so i thought --include must have just been verbose output but combined into stdout. Let me try out some stuff real quick with --include

@pfeiferj
Copy link
Owner

pfeiferj commented Jul 4, 2023

Oh ok, I see what you're saying. AFAIK the Header info is populated into stdout by the --include flag with a single newline separating the Header information from the body/content. Where I'm getting this from is from the stdout section over here. When I was messing about with it earlier the only way I could get Header info was with --include which populated stdin with the Headers.

ah, gotcha, so if we were to take the two window approach we would drop the --include so that we would have a separation of stderr=hurl colored term output, stdout=the last request's response data

wait, just realized what you meant, I've only ever really used the verbose option to get header info, so i thought --include must have just been verbose output but combined into stdout. Let me try out some stuff real quick with --include

ok, new new idea with a few parts:
1.) we should allow for options to be passed to hurl from the vim command
2.) we should detect --include option and if used we split the stdout into two windows based off the first newline on a blank line
3.) if we get stderr we place it into its own window
4.) if we got both stderr and stdout we open two windows

@pfeiferj pfeiferj added the enhancement New feature or request label Jul 4, 2023
@PriceHiller
Copy link
Contributor Author

I can take a look at doing all of this tomorrow (if I get a minute), if not it'll have to be this coming weekend or Friday.

The only thing that might be sketchy is the window split, as currently we're opening a floating window by default. Does this mean we're moving to a vertical split with two windows stacked on top of each other? Basically, that can't be fully configurable, might have to give up on some things on the user config side.

@pfeiferj
Copy link
Owner

pfeiferj commented Jul 4, 2023

The only thing that might be sketchy is the window split, as currently we're opening a floating window by default. Does this mean we're moving to a vertical split with two windows stacked on top of each other? Basically, that can't be fully configurable, might have to give up on some things on the user config side.

Probably fine to default to splits, I can look into a floating window option for it. Other things like telescope use multiple floating windows to separate information so it should definitely be possible

PriceHiller added a commit to PriceHiller/nvim-hurl that referenced this issue Jul 5, 2023
With `vim.system` we get better type coersion and general support from
libuv. Jobstart is primarily meant to be used from vimscript, not lua.
Per `:h jobstart` it recommends using `vim.system`.

This also makes it easier to handle building the commands list for hurl
as now we only have to manage a table.

This is the first of a few commits related to pfeiferj#9 and pfeiferj#7.
PriceHiller added a commit to PriceHiller/nvim-hurl that referenced this issue Jul 5, 2023
With `vim.system` we get better type coersion and general support from
libuv. Jobstart is primarily meant to be used from vimscript, not lua.
Per `:h jobstart` it recommends using `vim.system`.

This also makes it easier to handle building the commands list for hurl
as now we only have to manage a table.

This is the first of a few commits related to pfeiferj#9 and pfeiferj#7.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants