Skip to content

Handle newline in msg and empty msg#540

Merged
chris-laplante merged 1 commit intoconsole-rs:mainfrom
RDruon:rdruon/newline_empty_msg
May 11, 2023
Merged

Handle newline in msg and empty msg#540
chris-laplante merged 1 commit intoconsole-rs:mainfrom
RDruon:rdruon/newline_empty_msg

Conversation

@RDruon
Copy link
Copy Markdown
Contributor

@RDruon RDruon commented May 11, 2023

Using println() with a new line at the beginning of the message trigger some artifacts, see below:

newline_artifact.rs

use indicatif::ProgressBar;

fn main() {
    let pb = ProgressBar::new(1024);
    pb.inc(1);
    pb.println("\nShould be a new line before this message and no duplicate progress bar");
    pb.finish_with_message("done");
}

Output on main:
image
Output with this patch:
image

Using println() with an empty string does not trigger new line:
empty_msg.rs

use indicatif::ProgressBar;

fn main() {
    let pb = ProgressBar::new(1024);
    pb.inc(1);
    pb.println("");
    pb.println("There should an empty line before this message");
    pb.finish_with_message("done");
}

Output on main:
image

Output with this patch:
image

New test in render.rs showcase both issue

djc
djc approved these changes May 11, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

This makes sense to me, modulo the formatting issue found in CI.

@RDruon RDruon force-pushed the rdruon/newline_empty_msg branch from 74441ac to 7dfa46e Compare May 11, 2023 14:57
@RDruon
Copy link
Copy Markdown
Contributor Author

RDruon commented May 11, 2023

This makes sense to me, modulo the formatting issue found in CI.

I fixed the format issue

@chris-laplante chris-laplante merged commit 59cd606 into console-rs:main May 11, 2023
@djc djc mentioned this pull request May 24, 2023
Merged
@indiv0 indiv0 mentioned this pull request Jun 13, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@djc djc djc approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@RDruon @djc @chris-laplante