Restore newlines when writing Bstrs

[smoelius]

Fixes #137 (cc: @tailhook)

I implemented 6 from #137 (comment), since that seemed to be closest in spirit to the code that was already there.

Note: if a piece of text meets both the "too many lines" and "too many bytes" conditions, the former takes precedence, i.e., I display the text by omitting lines rather than bytes. My reasoning is: if a piece of text meets the "too many lines" condition, it is likely to also meet the "too many bytes" condition. In other words, if you didn't do it this way, this enhancement would almost never apply. The downside to this choice is that it could allow really long lines.

[smoelius]

I pushed a commit to address @tailhook's feedback.

Regarding:

I'm thinking many \n can be in binary data too. So It might be good to ensure that there are at least 10 characters on each line or something to have nicely printed output.

I think this point is perfectly valid, but I also think there are a wide range of heuristics that could be applied. So, assuming this feature is merged, maybe it would be better to let people use it first and see how it behaves on real-world data(?).

[tailhook]

Okay. But I also think NewlineRestorer is a bit more complex than needed. Why not format the same as in the "too many lines" case, but don't omit lines?

[smoelius]

Why not format the same as in the "too many lines" case, but don't omit lines?

Because I had tunnel vision. :|

How's this?

[tailhook]

Looks good, just two tweaks.

1. I think we should tweak defaults a bit:

   const LINES_MIN_OVERFLOW: usize = 100;
   const LINES_MAX_START: usize = 25;
   const LINES_MAX_END: usize = 50;  // tail is more likely to contain error in CLI

2. We probably should have an environment variable like ASSERT_CMD_FULL_OUTPUT to print all the lines.

But now let maintainers of the repo chime in first. /cc @epage

[smoelius]

2. We probably should have an environment variable like ASSERT_CMD_FULL_OUTPUT to print all the lines.

@epage Should I implement this?

[epage]

2. We probably should have an environment variable like ASSERT_CMD_FULL_OUTPUT to print all the lines.

@epage Should I implement this?

Considering we elide output today, I would not consider it a blocker for this but something we can discuss in a new issue.

[epage]

Thanks!

[smoelius]

This feature has significantly improved my quality of life. :) Thanks very much for your feedback and review @tailhook @epage.