search

davazp / graphql-mode

An Emacs mode for GraphQL
GNU General Public License v3.0
162 stars 31 forks source link

Don't use temp buffer, as it breaks local variables #24

Closed Malabarba closed 5 years ago

Malabarba commented 5 years ago

In the previous implementation, setting graphql-extra-headers as a local variable didn't work because the variable was used inside graphql-post-request which was called inside a temp buffer.

Removing the temp buffer solves this issue.

Maybe the temp buffer was there for a specific reason, and shouldn't be removed. Let me know if that's the case.

davazp commented 5 years ago

Thanks! Note that, from the request documentation:

Request.el has a single entry point. It is ‘request’. ... Keyword argument Explanation ... DATA (string/alist) data to be sent to the server PARSER (symbol) a function that reads current buffer and return data

https://github.com/davazp/graphql-mode/blob/ab58192967f589f2d767cf557d73140d3706090d/graphql-mode.el#L115-L120

So it seems the json-read would need to read the json from a buffer? but :data already contains the body. Can you double check that we can send POST queries to a server after this change? If it works, we can merge it.

Otherwise we would need to read the required file variables before/after.

Malabarba commented 5 years ago

From what I understand, the :parser argument is for the response data, not the input data. The input data is already provided in the :data argument. So that shouldn't be an issue.

That said, it seems that my PR does break the command. So I'm still trying to figure out why that is.

Malabarba commented 5 years ago

Actually, never mind, the code is working for me. What confused me was the fact that ran into another bug (opening a separate PR for that).

davazp commented 5 years ago

Great. Thanks again.