From 8077a29f2c2085f91b78468ce687bced8c90ebce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= <amatej@redhat.com>
Date: Wed, 4 Dec 2024 15:10:33 +0100
Subject: [PATCH] Fix overwriting of old output from `MultiProgressBar`

Since PR https://github.com/rpm-software-management/dnf5/pull/1825
we are overwriting old output instead of specifically clearing it.
This causes problems when messages are removed from progressbars like it
happens when scriptlets finish successfully without any logs.

Closes: https://github.com/rpm-software-management/dnf5/issues/1899

Also adds couple describing comments.
---
 .../progressbar/multi_progress_bar.hpp        |  4 +++
 .../progressbar/multi_progress_bar.cpp        | 25 +++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/include/libdnf5-cli/progressbar/multi_progress_bar.hpp b/include/libdnf5-cli/progressbar/multi_progress_bar.hpp
index e5683e0db..557a2deb1 100644
--- a/include/libdnf5-cli/progressbar/multi_progress_bar.hpp
+++ b/include/libdnf5-cli/progressbar/multi_progress_bar.hpp
@@ -43,6 +43,9 @@ class LIBDNF_CLI_API MultiProgressBar {
     ~MultiProgressBar();
 
     void add_bar(std::unique_ptr<ProgressBar> && bar);
+
+    // In interactive mode MultiProgressBar doesn't print a newline after unfinished progressbars.
+    // Finished progressbars always end with a newline.
     void print() {
         std::cerr << *this;
         std::cerr << std::flush;
@@ -69,6 +72,7 @@ class LIBDNF_CLI_API MultiProgressBar {
     std::vector<ProgressBar *> bars_todo;
     std::vector<ProgressBar *> bars_done;
     DownloadProgressBar total;
+    // Whether the last line was printed without a new line ending (such as an in progress bar)
     bool line_printed{false};
     std::size_t num_of_lines_to_clear{0};
 };
diff --git a/libdnf5-cli/progressbar/multi_progress_bar.cpp b/libdnf5-cli/progressbar/multi_progress_bar.cpp
index 1787d04bc..d5016d7d3 100644
--- a/libdnf5-cli/progressbar/multi_progress_bar.cpp
+++ b/libdnf5-cli/progressbar/multi_progress_bar.cpp
@@ -106,6 +106,9 @@ std::ostream & operator<<(std::ostream & stream, MultiProgressBar & mbar) {
     text_buffer.str("");
     text_buffer.clear();
 
+    std::size_t last_num_of_lines_to_clear = mbar.num_of_lines_to_clear;
+    std::size_t num_of_lines_permanent = 0;
+
     if (is_interactive && mbar.num_of_lines_to_clear > 0) {
         if (mbar.num_of_lines_to_clear > 1) {
             // Move the cursor up by the number of lines we want to write over
@@ -133,6 +136,8 @@ std::ostream & operator<<(std::ostream & stream, MultiProgressBar & mbar) {
         numbers.pop_back();
         text_buffer << *bar;
         text_buffer << std::endl;
+        num_of_lines_permanent++;
+        num_of_lines_permanent += bar->get_messages().size();
         mbar.bars_done.push_back(bar);
         // TODO(dmach): use iterator
         mbar.bars_todo.erase(mbar.bars_todo.begin() + static_cast<int>(i));
@@ -212,6 +217,26 @@ std::ostream & operator<<(std::ostream & stream, MultiProgressBar & mbar) {
         }
     }
 
+    // If we have written less lines than last time we need to clear the rest otherwise
+    // there would be garbage under the updated progressbar. This is because normally
+    // we don't actually clear the lines we just write over the old output to ensure smooth
+    // output updating.
+    // TODO(amatej): It would be sufficient to do this only once after all progressbars have
+    // finished but unfortunaly MultiProgressBar doesn't have a pImpl so we cannot
+    // store the highest line count it had. We could fix this when breaking ABI.
+    size_t all_written = num_of_lines_permanent + mbar.num_of_lines_to_clear;
+    if (last_num_of_lines_to_clear > all_written) {
+        auto delta = last_num_of_lines_to_clear - all_written;
+        if (!mbar.line_printed) {
+            text_buffer << tty::clear_line;
+        }
+        for (std::size_t i = 0; i < delta; i++) {
+            text_buffer << tty::cursor_down << tty::clear_line;
+        }
+        // Move cursor back up after clearing lines leftover from previous print
+        text_buffer << "\033[" << delta << "A";
+    }
+
     stream << text_buffer.str();  // Single syscall to output all commands
 
     return stream;