commit 43e70faca24ce0bf4f941a7a3358fbc701228958
parent b43f92331f753c853e2ff1d6d33ffa425a27e6d3
Author: chausner <15180557+chausner@users.noreply.github.com>
Date: Thu, 22 Apr 2021 20:46:29 +0200
Implement VT-based progress reporting (#883)
* Implement VT-based progress reporting
* Update GitHub spelling checker dictionary
* Throw E_INVALIDARG instead of E_UNEXPECTED
Co-authored-by: JohnMcPMS <johnmcp@microsoft.com>
* Rename ProgressState -> State, ProgressBar -> Construct, change parameter to uint32_t
Co-authored-by: chausner <chausner@users.noreply.github.com>
Co-authored-by: JohnMcPMS <johnmcp@microsoft.com>
Diffstat:
5 files changed, 118 insertions(+), 22 deletions(-)
diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt
@@ -48,6 +48,7 @@ cnt
codepage
COMMANDBARFLYOUT
Commandline
+conemu
config
Configurability
constexpr
@@ -421,7 +422,7 @@ tdbuild
tdd
tellg
tempdb
-terabyate
+terabyte
testcontainer
testmoniker
Testrun
@@ -511,6 +512,7 @@ wix
wmain
woah
wofstream
+workaround
workflow
wostream
wpfn
diff --git a/src/AppInstallerCLICore/ExecutionProgress.cpp b/src/AppInstallerCLICore/ExecutionProgress.cpp
@@ -19,7 +19,7 @@ namespace AppInstaller::CLI::Execution
BytesFormatData s_bytesFormatData[] =
{
- // Multi-terabyate installers should be fairly rare for the foreseeable future...
+ // Multi-terabyte installers should be fairly rare for the foreseeable future...
{ 40, "TB"sv },
{ 30, "GB"sv },
{ 20, "MB"sv },
@@ -167,7 +167,7 @@ namespace AppInstaller::CLI::Execution
if (!m_spinnerJob.valid() && !m_spinnerRunning && !m_canceled)
{
m_spinnerRunning = true;
- m_spinnerJob = std::async(std::launch::async, (UseVT() ? &IndefiniteSpinner::ShowSpinnerInternalWithVT : &IndefiniteSpinner::ShowSpinnerInternalNoVT), this);
+ m_spinnerJob = std::async(std::launch::async, &IndefiniteSpinner::ShowSpinnerInternal, this);
}
}
@@ -180,7 +180,7 @@ namespace AppInstaller::CLI::Execution
}
}
- void IndefiniteSpinner::ShowSpinnerInternalNoVT()
+ void IndefiniteSpinner::ShowSpinnerInternal()
{
char spinnerChars[] = { '-', '\\', '|', '/' };
@@ -188,28 +188,37 @@ namespace AppInstaller::CLI::Execution
// showing anything, or a progress task to skip straight to progress.
Sleep(100);
- // Indent two spaces for the spinner, but three here so that we can overwrite it in the loop.
- m_out << " ";
-
- for (size_t i = 0; !m_canceled; ++i)
+ if (!m_canceled)
{
- constexpr size_t repetitionCount = 20;
- ApplyStyle(i % repetitionCount, repetitionCount, true);
- m_out << '\b' << spinnerChars[i % ARRAYSIZE(spinnerChars)] << std::flush;
- Sleep(250);
+ if (UseVT())
+ {
+ // Additional VT-based progress reporting, for terminals that support it
+ m_out << Progress::Construct(Progress::State::Indeterminate);
+ }
+
+ // Indent two spaces for the spinner, but three here so that we can overwrite it in the loop.
+ m_out << " ";
+
+ for (size_t i = 0; !m_canceled; ++i)
+ {
+ constexpr size_t repetitionCount = 20;
+ ApplyStyle(i % repetitionCount, repetitionCount, true);
+ m_out << '\b' << spinnerChars[i % ARRAYSIZE(spinnerChars)] << std::flush;
+ Sleep(250);
+ }
+
+ m_out << "\b \r";
+
+ if (UseVT())
+ {
+ m_out << Progress::Construct(Progress::State::None);
+ }
}
- m_out << "\b \r";
m_canceled = false;
m_spinnerRunning = false;
}
- void IndefiniteSpinner::ShowSpinnerInternalWithVT()
- {
- // Nothing special to do at the moment, can use NoVT version.
- ShowSpinnerInternalNoVT();
- }
-
void ProgressBar::ShowProgress(uint64_t current, uint64_t maximum, ProgressType type)
{
if (current < m_lastCurrent)
@@ -242,6 +251,15 @@ namespace AppInstaller::CLI::Execution
{
m_out << std::endl;
}
+
+ if (UseVT())
+ {
+ // We always clear the VT-based progress bar, even if hideProgressWhenDone is false
+ // since it would be confusing for users if progress continues to be shown after winget exits
+ // (it is typically not automatically cleared by terminals on process exit)
+ m_out << Progress::Construct(Progress::State::None);
+ }
+
m_isVisible = false;
}
}
@@ -349,6 +367,9 @@ namespace AppInstaller::CLI::Execution
m_out << static_cast<int>(percentage * 100) << '%';
break;
}
+
+ // Additional VT-based progress reporting, for terminals that support it
+ m_out << Progress::Construct(Progress::State::Normal, static_cast<int>(percentage * 100));
}
else
{
diff --git a/src/AppInstallerCLICore/ExecutionProgress.h b/src/AppInstallerCLICore/ExecutionProgress.h
@@ -55,9 +55,7 @@ namespace AppInstaller::CLI::Execution
std::atomic<bool> m_spinnerRunning = false;
std::future<void> m_spinnerJob;
- void ShowSpinnerInternalNoVT();
-
- void ShowSpinnerInternalWithVT();
+ void ShowSpinnerInternal();
};
// Displays progress
diff --git a/src/AppInstallerCLICore/VTSupport.cpp b/src/AppInstallerCLICore/VTSupport.cpp
@@ -164,4 +164,63 @@ namespace AppInstaller::CLI::VirtualTerminal
const Sequence EraseLineBackward{ AICLI_VT_CSI "1K" };
const Sequence EraseLineEntirely{ AICLI_VT_CSI "2K" };
}
+
+ namespace Progress
+ {
+ ConstructedSequence Construct(State state, std::optional<uint32_t> percentage)
+ {
+ // See https://conemu.github.io/en/AnsiEscapeCodes.html#ConEmu_specific_OSC
+
+ THROW_HR_IF(E_BOUNDS, percentage.has_value() && percentage > 100u);
+
+ // Workaround some quirks in the Windows Terminal implementation of the progress OSC sequence
+ switch (state)
+ {
+ case State::None:
+ case State::Indeterminate:
+ // Windows Terminal does not recognize the OSC sequence if the progress value is left out.
+ // As a workaround, we can specify an arbitrary value since it does not matter for None and Indeterminate states.
+ percentage = percentage.value_or(0);
+ break;
+ case State::Normal:
+ case State::Error:
+ case State::Paused:
+ // Windows Terminal does not support switching progress states without also setting a progress value at the same time,
+ // so we disallow this case for now.
+ THROW_HR_IF(E_INVALIDARG, !percentage.has_value());
+ break;
+ }
+
+ int stateId;
+ switch (state)
+ {
+ case State::None:
+ stateId = 0;
+ break;
+ case State::Indeterminate:
+ stateId = 3;
+ break;
+ case State::Normal:
+ stateId = 1;
+ break;
+ case State::Error:
+ stateId = 2;
+ break;
+ case State::Paused:
+ stateId = 4;
+ break;
+ default:
+ THROW_HR(E_UNEXPECTED);
+ }
+
+ std::ostringstream result;
+ result << AICLI_VT_OSC "9;4;" << stateId << ";";
+ if (percentage.has_value())
+ {
+ result << percentage.value();
+ }
+ result << AICLI_VT_ESCAPE << "\\";
+ return ConstructedSequence{ result.str() };
+ }
+ }
}
diff --git a/src/AppInstallerCLICore/VTSupport.h b/src/AppInstallerCLICore/VTSupport.h
@@ -3,7 +3,9 @@
#pragma once
#include <AppInstallerLanguageUtilities.h>
+#include <cstdint>
#include <iostream>
+#include <optional>
#include <string>
@@ -133,6 +135,20 @@ namespace AppInstaller::CLI::VirtualTerminal
extern const Sequence EraseLineBackward;
extern const Sequence EraseLineEntirely;
}
+
+ namespace Progress
+ {
+ enum class State
+ {
+ None,
+ Indeterminate,
+ Normal,
+ Paused,
+ Error
+ };
+
+ ConstructedSequence Construct(State state, std::optional<uint32_t> percentage = std::nullopt);
+ }
}
inline std::ostream& operator<<(std::ostream& o, const AppInstaller::CLI::VirtualTerminal::Sequence& s)
