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

Code before:
        // GH#6423 - don't dismiss selection if the key that was pressed was a
        // modifier key. We'll wait for a real keystroke to dismiss the
        // GH #7395 - don't dismiss selection when taking PrintScreen
        // selection.
        // GH#8522, GH#3758 - Only modify the selection on key _down_. If we
        // modify on key up, then there's chance that we'll immediately dismiss
        // a selection created by an action bound to a keydown.
        if (HasSelection() &&
            !KeyEvent::IsModifierKey(vkey) &&
            vkey != VK_SNAPSHOT &&
            keyDown)
        {
            if (modifiers.IsCtrlPressed() && vkey == 'A')
            {
                auto lock = _terminal->LockForWriting();
                _terminal->SelectAll();
                _renderer->TriggerSelection();
                return true;
            }

            // try to update the selection
            if (const auto updateSlnParams{ _terminal->ConvertKeyEventToUpdateSelectionParams(modifiers, vkey) })
            {
                auto lock = _terminal->LockForWriting();
                const std::optional<bool> markModeOverride = _terminal->IsInMarkMode() ? modifiers.IsShiftPressed() : std::optional<bool>{};
                _terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, markModeOverride);
                _renderer->TriggerSelection();
                return true;
            }

            // GH#8791 - don't dismiss selection if Windows key was also pressed as a key-combination.
            if (!modifiers.IsWinPressed())
            {
                _terminal->ClearSelection();
                _renderer->TriggerSelection();
            }

            // When there is a selection active, escape should clear it and NOT flow through
            // to the terminal. With any other keypress, it should clear the selection AND
            // flow through to the terminal.
            if (vkey == VK_ESCAPE)
            {
                return true;
            }
        }

        // If the terminal translated the key, mark the event as handled.
        // This will prevent the system from trying to get the character out
        // of it and sending us a CharacterReceived event.
        return vkey ? _terminal->SendKeyEvent(vkey,
                                              scanCode,

Review comment (suggestion):
The `UpdateSelection` logic already depends on `_markMode` and now it depends on it twice - once explicitly internally (`_markMode`) and once implicitly (this parameter being dependent on `_markMode`, but computed from "the outside").

You can add a `ControlKeyStates mods` parameter instead and pass `modifiers` directly, the same way you do it with `ConvertKeyEventToUpdateSelectionParams`. Then ignore the parameter depending on the `_markMode` state.

This works well IMO since `ConvertKeyEventToUpdateSelectionParams` already has a very closely related `(_markMode || mods.IsShiftPressed())` condition.

Code after:
        // GH#6423 - don't dismiss selection if the key that was pressed was a
        // modifier key. We'll wait for a real keystroke to dismiss the
        // GH #7395 - don't dismiss selection when taking PrintScreen
        // selection.
        // GH#8522, GH#3758 - Only modify the selection on key _down_. If we
        // modify on key up, then there's chance that we'll immediately dismiss
        // a selection created by an action bound to a keydown.
        if (HasSelection() &&
            !KeyEvent::IsModifierKey(vkey) &&
            vkey != VK_SNAPSHOT &&
            keyDown)
        {
            if (_terminal->IsInMarkMode() && modifiers.IsCtrlPressed() && vkey == 'A')
            {
                auto lock = _terminal->LockForWriting();
                _terminal->SelectAll();
                _renderer->TriggerSelection();
                return true;
            }

            // try to update the selection
            if (const auto updateSlnParams{ _terminal->ConvertKeyEventToUpdateSelectionParams(modifiers, vkey) })
            {
                auto lock = _terminal->LockForWriting();
                _terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, modifiers);
                _renderer->TriggerSelection();
                return true;
            }

            // GH#8791 - don't dismiss selection if Windows key was also pressed as a key-combination.
            if (!modifiers.IsWinPressed())
            {
                _terminal->ClearSelection();
                _renderer->TriggerSelection();
            }

            // When there is a selection active, escape should clear it and NOT flow through
            // to the terminal. With any other keypress, it should clear the selection AND
            // flow through to the terminal.
            if (vkey == VK_ESCAPE)
            {
                return true;
            }
        }

        // If the terminal translated the key, mark the event as handled.
        // This will prevent the system from trying to get the character out
        // of it and sending us a CharacterReceived event.
        return vkey ? _terminal->SendKeyEvent(vkey,
                                              scanCode,
                                              modifiers,