Skip to content

fix parsing of nested tables and curl arguments #138

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

Merged
merged 4 commits into from
Nov 25, 2022
Merged

fix parsing of nested tables and curl arguments #138

merged 4 commits into from
Nov 25, 2022

Conversation

udayvir-singh
Copy link
Contributor

@udayvir-singh udayvir-singh commented Aug 28, 2022

Fixes errors when you pass array or object in request body without setting request body to application/json, for example:

POST http://localhost:8000/login

{
  "username": "foo",
  "args": {
    "key": "val"
  }
}

would result in following error:

[rest.nvim] Failed to perform the request.
Make sure that you have entered the proper URL and the server is running.
Traceback: ...site/pack/packer/start/plenary.nvim/lua/plenary/curl.lua:54: attempt to concatenate a table value

but this PR fixes it and gives the following query:

curl -sSL --compressed -X 'POST' -d 'args={"key": "val"}' -d 'username=foo' 'http://localhost:8000/login'

@NTBBloodbath
Copy link
Member

Hey @teto, hope you're doing well (and sorry for the mention).

Do you think it's worth it merging this one? I was checking the PRs that are open and found #120 (that is yours) and it does fix body send by sending the whole body as a string.

Would your PR fix this problem and many others too? If so, we could update your PR branch and merge that one instead and close this one in favor of #120.

@udayvir-singh
Copy link
Contributor Author

@NTBBloodbath the issue that #120 was trying to solve has already been fixed by my previous PR #125.

This PR fixes errors when you try to include json values in application/x-www-form-urlencoded content type which is default on curl.

@teto
Copy link
Collaborator

teto commented Aug 29, 2022

I was wondering if my PR was still relevant, thanks for addressing this. Could you add a test that fails without this please ?

@udayvir-singh
Copy link
Contributor Author

@teto

I have added a test post_json_form, it should return a error if you try to run it from the master branch.

@udayvir-singh udayvir-singh changed the title fix parsing of nested tables in request body fix parsing of nested tables and curl arguments Sep 13, 2022
@udayvir-singh
Copy link
Contributor Author

@NTBBloodbath any updates on this PR?

I have also added another commit that fixes parsing of curl arguments with spaces in them. For example:

GET http://localhost:8000/

-c cookies

the above request will create file _cookies instead of just cookies. This commit fixes that by adding a parser for space separated arguments.

This also allows for more complex queries like:

GET http://localhost:8000/

-c some\ cokkie\ file -u user:password

@NTBBloodbath NTBBloodbath merged commit 3811092 into rest-nvim:main Nov 25, 2022
@NTBBloodbath
Copy link
Member

Hey, sorry for the late reply!

Thought actual codebase rewrite would take less time (it's still in its time to wear diapers, but I'm making very good decisions behind the scenes!) so I didn't want to merge any actual PR in order to keep codebase simpler to track for me, however, that's not the case right now and we gotta merge these fixes and some new features as people really needs them.

Also thank you so much for this PR changes, everything looks good to me. Thank you again for fixing cURL arguments parsing too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants