danirod / cartero

Make HTTP requests and test APIs
https://cartero.danirod.es
GNU General Public License v3.0
399 stars 30 forks source link

feat. format json on response body tab #38

Closed devLuisDonaldo closed 3 months ago

devLuisDonaldo commented 3 months ago

First of all, i'm sorry

I need to say that i am not a rust/gtk dev, maybe I shouldn't do this

Sometimes responses json type dont format correctly on body panel Captura de pantalla 2024-07-26 a la(s) 12 17 02 a m

for that i applied some lines for try render a bit better json values

Captura de pantalla 2024-07-26 a la(s) 12 16 00 a m

I'm not sure if this help or improve performance with long single line

i really apreciate any type of feedback and tips, or if I can help you in other ways i'll do it with pleasure

pd: i like yout content, you do a great job

danirod commented 3 months ago

Okay, first of all, thank you so much for adding this feature. I had been told that prettifying the JSON document was necessary because JSON documents that are minified are difficult to read and even cause performance issues if they are too long. So this will be very helpful.

I believe, however, that the way the body is probed to check if it is a JSON document may be non-exhaustive:

https://github.com/danirod/cartero/blob/9194960aa60ace06dbd402b01c5e4c74fd37af1f/src/widgets/response_panel.rs#L263

Specifically, I see two downsides here:

A more concise test may be to check whether the response has a Content-Type header that is of JSON type. Something like "does the header contains /json or contains +json?" That should catch content-types like application/json, text/json and application/vnd.github.raw+json (which is still JSON).

I am just talking and haven't checked whether this compiles, but it should be possible to get the content-type via the Response struct, and then checking if there is a header there that contains +json or /json.

if let let Some(type) = resp.headers.header("Content-Type") {
  if type[0].contains("+json") || type[0].contains("/json") {
    // Is JSON
  }
}

I was planning on releasing 0.1 today. I can patch it after work and let the 0.1 release already have this feature. However, if you want to change it by yourself, it's fine.

devLuisDonaldo commented 3 months ago

Thank you for the observations. I wanted to apply your validations, but I was working, and maybe I would have been wrong. I saw your stream, and I need to practice more to understand how this works Certainly, your streams teach like it was a course

BTW, congratulations for the release