Repository: microsoft/terminal
File: src/host/inputBuffer.cpp (C++)

Code before:
        {
            size_t read = 0;

            for (const auto& wch : source)
            {
                char buffer[8];
                const auto length = WideCharToMultiByte(cp, 0, &wch, 1, &buffer[0], sizeof(buffer), nullptr, nullptr);
                THROW_LAST_ERROR_IF(length <= 0);

                std::string_view slice{ &buffer[0], gsl::narrow_cast<size_t>(length) };
                til::bytes_transfer(target, slice);

                ++read;

                if (!slice.empty())
                {
                    _cachedTextA = slice;
                    _cachedTextReaderA = _cachedTextA;
                    break;
                }
            }

            source = source.substr(read);
        }
    }
}

// Same as `Consume`, but without any `source` characters.
void InputBuffer::ConsumeCached(bool isUnicode, std::span<char>& target)
{
    _switchReadingMode(isUnicode ? ReadingMode::StringW : ReadingMode::StringA);

    if (isUnicode)
    {
        if (!_cachedTextReaderW.empty())
        {
            til::bytes_transfer(target, _cachedTextReaderW);

            if (_cachedTextReaderW.empty())
            {
                // This is just so that we release memory eagerly.
                _cachedTextW = std::wstring{};
            }
        }
    }
    else
    {
        if (!_cachedTextReaderA.empty())
        {
            til::bytes_transfer(target, _cachedTextReaderA);


Review comment (suggestion):
in this case, `target.empty()` means `target` is full, right? based on how `bytes_transfer` works, the length will be zero when there is no space remaining

Code after:
        {
            size_t read = 0;

            for (const auto& s : til::utf16_iterator{ source })
            {
                char buffer[8];
                const auto length = WideCharToMultiByte(cp, 0, s.data(), gsl::narrow_cast<int>(s.size()), &buffer[0], sizeof(buffer), nullptr, nullptr);
                THROW_LAST_ERROR_IF(length <= 0);

                std::string_view slice{ &buffer[0], gsl::narrow_cast<size_t>(length) };
                til::bytes_transfer(target, slice);

                ++read;

                // The _cached members store characters in `target`'s encoding that didn't fit into
                // the client's buffer. So, if slice.empty() == false, then we'll store `slice` there.
                //
                // But it would be incorrect to test for slice.empty() == false, because the exit
                // condition is actually "if the target has no space left" and that's subtly different.
                // This difference can be seen when `source` contains "abc" and `target` is 1 character large.
                // Testing for `target.empty() will ensure we:
                // * exit right after copying "a"
                // * don't store anything in `_cachedTextA`
                // * leave "bc" in the `source` string, for the caller to handle
                // Otherwise we'll copy "a", store "b" and return "c", which is wrong. See GH#16223.
                if (target.empty())
                {
                    if (!slice.empty())
                    {
                        _cachedTextA = slice;
                        _cachedTextReaderA = _cachedTextA;
                    }
                    break;
                }
            }

            source = source.substr(read);
        }
    }
}

// Same as `Consume`, but without any `source` characters.
void InputBuffer::ConsumeCached(bool isUnicode, std::span<char>& target)
{
    _switchReadingMode(isUnicode ? ReadingMode::StringW : ReadingMode::StringA);

    if (isUnicode)
    {
        if (!_cachedTextReaderW.empty())
        {
            til::bytes_transfer(target, _cachedTextReaderW);