Skip to content

Better debug print of stdin#103

Merged
epage merged 2 commits intoassert-rs:masterfrom
tailhook:shorter_stdin
Jan 26, 2021
Merged

Better debug print of stdin#103
epage merged 2 commits intoassert-rs:masterfrom
tailhook:shorter_stdin

Conversation

@tailhook
Copy link
Contributor

  1. Even if stdout is not fully utf-8, we print all ASCII printable
    characters verbatim and hex-escapes \xAB for non-printable
  2. For inputs larger than 8KiB we only print first and last 2KiB and
    omit bytes in the middle.

This helps reading error messages on larger stdin data (in my use case assert_cmd dumped ~440Mb of data on the terminal).

Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

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

Good idea! I've not run into non-utf8 or large buffers, so this hasn't been a problem for me :)

src/output.rs Outdated
write!(f, "{}", buffer)
} else {
write!(f, "{:?}", buffer)
write_bytes(buffer, f)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should update dump_buffer to match
(ideally one would be implemented in terms of the other)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in the new push. Although, I've replaced it with DebugBytes everywhere, maybe should have been changed the function to return DebugBytes instead.

@tailhook tailhook force-pushed the shorter_stdin branch 4 times, most recently from 5e69026 to 2bc008f Compare January 22, 2021 14:53
1. Even if stdout is not fully utf-8, we print all ASCII printable
   characters verbatim and hex-escapes `\xAB` for non-printable
   (this is actually done by bstr library)
2. For inputs larger than 8KiB we only print first and last 2KiB and
   omit bytes in the middle.

This helps reading error messages on larger stdin data
the latter has nicer representation
@tailhook
Copy link
Contributor Author

Finally got my hands on this. Probably fixed all the feedback.

@epage
Copy link
Contributor

epage commented Jan 26, 2021

Sorry for being slow in responding and thanks for doing this!

I'm working through my crates, doing updates. I can rush out a release of this or wait until I can include my other updates. let me know what your timeline is for this.

@epage epage merged commit 9e8c62a into assert-rs:master Jan 26, 2021
@tailhook tailhook deleted the shorter_stdin branch January 26, 2021 11:44
@tailhook
Copy link
Contributor Author

I'm fine using git version for now. Would appreciate if you ping me once it's released.

@epage
Copy link
Contributor

epage commented Feb 2, 2021

Just released a new version!

takumi-earth pushed a commit to earthlings-dev/assert_cmd that referenced this pull request Jan 27, 2026
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