Skip to content

Commit

Permalink
Fix overwriting of old output from MultiProgressBar
Browse files Browse the repository at this point in the history
Since PR rpm-software-management#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: rpm-software-management#1899

Also adds couple describing comments.
  • Loading branch information
kontura committed Dec 4, 2024
1 parent 1b34c4e commit 8077a29
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
4 changes: 4 additions & 0 deletions include/libdnf5-cli/progressbar/multi_progress_bar.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
};
Expand Down
25 changes: 25 additions & 0 deletions libdnf5-cli/progressbar/multi_progress_bar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 8077a29

Please sign in to comment.