From 0b2d58a92549069f0433b7857a49dddaf361ddd7 Mon Sep 17 00:00:00 2001 From: Alois Wohlschlager Date: Sat, 8 Mar 2025 13:14:59 +0100 Subject: [PATCH] libmain/progress-bar: fix writeToStdout in multiline mode The multiline progress bar would not clear itself before writing to stdout, leading to interference with the redraw; the most visible symptom is after building in the repl, where the lines indicating the build outputs would be wiped instead of the progress bar header. Separate the steps of erasing and redrawing the progress bar, so that arbitrary output can happen in between in a non-awkward way. In addition to fixing the bug, the code is simplified. Change-Id: I142cf38f5ba5cd672cd6016e996b2f7bf726e9cd --- doc/manual/rl-next/multiline-interference.md | 9 +++++ lix/libmain/progress-bar.cc | 40 +++++++------------- lix/libmain/progress-bar.hh | 4 +- 3 files changed, 24 insertions(+), 29 deletions(-) create mode 100644 doc/manual/rl-next/multiline-interference.md diff --git a/doc/manual/rl-next/multiline-interference.md b/doc/manual/rl-next/multiline-interference.md new file mode 100644 index 000000000..071eb9eba --- /dev/null +++ b/doc/manual/rl-next/multiline-interference.md @@ -0,0 +1,9 @@ +--- +synopsis: Fix interference of the multiline progress bar with output +category: Fixes +cls: [2774] +credits: [alois31] +--- +In some situations, the progress indicator of the multiline progress bar would interfere with persistent output. +This would result in progress bar headers being visible in place of the desired text, for example the outputs shown after a `:b` command in the repl. +The underlying ordering issue has been fixed, so that the undesired interference does not happen any more. diff --git a/lix/libmain/progress-bar.cc b/lix/libmain/progress-bar.cc index d5a74a24e..02fd4766a 100644 --- a/lix/libmain/progress-bar.cc +++ b/lix/libmain/progress-bar.cc @@ -91,7 +91,8 @@ void ProgressBar::resume() while (state->paused == 0) { if (!state->haveUpdate) state.wait_for(updateCV, nextWakeup); - nextWakeup = draw(*state, {}); + eraseProgressDisplay(*state); + nextWakeup = restoreProgressDisplay(*state); state.wait_for(quitCV, std::chrono::milliseconds(50)); } eraseProgressDisplay(*state); @@ -122,13 +123,9 @@ void ProgressBar::logEI(const ErrorInfo & ei) void ProgressBar::log(State & state, Verbosity lvl, std::string_view s) { - if (state.paused == 0) { - draw(state, s); - } else { - auto s2 = s + ANSI_NORMAL "\n"; - if (!isTTY) s2 = filterANSIEscapes(s2, true); - writeLogsToStderr(s2); - } + if (state.paused == 0) eraseProgressDisplay(state); + writeLogsToStderr(filterANSIEscapes(s + ANSI_NORMAL "\n", !isTTY)); + restoreProgressDisplay(state); } void ProgressBar::startActivity( @@ -331,12 +328,12 @@ void ProgressBar::eraseProgressDisplay(State & state) } } -std::chrono::milliseconds ProgressBar::draw(State & state, const std::optional & s) +std::chrono::milliseconds ProgressBar::restoreProgressDisplay(State & state) { auto nextWakeup = A_LONG_TIME; state.haveUpdate = false; - if (state.paused > 0) return nextWakeup; + if (state.paused > 0) return nextWakeup; // when paused, the progress display should not actually be shown auto windowSize = getWindowSize(); auto width = windowSize.second; @@ -344,13 +341,8 @@ std::chrono::milliseconds ProgressBar::draw(State & state, const std::optional::max(); } - eraseProgressDisplay(state); - state.lastLines = 0; - if (s != std::nullopt) - writeLogsToStderr(filterANSIEscapes(s.value(), !isTTY) + ANSI_NORMAL "\n"); - std::string line; std::string status = getStatus(state); if (!status.empty()) { @@ -363,8 +355,8 @@ std::chrono::milliseconds ProgressBar::draw(State & state, const std::optional 0 ? windowSize.first : 25; - auto moreActivities = 0; + auto height = windowSize.first > 0 ? windowSize.first : 25; + auto moreActivities = 0; auto now = std::chrono::steady_clock::now(); std::string activity_line; @@ -546,15 +538,9 @@ std::string ProgressBar::getStatus(State & state) void ProgressBar::writeToStdout(std::string_view s) { auto state(state_.lock()); - if (state->paused == 0) { - if (isTTY && !printMultiline) { - eraseProgressDisplay(*state); - } - Logger::writeToStdout(s); - draw(*state, {}); - } else { - Logger::writeToStdout(s); - } + if (state->paused == 0) eraseProgressDisplay(*state); + Logger::writeToStdout(s); + restoreProgressDisplay(*state); } std::optional ProgressBar::ask(std::string_view msg) @@ -565,7 +551,7 @@ std::optional ProgressBar::ask(std::string_view msg) std::cerr << msg; auto s = trim(readLine(STDIN_FILENO)); if (s.size() != 1) return {}; - draw(*state, {}); + restoreProgressDisplay(*state); return s[0]; } diff --git a/lix/libmain/progress-bar.hh b/lix/libmain/progress-bar.hh index 99e280ced..d8826fe13 100644 --- a/lix/libmain/progress-bar.hh +++ b/lix/libmain/progress-bar.hh @@ -97,8 +97,6 @@ struct ProgressBar : public Logger void update(State & state); - std::chrono::milliseconds draw(State & state, const std::optional & s); - std::string getStatus(State & state); void writeToStdout(std::string_view s) override; @@ -111,6 +109,8 @@ struct ProgressBar : public Logger private: void eraseProgressDisplay(State & state); + + std::chrono::milliseconds restoreProgressDisplay(State & state); }; Logger * makeProgressBar();