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

stdout/stderr for text processes is ugly #137

Closed
tailhook opened this issue Oct 21, 2021 · 4 comments · Fixed by #161
Closed

stdout/stderr for text processes is ugly #137

tailhook opened this issue Oct 21, 2021 · 4 comments · Fixed by #161
Labels
enhancement Improve the expected

Comments

@tailhook
Copy link
Contributor

This is partially my failure in #103. As we concentrated in that PR on binary data (which is my other use case), but not on default text output. So instead of printing the text nicely for more common case we print it with escaped \n, so if the output if not one-liner (i.e. some log) it's hard to read.

I see one of two possible approaches:

  1. Format as binary, but line-by line. So weird characters are visible as escapes, but multi-line text output is readable
  2. Detect whether the output is binary or not, (by trying from_utf8 first?). And then print text normally.

What do you think?

@epage epage added the enhancement Improve the expected label Oct 21, 2021
@epage
Copy link
Contributor

epage commented Oct 21, 2021

I'm in favor of

Format as binary, but line-by line. So weird characters are visible as escapes, but multi-line text output is readable

@smoelius
Copy link
Contributor

Format as binary, but line-by line. So weird characters are visible as escapes, but multi-line text output is readable

Is the idea that a linebreak would be added after each \n?

E.g., something like this:

stderr=```<9910 bytes total>"    Blocking waiting for file lock on package cache\n    Checking libc v0.2.138\n    Checking cfg-if v1.0.0\n ....

Would instead look like this?

stderr=```<9910 bytes total>"    Blocking waiting for file lock on package cache\n
    Checking libc v0.2.138\n
    Checking cfg-if v1.0.0\n
....

@tailhook Are you working on this, by chance?

@tailhook
Copy link
Contributor Author

No I'm not working on it.

You don't need \n unescaped linebreak works fine. (if you past your example into a Rust without editing you'll get two linebreaks, which is inconvenient)

@smoelius
Copy link
Contributor

I have a prototype working, but admittedly the boundaries between stdout, stderr, etc. become hard to find when the number of lines is large.

I can see at least six options.

  1. Do nothing and live with the current "ugly" output.
  2. Break lines unconditionally and live with the boundaries being hard to find.
  3. Indent. However, like @tailhook aluded to above, this would cause a disparity between the actual text and the text displayed.
  4. Add a method to Assert to enable this option. However, this could make for a confusing API. E.g., something like the following could cause a user to think with_linebreaks affects the predicate in stdout:
    Command::new(...)
        .assert()
        .with_linebreaks()
        .stdout(...);
  5. Enable this functionality with a feature.
  6. Choose a reasonable limit (similar to MIN_OVERFLOW, etc.) and show only a subset of the lines when the total number of lines would exceed that limit.

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve the expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants