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

parse requests with treesitter #181

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

treywood
Copy link
Contributor

Howdy! I hope this sort of PR is welcome.

Thanks for this plugin, i've been using rest.nvim for a while and it's been starting to show it's age. I have a bunch of http snippets that i use frequently and they worked with rest.nvim but were kinda tricky to use with kulala. I eventually figured out that it was because rest.nvim doesn't conform as well to the http file spec, and when i dug in deeper (into the spec and this plugin) I saw an opportunity to clean up some of the manual string parsing you're using with (hopefully) simpler parsing with treesitter.

I don't know if you were hoping to avoid having treesitter as a required dependency so I put this refactor behind an undocumented configuration option to maybe let it bake if you're interested but unsure.

@treywood treywood force-pushed the treesitter-parsing branch 2 times, most recently from e80a7a0 to ae0bbd3 Compare August 22, 2024 16:54
@gorillamoe gorillamoe self-assigned this Aug 22, 2024
@gorillamoe gorillamoe added the enhancement New feature or request label Aug 22, 2024
@gorillamoe
Copy link
Member

Hey thanks for the awesome contribution 🥰. When I started Kulala, I made heavy use of the http TS grammar, but I struggled with TS failing with stuff that is part of the http spec all the time and user complaints started bulking up.

That's when I decided to rewrite everything from scratch and don't rely on the http TS grammar at all.

Making it opt-in is a good middle ground.

I haven't checked the latest improvements @boltlessengineer made on the http grammar, so maybe all these issues are now resolved?! 🤔

@treywood
Copy link
Contributor Author

treywood commented Aug 22, 2024

I haven't checked the latest improvements @boltlessengineer made on the http grammar, so maybe all these issues are now resolved?!

I noticed the http parser was updated pretty significantly recently, to the point that all the features I could find in your documentation were simple to support. It looks like you've started adding support for redirecting response bodies to files (looks like with >> and >>! operators) and I don't think that's yet part of the TS grammar, but maybe easy enough to add? I have experience writing TS grammars so I could submit that change once i orient myself with that part of the spec

@gorillamoe
Copy link
Member

It looks like you've started adding support for redirecting response bodies to files (looks like with >> and >>! operators) and I don't think that's yet part of the TS grammar, but maybe easy enough to add? I have experience writing TS grammars so I could submit that change once i orient myself with that part of the spec

I don't think it's part of the spec, but jetbrains intellij supports this and some other things that Kulala should also support.

Let me quickly glance over the PR and if everything works, I don't mind merging this, because it's behind a feature flag (for now).

@gorillamoe
Copy link
Member

I have just tested a bit, and without treesitter, this works like a charm:

# @name REQUEST_BASIC_AUTH_TEST
GET https://httpbingo.org/basic-auth/gorilla/bananas HTTP/1.1
accept: application/json
authorization: Basic gorilla:bananas

###

# @name REQUEST_ONE
POST https://httpbin.org/post HTTP/1.1
Accept: application/json
Content-Type: application/json
Authorization: Bearer Foo:bar

{
  "foo": "bar"
}

> ./../scripts/script.js

###

# @name REQUEST_TWO
POST https://httpbin.org/post HTTP/1.1
accept: application/json
content-type: application/json

{
  "token": "{{REQUEST_ONE.response.body.$.json.foo}}",
  "gorilla": "{{RANDOM_DATE}}"
}

###

POST https://httpbin.org/post HTTP/1.1
content-type: application/json
# @env-json-key AUTH_TOKEN json.token

{
  "username": "{{REQUEST_TWO.response.headers['Content-Type']}}",
  "password": "{{PASSWORD}}",
  "token": "foobar"
}

###

GET https://httpbin.org/get HTTP/1.1
accept: application/json
authorization: Bearer {{AUTH_TOKEN}}

###

POST https://httpbin.org/post HTTP/1.1
content-type: application/json
accept: application/json
# @env-json-key AUTH_TOKEN json.token
# @env-json-key AUTH_USERNAME json.username

{
  "username": "{{USERNAME}}",
  "password": "{{PASSWORD}}",
  "token": "foobar"
}

###

POST https://httpbin.org/post HTTP/1.1
content-type: application/json
accept: application/json
authorization: Bearer {{AUTH_TOKEN}}

{
  "success": true,
  "username": "{{AUTH_USERNAME}}"
}

###

POST https://swapi-graphql.netlify.app/.netlify/functions/index HTTP/1.1
accept: application/json
X-REQUEST-TYPE: GraphQL

query Person($id: ID) {
  person(personID: $id) {name}
}

{ "id": 1 }

###

POST https://swapi-graphql.netlify.app/.netlify/functions/index HTTP/1.1
X-REQUEST-TYPE: GraphQL

query Query {
  allFilms {
    films {
      title
      director
      releaseDate
      speciesConnection {
        species {
          name
          classification
          homeworld {
            name
          }
        }
      }
    }
  }
}

With treesitter enabled, it's like every query is failing, I'm on the latest http grammar.

This is my script:

const randomDate = require('moment')().format('YYYY-MM-DD HH:mm:ss');
client.global.set('RANDOM_DATE', randomDate);

And this is my http-client.env.json:

{
  "dev": {
    "USERNAME": "gorillamoe",
    "PASSWORD": "bananas",
    "pokemon": "ditto",
    "auth_header": "Authorization: Basic Base64encodedstring=="
  },
  "prod": {
    "USERNAME": "marco",
    "PASSWORD": "polo",
    "pokemon": "pikachu",
    "auth_header": "Authorization: Basic Base64encodedstring=="
  }
}

@gorillamoe
Copy link
Member

Never ever trust what nvim tells you.. TSUpdate http -> http grammar is up to date.. it lied to me.. I reinstalled and now it seems to work 👍

@gorillamoe
Copy link
Member

I'll merge this, but run_all() is broken with treesitter, but as it is behind a feature flag, no harm is done :)

@gorillamoe gorillamoe merged commit 14babc6 into mistweaverco:main Aug 22, 2024
@gorillamoe
Copy link
Member

oh noez, I fucked up, it's still broken, was on the wrong pr, when I tried again with updated grammar.. good thing is this is behind a flag..

gorillamoe added a commit that referenced this pull request Aug 22, 2024
gorillamoe added a commit that referenced this pull request Aug 22, 2024
@gorillamoe
Copy link
Member

I would like to have this still behind a feature flag, once it passes the attached test-files, maybe you can work on that and open another PR @treywood?

@treywood
Copy link
Contributor Author

treywood commented Aug 22, 2024

I would like to have this still behind a feature flag, once it passes the attached test-files, maybe you can work on that and open another PR @treywood?

sounds good (i totally missed run_all!), i'll work on that. i noticed something this in your examples above:

the treesitter grammar doesn't parse comments between headers and the body, so this doesn't parse

POST https://httpbin.org/post HTTP/1.1
content-type: application/json
accept: application/json
# @env-json-key AUTH_TOKEN json.token
# @env-json-key AUTH_USERNAME json.username

{
  "username": "{{USERNAME}}",
  "password": "{{PASSWORD}}",
  "token": "foobar"
}

unless its rewritten like:

# @env-json-key AUTH_TOKEN json.token
# @env-json-key AUTH_USERNAME json.username
POST https://httpbin.org/post HTTP/1.1
content-type: application/json
accept: application/json

{
  "username": "{{USERNAME}}",
  "password": "{{PASSWORD}}",
  "token": "foobar"
}

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

Successfully merging this pull request may close these issues.

2 participants