-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[Feature] Prettify Json When Saving Response To File #1829
[Feature] Prettify Json When Saving Response To File #1829
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @elemanhillary, thanks for taking a crack at this! I've left quite a few comments here so let me know if you have any questions.
I also think we should present an additional dropdown button to save "prettified" response because saving the original un-prettified response is still something that should be possible.
cf04e5a
to
4f564f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple more comments 🙂
@@ -96,11 +98,21 @@ class ResponsePane extends React.PureComponent<Props> { | |||
} | |||
|
|||
const readStream = models.response.getBodyStream(response); | |||
let responseBody = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a string, your logic will force data
to be converted from a Buffer a string. For binary data, we need to ensure that it remains a Buffer if it's not a string content-type (eg. PDF document, Image, or mp3).
There's some other code that does this over here that can be referenced:
insomnia/packages/insomnia-app/app/ui/components/viewers/response-multipart.js
Lines 177 to 180 in c03c52e
const dataBuffers = []; | |
part.on('data', data => { | |
dataBuffers.push(data); | |
}); |
}); | ||
readStream.on('end', () => { | ||
const to = fs.createWriteStream(outputPath); | ||
to.write(prettify.json(responseBody)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I think the only thing left to do here is add a check to see if the content type is actually JSON.
There is a contentType
variable defined above that can be used to see if it contains the substring json
. If it is not JSON, we should leave it unprocessed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't know how i forgot that but let me work on it, it's that i get to work on insomnia when am over worked at my work place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No rush @elemanhillary, we're just grateful you're contributing 😄
Also, we're happy to help finish this one if you don't have the time 🤗
4f564f4
to
7872f6d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for making those changes so quickly ❤️
This should be an option and not done by default. Saving the response with whitespace can increase the file size considerably. |
Closes #764