Repository: microsoft/terminal
File: src/cascadia/TerminalCore/TerminalApi.cpp (C++)

Code before:
    if (_currentPrompt)
    {
        const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() };
        // Mark the current cursor pos as the the end of the command. The command
        // is between [mark.end, mark.commandEnd)
        _currentPrompt->commandEnd = cursorPos;
    }
}

void Terminal::CommandFinished(std::optional<unsigned int> error)
{
    if (_currentPrompt)
    {
        const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() };
        // Mark the current cursor pos as the the end of the output. The command
        // is between [mark.commandEnd, mark.outputEnd)
        _currentPrompt->outputEnd = cursorPos;

        if (error.has_value())
        {
            _currentPrompt->category = *error == 0u ?
                                           DispatchTypes::MarkCategory::Success :
                                           DispatchTypes::MarkCategory::Error;
        }
    }
}

// Method Description:
// - Reacts to a client asking us to show or hide the window.
// Arguments:
// - showOrHide - True for show. False for hide.
// Return Value:
// - <none>
void Terminal::ShowWindow(bool showOrHide)
{
    if (_pfnShowWindowChanged)
    {
        _pfnShowWindowChanged(showOrHide);
    }
}

bool Terminal::IsConsolePty() const
{
    return false;
}

bool Terminal::IsVtInputEnabled() const
{
    return false;
}


Review comment (suggestion):
nit: I think this code would be more robust if you implemented it as a small state machine or similar and maybe replace the 3 functions with a single "state transition" function. Because then we could be more confident that the code is robust against invalid VT sequences and that it maintains a proper state at all times.

---

Also, it would lend itself to get rid of the unsafe `_currentPrompt` pointer. That one is somewhat dangerous, since any rogue `_scrollMarks` modification can turn it into a dangling pointer. Unless I'm mistaken, the above `Terminal::AddMark` modification is proof of that: It calls the other `AddMark` overload which calls `_currentPrompt.push_back()`, which might reallocate the vector's backing buffer. Any existing pointer into the vector would then be a dangling pointer, and since `_currentPrompt = last` is only done conditionally, this would potentially make `_currentPrompt` point into deallocated (or since reused) memory. The above `_currentPrompt->` writes would then overwrite random memory.

Because of that I'd suggest changing this into a state machine and get rid of `_currentPrompt`. Instead I'd only call `AddMark` once the state machine returns into the "ground" state.

Code after:
    // redraw the screen
    try
    {
        _activeBuffer().TriggerRedrawAll();
    }
    CATCH_LOG();
}

// NOTE: This is the version of AddMark that comes from VT
void Terminal::MarkPrompt(const DispatchTypes::ScrollMark& mark)
{
    static bool logged = false;
    if (!logged)
    {
        TraceLoggingWrite(
            g_hCTerminalCoreProvider,
            "ShellIntegrationMarkAdded",
            TraceLoggingDescription("A mark was added via VT at least once"),
            TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
            TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));

        logged = true;
    }

    const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() };
    AddMark(mark, cursorPos, cursorPos, false);

    if (mark.category == DispatchTypes::MarkCategory::Prompt)
    {
        _currentPromptState = PromptState::Prompt;
    }
}

void Terminal::MarkCommandStart()
{
    const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() };

    if ((_currentPromptState == PromptState::Prompt) &&
        (_scrollMarks.size() > 0))
    {
        // We were in the right state, and there's a previous mark to work
        // with.
        //
        //We can just do the work below safely.
    }
    else
    {
        // If there was no last mark, or we're in a weird state,
        // then abandon the current state, and just insert a new Prompt mark that
        // start's & ends here, and got to State::Command.