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

Extra line after the progress bar #334

Closed
ishitatsuyuki opened this issue Dec 16, 2021 · 5 comments · Fixed by #338
Closed

Extra line after the progress bar #334

ishitatsuyuki opened this issue Dec 16, 2021 · 5 comments · Fixed by #338

Comments

@ishitatsuyuki
Copy link
Contributor

It seems that indicatif has an extra line after the progress bar, which might look weird to some users. Comparison below:

Yarn

image

Cargo

image

Indicatif

image

@djc
Copy link
Collaborator

djc commented Dec 16, 2021

I suppose this might be a regression from #319, would you mind checking? If not, I'd be open to PRs to fix this, though ideally we would some digging first to see if there was a good reason to do this.

@redzic
Copy link
Contributor

redzic commented Dec 16, 2021

It was always printed with an extra line, even before #319

@djc
Copy link
Collaborator

djc commented Dec 16, 2021

@mitsuhiko do you remember context/rationale for this?

@sigmaSd
Copy link
Contributor

sigmaSd commented Dec 24, 2021

I think it was just easier to implement because indicatif supports multiple lines

Here is my attempt to make it work, the general idea is to not append a '\n' to the last line, and fix the math for it (changes console and indicatif)

diff --git a/src/term.rs b/src/term.rs
index 56287dc..2d8cee1 100644
--- a/src/term.rs
+++ b/src/term.rs
@@ -435,12 +435,14 @@ impl Term {
     /// This positions the cursor at the beginning of the first line
     /// that was cleared.
     pub fn clear_last_lines(&self, n: usize) -> io::Result<()> {
-        self.move_cursor_up(n)?;
-        for _ in 0..n {
+        self.move_cursor_up(n.saturating_sub(1))?;
+        for i in 0..n {
             self.clear_line()?;
-            self.move_cursor_down(1)?;
+            if i + 1 != n {
+                self.move_cursor_down(1)?;
+            }
         }
-        self.move_cursor_up(n)?;
+        self.move_cursor_up(n.saturating_sub(1))?;
         Ok(())
     }
 
diff --git a/src/draw_target.rs b/src/draw_target.rs
index 704026a..949c4e1 100644
--- a/src/draw_target.rs
+++ b/src/draw_target.rs
@@ -389,8 +389,13 @@ impl ProgressDrawState {
     }
 
     pub fn draw_to_term(&self, term: &Term) -> io::Result<()> {
-        for line in &self.lines {
-            term.write_line(line)?;
+        let len = self.lines.len();
+        for (idx, line) in self.lines.iter().enumerate() {
+            if idx + 1 != len {
+                term.write_line(line)?;
+            } else {
+                term.write_str(line)?;
+            }
         }
         Ok(())
    }

@djc
Copy link
Collaborator

djc commented Dec 27, 2021

I'm fine with reviewing changes along these lines if someone submits a PR.

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 a pull request may close this issue.

4 participants