Prefer FMT_COMPILE for string formatting in VtRenderer

.github/actions/spelling/allow/microsoft.txt

@@ -29,6 +29,7 @@ mfcribbon
 microsoft
 microsoftonline
 msixbundle
+MSVC
 muxc
 netcore
 osgvsowi

src/host/ut_host/VtRendererTests.cpp

@@ -1562,7 +1562,7 @@ void VtRendererTest::FormattedString()
         TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method")
     END_TEST_METHOD_PROPERTIES();
 
-    static const std::string format("\x1b[%dm");
+    static const auto format = FMT_COMPILE("\x1b[{}m");
     const auto value = 12;
 
     Viewport view = SetUpViewport();
@@ -1573,15 +1573,15 @@ void VtRendererTest::FormattedString()
 
     Log::Comment(L"1.) Write it once. It should resize itself.");
     qExpectedInput.push_back("\x1b[12m");
-    VERIFY_SUCCEEDED(engine->_WriteFormattedString(&format, value));
+    VERIFY_SUCCEEDED(engine->_WriteFormatted(format, value));
 
     Log::Comment(L"2.) Write the same thing again, should be fine.");
     qExpectedInput.push_back("\x1b[12m");
-    VERIFY_SUCCEEDED(engine->_WriteFormattedString(&format, value));
+    VERIFY_SUCCEEDED(engine->_WriteFormatted(format, value));
 
     Log::Comment(L"3.) Now write something huge. Should resize itself and still be fine.");
-    static const std::string bigFormat("\x1b[28;3;%d;%d;%dm");
+    static const auto bigFormat = FMT_COMPILE("\x1b[28;3;{};{};{}m");
     const auto bigValue = 500;
     qExpectedInput.push_back("\x1b[28;3;500;500;500m");
-    VERIFY_SUCCEEDED(engine->_WriteFormattedString(&bigFormat, bigValue, bigValue, bigValue));
+    VERIFY_SUCCEEDED(engine->_WriteFormatted(bigFormat, bigValue, bigValue, bigValue));
 }

src/renderer/vt/VtSequences.cpp

@@ -81,8 +81,7 @@ using namespace Microsoft::Console::Render;
 // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
 [[nodiscard]] HRESULT VtEngine::_EraseCharacter(const short chars) noexcept
 {
-    static const std::string format = "\x1b[%dX";
-    return _WriteFormattedString(&format, chars);
+    return _WriteFormatted(FMT_COMPILE("\x1b[{}X"), chars);
 }
 
 // Method Description:
@@ -93,8 +92,7 @@ using namespace Microsoft::Console::Render;
 // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
 [[nodiscard]] HRESULT VtEngine::_CursorForward(const short chars) noexcept
 {
-    static const std::string format = "\x1b[%dC";
-    return _WriteFormattedString(&format, chars);
+    return _WriteFormatted(FMT_COMPILE("\x1b[{}C"), chars);
 }
 
 // Method Description:
@@ -132,9 +130,8 @@ using namespace Microsoft::Console::Render;
     {
         return _Write(fInsertLine ? "\x1b[L" : "\x1b[M");
     }
-    const std::string format = fInsertLine ? "\x1b[%dL" : "\x1b[%dM";
 
-    return _WriteFormattedString(&format, sLines);
+    return _WriteFormatted(FMT_COMPILE("\x1b[{}{}"), sLines, fInsertLine ? 'L' : 'M');
 }
 
 // Method Description:
@@ -171,14 +168,12 @@ using namespace Microsoft::Console::Render;
 // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
 [[nodiscard]] HRESULT VtEngine::_CursorPosition(const COORD coord) noexcept
 {
-    static const std::string cursorFormat = "\x1b[%d;%dH";
-
     // VT coords start at 1,1
     COORD coordVt = coord;
     coordVt.X++;
     coordVt.Y++;
 
-    return _WriteFormattedString(&cursorFormat, coordVt.Y, coordVt.X);
+    return _WriteFormatted(FMT_COMPILE("\x1b[{};{}H"), coordVt.Y, coordVt.X);
 }
 
 // Method Description:
@@ -213,8 +208,6 @@ using namespace Microsoft::Console::Render;
 [[nodiscard]] HRESULT VtEngine::_SetGraphicsRendition16Color(const WORD wAttr,
                                                              const bool fIsForeground) noexcept
 {
-    static const std::string fmt = "\x1b[%dm";
-
     // Always check using the foreground flags, because the bg flags constants
     //  are a higher byte
     // Foreground sequences are in [30,37] U [90,97]
@@ -234,7 +227,7 @@ using namespace Microsoft::Console::Render;
                         (WI_IsFlagSet(wAttr, FOREGROUND_GREEN) ? 2 : 0) +
                         (WI_IsFlagSet(wAttr, FOREGROUND_BLUE) ? 4 : 0);
 
-    return _WriteFormattedString(&fmt, vtIndex);
+    return _WriteFormatted(FMT_COMPILE("\x1b[{}m"), vtIndex);
 }
 
 // Method Description:
@@ -248,11 +241,7 @@ using namespace Microsoft::Console::Render;
 [[nodiscard]] HRESULT VtEngine::_SetGraphicsRendition256Color(const WORD index,
                                                               const bool fIsForeground) noexcept
 {
-    const std::string fmt = fIsForeground ?
-                                "\x1b[38;5;%dm" :
-                                "\x1b[48;5;%dm";
-
-    return _WriteFormattedString(&fmt, ::Xterm256ToWindowsIndex(index));
+    return _WriteFormatted(FMT_COMPILE("\x1b[{}8;5;{}m"), fIsForeground ? '3' : '4', ::Xterm256ToWindowsIndex(index));
 }
 
 // Method Description:
@@ -266,15 +255,10 @@ using namespace Microsoft::Console::Render;
 [[nodiscard]] HRESULT VtEngine::_SetGraphicsRenditionRGBColor(const COLORREF color,
                                                               const bool fIsForeground) noexcept
 {
-    const std::string fmt = fIsForeground ?
-                                "\x1b[38;2;%d;%d;%dm" :
-                                "\x1b[48;2;%d;%d;%dm";
-
-    DWORD const r = GetRValue(color);
-    DWORD const g = GetGValue(color);
-    DWORD const b = GetBValue(color);
-
-    return _WriteFormattedString(&fmt, r, g, b);
+    const uint8_t r = GetRValue(color);
+    const uint8_t g = GetGValue(color);
+    const uint8_t b = GetBValue(color);
+    return _WriteFormatted(FMT_COMPILE("\x1b[{}8;2;{};{};{}m"), fIsForeground ? '3' : '4', r, g, b);
 }
 
 // Method Description:
@@ -286,9 +270,7 @@ using namespace Microsoft::Console::Render;
 // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
 [[nodiscard]] HRESULT VtEngine::_SetGraphicsRenditionDefaultColor(const bool fIsForeground) noexcept
 {
-    const std::string_view fmt = fIsForeground ? ("\x1b[39m") : ("\x1b[49m");
-
-    return _Write(fmt);
+    return _Write(fIsForeground ? ("\x1b[39m") : ("\x1b[49m"));
 }
 
 // Method Description:
@@ -300,13 +282,12 @@ using namespace Microsoft::Console::Render;
 // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
 [[nodiscard]] HRESULT VtEngine::_ResizeWindow(const short sWidth, const short sHeight) noexcept
 {
-    static const std::string resizeFormat = "\x1b[8;%d;%dt";
     if (sWidth < 0 || sHeight < 0)
     {
         return E_INVALIDARG;
     }
 
-    return _WriteFormattedString(&resizeFormat, sHeight, sWidth);
+    return _WriteFormatted(FMT_COMPILE("\x1b[8;{};{}t"), sHeight, sWidth);
 }
 
 // Method Description:
@@ -329,8 +310,7 @@ using namespace Microsoft::Console::Render;
 // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
 [[nodiscard]] HRESULT VtEngine::_ChangeTitle(_In_ const std::string& title) noexcept
 {
-    const std::string titleFormat = "\x1b]0;" + title + "\x7";
-    return _Write(titleFormat);
+    return _WriteFormatted(FMT_COMPILE("\x1b]0;{}\x7"), title);
 }
 
 // Method Description:
@@ -472,19 +452,17 @@ using namespace Microsoft::Console::Render;
         // send the auto-assigned ID, prefixed with the PID of this session
         // (we do this so different conpty sessions do not overwrite each other's hyperlinks)
         const auto sessionID = GetCurrentProcessId();
-        const std::string uri_str{ til::u16u8(uri) };
-        auto s = fmt::format(FMT_COMPILE("\x1b]8;id={}-{};{}\x1b\\"), sessionID, numberId, uri_str);
-        return _Write(s);
+        const auto uriStr = til::u16u8(uri);
+        return _WriteFormatted(FMT_COMPILE("\x1b]8;id={}-{};{}\x1b\\"), sessionID, numberId, uriStr);
     }
     else
     {
         // This is the case of user-defined IDs:
         // send the user-defined ID, prefixed with a "u"
         // (we do this so no application can accidentally override a user defined ID)
-        const std::string uri_str{ til::u16u8(uri) };
-        const std::string customId_str{ til::u16u8(customId) };
-        auto s = fmt::format(FMT_COMPILE("\x1b]8;id=u-{};{}\x1b\\"), customId_str, uri_str);
-        return _Write(s);
+        const auto uriStr = til::u16u8(uri);
+        const auto customIdStr = til::u16u8(customId);
+        return _WriteFormatted(FMT_COMPILE("\x1b]8;id=u-{};{}\x1b\\"), customIdStr, uriStr);
     }
 }
 

src/renderer/vt/state.cpp

@@ -115,7 +115,7 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe,
 
     if (!_pipeBroken)
     {
-        bool fSuccess = !!WriteFile(_hFile.get(), _buffer.data(), static_cast<DWORD>(_buffer.size()), nullptr, nullptr);
+        bool fSuccess = !!WriteFile(_hFile.get(), _buffer.data(), gsl::narrow_cast<DWORD>(_buffer.size()), nullptr, nullptr);
         _buffer.clear();
         if (!fSuccess)
         {
@@ -178,71 +178,6 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe,
     return _Write(needed);
 }
 
-// Method Description:
-// - Helper for calling _Write with a string for formatting a sequence. Used
-//      extensively by VtSequences.cpp
-// Arguments:
-// - pFormat: pointer to format string to write to the pipe
-// - ...: a va_list of args to format the string with.
-// Return Value:
-// - S_OK, E_INVALIDARG for a invalid format string, or suitable HRESULT error
-//      from writing pipe.
-[[nodiscard]] HRESULT VtEngine::_WriteFormattedString(const std::string* const pFormat, ...) noexcept
-try
-{
-    va_list args;
-    va_start(args, pFormat);
-
-    // NOTE: pFormat is a pointer because varargs refuses to operate with a ref in that position
-    // NOTE: We're not using string_view because it doesn't guarantee null (which will be needed
-    //       later in the formatting method).
-
-    HRESULT hr = E_FAIL;
-
-    // We're going to hold onto our format string space across calls because
-    // the VT renderer will be formatting a LOT of strings and alloc/freeing them
-    // over and over is going to be way worse for perf than just holding some extra
-    // memory for formatting purposes.
-    // See _formatBuffer for its location.
-
-    // First, plow ahead using our pre-reserved string space.
-    LPSTR destEnd = nullptr;
-    size_t destRemaining = 0;
-    if (SUCCEEDED(StringCchVPrintfExA(_formatBuffer.data(),
-                                      _formatBuffer.size(),
-                                      &destEnd,
-                                      &destRemaining,
-                                      STRSAFE_NO_TRUNCATION,
-                                      pFormat->c_str(),
-                                      args)))
-    {
-        return _Write({ _formatBuffer.data(), _formatBuffer.size() - destRemaining });
-    }
-
-    // If we didn't succeed at filling/using the existing space, then
-    // we're going to take the long way by counting the space required and resizing up to that
-    // space and formatting.
-
-    const auto needed = _scprintf(pFormat->c_str(), args);
-    // -1 is the _scprintf error case https://msdn.microsoft.com/en-us/library/t32cf9tb.aspx
-    if (needed > -1)
-    {
-        _formatBuffer.resize(static_cast<size_t>(needed) + 1);
-
-        const auto written = _vsnprintf_s(_formatBuffer.data(), _formatBuffer.size(), needed, pFormat->c_str(), args);
-        hr = _Write({ _formatBuffer.data(), gsl::narrow<size_t>(written) });
-    }
-    else
-    {
-        hr = E_INVALIDARG;
-    }
-
-    va_end(args);
-
-    return hr;
-}
-CATCH_RETURN();
-
 // Method Description:
 // - This method will update the active font on the current device context
 //      Does nothing for vt, the font is handed by the terminal.

src/renderer/vt/vtrenderer.hpp

@@ -154,9 +154,18 @@ namespace Microsoft::Console::Render
         std::optional<TextColor> _newBottomLineBG{ std::nullopt };
 
         [[nodiscard]] HRESULT _Write(std::string_view const str) noexcept;
-        [[nodiscard]] HRESULT _WriteFormattedString(const std::string* const pFormat, ...) noexcept;
         [[nodiscard]] HRESULT _Flush() noexcept;
 
+        template<typename S, typename... Args>
+        [[nodiscard]] HRESULT _WriteFormatted(S&& format, Args&&... args)
+        try
+        {
+            fmt::basic_memory_buffer<char, 64> buf;
+            fmt::format_to(std::back_inserter(buf), std::forward<S>(format), std::forward<Args>(args)...);
+            return _Write({ buf.data(), buf.size() });
+        }
+        CATCH_RETURN()
+
         void _OrRect(_Inout_ SMALL_RECT* const pRectExisting, const SMALL_RECT* const pRectToOr) const;
         bool _AllIsInvalid() const;