Repository: microsoft/terminal
File: src/cascadia/TerminalControl/ControlCore.cpp (C++)

Code before:
        auto terminalLock = _terminal->LockForWriting();
        auto context = winrt::make_self<CommandHistoryContext>();
        const auto& textBuffer = _terminal->GetTextBuffer();

        for (const auto& mark : _terminal->GetScrollMarks())
        {
            // The command text is between the `end` (which denotes the end of
            // the prompt) and the `commandEnd`.
            bool markHasCommand = mark.commandEnd.has_value() &&
                                  mark.commandEnd != mark.end;
            if (!markHasCommand)
            {
                continue;
            }

            // Get the text of the command
            const auto line = mark.end.y;
            const auto& row = textBuffer.GetRowByOffset(line);
            const auto commandText = row.GetText(mark.end.x, mark.commandEnd->x);

            // Trim off trailing spaces.
            const auto strEnd = commandText.find_last_not_of(UNICODE_SPACE);
            if (strEnd != std::string::npos)
            {
                const auto trimmed = commandText.substr(0, strEnd + 1);
                context->History().Append(winrt::hstring{ trimmed });
            }
        }
        return *context;
    }

    Core::Scheme ControlCore::ColorScheme() const noexcept
    {
        Core::Scheme s;

        // This part is definitely a hack.
        //
        // This function is usually used by the "Preview Color Scheme"
        // functionality in TerminalPage. If we've got an unfocused appearance,
        // then we've applied that appearance before this is even getting called
        // (because the command palette is open with focus on top of us). If we
        // return the _current_ colors now, we'll return out the _unfocused_
        // colors. If we do that, and the user dismisses the command palette,
        // then the scheme that will get restored is the _unfocused_ one, which
        // is not what we want.
        //
        // So if that's the case, then let's grab the colors from the focused
        // appearance as the scheme instead. We'll lose any current runtime
        // changes to the color table, but those were already blown away when we
        // switched to an unfocused appearance.
        //

Review comment (bug):
The call to `History()` can be hoisted out of the loop.
But what would be even better IMHO would be to make it a constructor argument. Since we got access to the implementation we can even pass the `std::vector` as is.
I'm not sure how I should express why I think it would be better, but you remember the number of bugs we had in the settings UI with the navigation item vector? 😅 I think we should try to avoid such design.

Code after:
        auto terminalLock = _terminal->LockForWriting();
        const auto& textBuffer = _terminal->GetTextBuffer();

        std::vector<winrt::hstring> commands;
        for (const auto& mark : _terminal->GetScrollMarks())
        {
            // The command text is between the `end` (which denotes the end of
            // the prompt) and the `commandEnd`.
            bool markHasCommand = mark.commandEnd.has_value() &&
                                  mark.commandEnd != mark.end;
            if (!markHasCommand)
            {
                continue;
            }

            // Get the text of the command
            const auto line = mark.end.y;
            const auto& row = textBuffer.GetRowByOffset(line);
            const auto commandText = row.GetText(mark.end.x, mark.commandEnd->x);

            // Trim off trailing spaces.
            const auto strEnd = commandText.find_last_not_of(UNICODE_SPACE);
            if (strEnd != std::string::npos)
            {
                const auto trimmed = commandText.substr(0, strEnd + 1);
                commands.push_back(winrt::hstring{ trimmed });
            }
        }
        auto context = winrt::make_self<CommandHistoryContext>(std::move(commands));

        return *context;
    }

    Core::Scheme ControlCore::ColorScheme() const noexcept
    {
        Core::Scheme s;

        // This part is definitely a hack.
        //
        // This function is usually used by the "Preview Color Scheme"
        // functionality in TerminalPage. If we've got an unfocused appearance,
        // then we've applied that appearance before this is even getting called
        // (because the command palette is open with focus on top of us). If we
        // return the _current_ colors now, we'll return out the _unfocused_
        // colors. If we do that, and the user dismisses the command palette,
        // then the scheme that will get restored is the _unfocused_ one, which
        // is not what we want.
        //
        // So if that's the case, then let's grab the colors from the focused
        // appearance as the scheme instead. We'll lose any current runtime
        // changes to the color table, but those were already blown away when we