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.

Sign up for GitHub

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

progressbar-padding #2

Closed
wants to merge 2 commits into from
Closed

progressbar-padding #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d20e63d
Add padding to ProgressBar messages to avoid overlapping
kontura Dec 6, 2024
92ab80c
Support ProgressBar messages with wide characters
evan-goode Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions libdnf5-cli/progressbar/download_progress_bar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.

#include "libdnf5-cli/progressbar/download_progress_bar.hpp"

#include "../utils/utf8.hpp"

#include "libdnf5-cli/tty.hpp"

#include <algorithm>
Expand Down Expand Up @@ -181,8 +183,10 @@ void DownloadProgressBar::to_stream(std::ostream & stream) {
auto message_type = msg.first;
auto message = msg.second;

const auto & prefix = ">>> ";

stream << std::endl;
stream << ">>> ";
stream << prefix;

color_used = false;
if (tty::is_interactive()) {
Expand All @@ -205,9 +209,17 @@ void DownloadProgressBar::to_stream(std::ostream & stream) {
}
}

// Add padding to fully fill the terminal_width, this is because MultiProgressBar
// overrides its own messages, it doesn't clear the lines.
// If the message is short some leftover characters could be still present after it.
const auto prefix_width = libdnf5::cli::utils::utf8::width(prefix);
const auto message_width = libdnf5::cli::utils::utf8::width(message);
if (message_width < terminal_width - prefix_width) {
message.append(terminal_width - message_width - prefix_width, ' ');
}

// print only part of the message that fits the terminal width
// subtracted '4' relates to the '>>> ' prefix
stream << message.substr(0, terminal_width - 4);
stream << libdnf5::cli::utils::utf8::substr_width(message, 0, terminal_width - prefix_width);

if (color_used) {
stream << tty::reset;
Expand Down
68 changes: 59 additions & 9 deletions test/libdnf5-cli/test_progressbar_interactive.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ void ProgressbarInteractiveTest::setUp() {
setenv("DNF5_FORCE_INTERACTIVE", "1", 1);
// Force columns to 70 to make output independ of where it is run
setenv("FORCE_COLUMNS", "70", 1);
// Wide characters do not work at all until we set locales in the code
setlocale(LC_ALL, "C.UTF-8");
}

void ProgressbarInteractiveTest::tearDown() {
Expand Down Expand Up @@ -175,15 +177,18 @@ void ProgressbarInteractiveTest::test_download_progress_bar_with_messages() {
download_progress_bar->set_state(libdnf5::cli::progressbar::ProgressBarState::STARTED);
download_progress_bar->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test message1");
download_progress_bar->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test message2");
download_progress_bar->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test もで 諤奯ゞ");

std::ostringstream oss;
(*download_progress_bar.*get(to_stream{}))(oss);
Pattern expected =
"\\[0/0\\] test 40% | ????? ??B\\/s | 4.0 B | ???????\n"
">>> test message1\n"
">>> test message2";
">>> test message1 \n"
">>> test message2 \n"
">>> test もで 諤奯ゞ ";
ASSERT_MATCHES(expected, oss.str());

download_progress_bar->pop_message();
download_progress_bar->pop_message();
download_progress_bar->pop_message();

Expand Down Expand Up @@ -238,7 +243,7 @@ void ProgressbarInteractiveTest::test_multi_progress_bar_with_messages_with_tota
oss << multi_progress_bar;
Pattern expected =
"\\[1/1\\] test 40% | ????? ??B\\/s | 4.0 B | ???????\n"
">>> test message1\n"
">>> test message1 \n"
"----------------------------------------------------------------------\n"
"\\[0/1\\] Total 40% | ????? ??B\\/s | 4.0 B | ???????";

Expand Down Expand Up @@ -270,7 +275,7 @@ void ProgressbarInteractiveTest::test_multi_progress_bar_with_messages_with_tota

download_progress_bar_raw->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test message1");
download_progress_bar_raw->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test message2");
download_progress_bar_raw->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test message3");
download_progress_bar_raw->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test もで 諤奯ゞ");
oss << multi_progress_bar;
download_progress_bar_raw->pop_message();
download_progress_bar_raw->pop_message();
Expand Down Expand Up @@ -312,19 +317,22 @@ void ProgressbarInteractiveTest::test_multi_progress_bars_with_messages_with_tot
download_progress_bar2_raw->set_state(libdnf5::cli::progressbar::ProgressBarState::STARTED);
download_progress_bar2_raw->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test message1");
download_progress_bar2_raw->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test message2");
download_progress_bar2_raw->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test もで 諤奯ゞ");

std::ostringstream oss;
oss << multi_progress_bar;
Pattern expected =
"\\[1/2\\] test1 100% | ????? ??B\\/s | 10.0 B | ???????\n"
"\\[2/2\\] test2 40% | ????? ??B\\/s | 4.0 B | ???????\n"
">>> test message1\n"
">>> test message2\n"
">>> test message1 \n"
">>> test message2 \n"
">>> test もで 諤奯ゞ \n"
"----------------------------------------------------------------------\n"
"\\[1/2\\] Total 70% | ????? ??B\\/s | 14.0 B | ???????";

ASSERT_MATCHES(expected, perform_control_sequences(oss.str()));

download_progress_bar2_raw->pop_message();
download_progress_bar2_raw->pop_message();
download_progress_bar2_raw->pop_message();
oss << multi_progress_bar;
Expand All @@ -333,6 +341,7 @@ void ProgressbarInteractiveTest::test_multi_progress_bars_with_messages_with_tot
"\\[2/2\\] test2 40% | ????? ??B\\/s | 4.0 B | ???????\n"
"----------------------------------------------------------------------\n"
"\\[1/2\\] Total 70% | ????? ??B\\/s | 14.0 B | ???????\n"
"\n"
"\n";
ASSERT_MATCHES(expected, perform_control_sequences(oss.str()));

Expand Down Expand Up @@ -363,7 +372,7 @@ void ProgressbarInteractiveTest::test_multi_progress_bar_with_messages() {
oss << multi_progress_bar;
Pattern expected =
"\\[1/1\\] test 40% | ????? ??B\\/s | 4.0 B | ???????\n"
">>> test message1";
">>> test message1 ";

ASSERT_MATCHES(expected, perform_control_sequences(oss.str()));

Expand Down Expand Up @@ -411,24 +420,28 @@ void ProgressbarInteractiveTest::test_multi_progress_bars_with_messages() {
download_progress_bar2_raw->set_state(libdnf5::cli::progressbar::ProgressBarState::STARTED);
download_progress_bar2_raw->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test message1");
download_progress_bar2_raw->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test message2");
download_progress_bar2_raw->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test こんにちは世界！");

std::ostringstream oss;
oss << multi_progress_bar;
Pattern expected =
"\\[1/2\\] test1 100% | ????? ??B\\/s | 10.0 B | ???????\n"
"\\[2/2\\] test2 40% | ????? ??B\\/s | 4.0 B | ???????\n"
">>> test message1\n"
">>> test message2";
">>> test message1 \n"
">>> test message2 \n"
">>> test こんにちは世界！ ";

ASSERT_MATCHES(expected, perform_control_sequences(oss.str()));

download_progress_bar2_raw->pop_message();
download_progress_bar2_raw->pop_message();
download_progress_bar2_raw->pop_message();
oss << multi_progress_bar;

expected =
"\\[1/2\\] test1 100% | ????? ??B\\/s | 10.0 B | ???????\n"
"\\[2/2\\] test2 40% | ????? ??B\\/s | 4.0 B | ???????\n"
"\n"
"\n";
ASSERT_MATCHES(expected, perform_control_sequences(oss.str()));

Expand All @@ -441,6 +454,7 @@ void ProgressbarInteractiveTest::test_multi_progress_bars_with_messages() {
expected =
"\\[1/2\\] test1 100% | ????? ??B\\/s | 10.0 B | ???????\n"
"\\[2/2\\] test2 40% | ????? ??B\\/s | 4.0 B | ???????\n"
"\n"
"\n";

ASSERT_MATCHES(expected, perform_control_sequences(oss.str()));
Expand All @@ -467,3 +481,39 @@ void ProgressbarInteractiveTest::test_multi_progress_bars_with_messages() {

ASSERT_MATCHES(expected, perform_control_sequences(oss.str()));
}

void ProgressbarInteractiveTest::test_multi_progress_bar_with_short_messages() {
// With single bar and Total disabled
// First print long message and remove it (successful scriptlet for package with long name),
// then print short msg (different package that prints something to the log and the message stays).

auto download_progress_bar = std::make_unique<libdnf5::cli::progressbar::DownloadProgressBar>(10, "test");

libdnf5::cli::progressbar::MultiProgressBar multi_progress_bar;
multi_progress_bar.set_total_bar_visible_limit(libdnf5::cli::progressbar::MultiProgressBar::NEVER_VISIBLE_LIMIT);
auto download_progress_bar_raw = download_progress_bar.get();
multi_progress_bar.add_bar(std::move(download_progress_bar));

download_progress_bar_raw->set_ticks(4);
download_progress_bar_raw->set_state(libdnf5::cli::progressbar::ProgressBarState::STARTED);
download_progress_bar_raw->add_message(
libdnf5::cli::progressbar::MessageType::INFO, "test loooooooooooooooooooooooooooooooooooooooooong message");

std::ostringstream oss;
oss << multi_progress_bar;
Pattern expected =
"\\[1/1\\] test 40% | ????? ??B\\/s | 4.0 B | ???????\n"
">>> test loooooooooooooooooooooooooooooooooooooooooong message ";

ASSERT_MATCHES(expected, perform_control_sequences(oss.str()));

download_progress_bar_raw->pop_message();
download_progress_bar_raw->add_message(libdnf5::cli::progressbar::MessageType::INFO, "test short message");

oss << multi_progress_bar;
expected =
"\\[1/1\\] test 40% | ????? ??B\\/s | 4.0 B | ???????\n"
">>> test short message ";

ASSERT_MATCHES(expected, perform_control_sequences(oss.str()));
}
2 changes: 2 additions & 0 deletions test/libdnf5-cli/test_progressbar_interactive.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class ProgressbarInteractiveTest : public CppUnit::TestCase {
CPPUNIT_TEST(test_multi_progress_bar_with_messages_with_total);
CPPUNIT_TEST(test_multi_progress_bars_with_messages_with_total);
CPPUNIT_TEST(test_multi_progress_bar_with_messages);
CPPUNIT_TEST(test_multi_progress_bar_with_short_messages);
CPPUNIT_TEST(test_multi_progress_bars_with_messages);

CPPUNIT_TEST_SUITE_END();
Expand All @@ -49,6 +50,7 @@ class ProgressbarInteractiveTest : public CppUnit::TestCase {
void test_multi_progress_bar_with_messages_with_total();
void test_multi_progress_bars_with_messages_with_total();
void test_multi_progress_bar_with_messages();
void test_multi_progress_bar_with_short_messages();
void test_multi_progress_bars_with_messages();
};

Expand Down
Loading