Skip to content

chore: dont special case json #169

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

Closed
wants to merge 1 commit into from
Closed

Conversation

teto
Copy link
Collaborator

@teto teto commented Dec 29, 2022

we special case json when checking the payload body via
encoding/decoding it.
This is done by checking the "content-type" in:
#157

There doesn't seem to be a need for that other than possibly validate
the payload ? in which case we should implement something more generic.

@teto
Copy link
Collaborator Author

teto commented Dec 29, 2022

@udayvir-singh what do you think ?

we special case json when checking the payload body via
encoding/decoding it.
This is done by checking the "content-type" in:
rest-nvim#157

There doesn't seem to be a need for that other than possibly validate
the payload ? in which case we should implement something more generic.
@udayvir-singh
Copy link
Contributor

udayvir-singh commented Dec 30, 2022

@teto fixed it with #170

@@ -67,23 +66,6 @@ local function get_body(bufnr, start_line, stop_line, has_json)
end
end

local is_json, json_body = pcall(vim.fn.json_decode, body)

if is_json then
Copy link
Contributor

@udayvir-singh udayvir-singh Dec 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@teto Don't delete these lines; this statement does two things:

  1. LINE 75: Minify json. ie. remove extra tabs and spaces to reduce bandwidth.
  2. LINE 78-83: Parse nested json into valid application/x-www-form-urlencoded string values. see fix parsing of nested tables and curl arguments #138

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 1 is non-relevant, also it means we dont send the exact same payload as specified by the user which is concerning (encoding/decoding floats can raise issues I suppose, same for numbers ?).

As for 2, application/x-www-form-urlencoded seems like a pain https://stackoverflow.com/questions/9870523/what-are-the-differences-between-application-json-and-application-x-www-form-url but indeed that's the curl default. One solution could be to default to application/json but that can be surprising too.

Maybe we could do is implement the same mechanism as for "formatter", but for inputs instead of outputs. have the ability to to preprocess the payload depending on content-type.

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.

2 participants