Skip to content
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

Avoid printing unnecessary line separators #174

Merged
merged 3 commits into from
Sep 10, 2021
Merged

Conversation

ducaale
Copy link
Owner

@ducaale ducaale commented Sep 10, 2021

Addresses #137 (comment)

For example, http -v --print= --follow httpbin.org/status/302 shouldn't
print any separators
src/printer.rs Outdated
Comment on lines 298 to 300
if self.print.request_body {
self.buffer.print("\n")?;
}
Copy link
Owner Author

@ducaale ducaale Sep 10, 2021

Choose a reason for hiding this comment

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

I am not a fan of this change so I will revisit this in a later PR when implementing a middleware system + Digest authentication.

src/printer.rs Outdated
// except for print_response_body. We are using this function when we have
// something to print after the response body.
pub fn print_separator(&mut self) -> io::Result<()> {
if self.print.request_body {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I understand this. It looks at the request_body setting, but it comes right after the response_body, right?

Would it make more sense to add the newline into whichever of print_request_body and print_response_body needs it?

(I'm also ok with ignoring this altogether, I don't know if matching the exact number of newlines is important as long as there are no unreasonably short or long gaps.)

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm not sure I understand this. It looks at the request_body setting, but it comes right after the response_body, right?

Aargh, I meant to check self.print.response_body instead of self.print.request_body.

Would it make more sense to add the newline into whichever of print_request_body and print_response_body needs it?

print_request_body does already print a newline. In contrast, print_response_body doesn't add a newline because it is usually the last thing that would be printed (except if it is an intermediate response).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ahh, now everything makes sense.

It might be clearer to inline print_separator at this point but I think it all works out.

@ducaale ducaale merged commit f1da825 into develop Sep 10, 2021
@ducaale ducaale deleted the line-separators branch September 10, 2021 21:35
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