Merge branch 'main' into dev/cazamor/selfhost/11-18

Author: carlos-zamora

.github/actions/spelling/expect/expect.txt

@@ -495,6 +495,7 @@ DSBUFFERDESC
 DSBVOLUME
 dsm
 dsound
+DSRCPR
 DSSCL
 DSwap
 DTo

src/cascadia/TerminalControl/ControlCore.cpp

@@ -1787,6 +1787,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     void ControlCore::ClearSearch()
     {
         const auto lock = _terminal->LockForWriting();
+
+        // GH #19358: select the focused search result before clearing search
+        if (const auto focusedSearchResult = _terminal->GetSearchHighlightFocused())
+        {
+            _terminal->SetSelectionAnchor(focusedSearchResult->start);
+            _terminal->SetSelectionEnd(focusedSearchResult->end);
+            _renderer->TriggerSelection();
+        }
+
         _terminal->SetSearchHighlights({});
         _terminal->SetSearchHighlightFocused(0);
         _renderer->TriggerSearchHighlight(_searcher.Results());

src/cascadia/TerminalCore/TerminalSelection.cpp

@@ -362,10 +362,30 @@ void Terminal::ToggleMarkMode()
     {
         // Enter Mark Mode
         // NOTE: directly set cursor state. We already should have locked before calling this function.
-        if (!IsSelectionActive())
+        if (IsSelectionActive())
+        {
+            // Selection already existed --> just target "end"
+            if (WI_AreAllFlagsClear(_selectionEndpoint, SelectionEndpoint::Start | SelectionEndpoint::End))
+            {
+                WI_SetFlag(_selectionEndpoint, SelectionEndpoint::End);
+            }
+        }
+        else if (const auto focusedSearchResult = GetSearchHighlightFocused())
+        {
+            // We have a focused search result --> treat it as selection
+            *_selection.write() = SelectionInfo{
+                .start = focusedSearchResult->start,
+                .end = focusedSearchResult->end,
+                .pivot = focusedSearchResult->start,
+                .blockSelection = false,
+                .active = true,
+            };
+            WI_SetFlag(_selectionEndpoint, SelectionEndpoint::End);
+        }
+        else
         {
             // If we're scrolled up, use the viewport origin as the selection start.
-            // Otherwise, use the cursor position.
+            // Otherwise, just use the cursor position.
             const auto cursorPos = _scrollOffset != 0 ? _GetVisibleViewport().Origin() :
                                                         _activeBuffer().GetCursor().GetPosition();
             *_selection.write() = SelectionInfo{
@@ -377,11 +397,6 @@ void Terminal::ToggleMarkMode()
             };
             WI_SetAllFlags(_selectionEndpoint, SelectionEndpoint::Start | SelectionEndpoint::End);
         }
-        else if (WI_AreAllFlagsClear(_selectionEndpoint, SelectionEndpoint::Start | SelectionEndpoint::End))
-        {
-            // Selection already existed
-            WI_SetFlag(_selectionEndpoint, SelectionEndpoint::End);
-        }
         _ScrollToPoint(_selection->start);
         _selectionMode = SelectionInteractionMode::Mark;
         _selectionIsTargetingUrl = false;

src/host/VtInputThread.cpp

@@ -21,13 +21,13 @@ using namespace Microsoft::Console::VirtualTerminal;
 // - hPipe - a handle to the file representing the read end of the VT pipe.
 // - inheritCursor - a bool indicating if the state machine should expect a
 //      cursor positioning sequence. See MSFT:15681311.
-VtInputThread::VtInputThread(_In_ wil::unique_hfile hPipe, const bool inheritCursor) :
+VtInputThread::VtInputThread(_In_ wil::unique_hfile hPipe) :
     _hFile{ std::move(hPipe) }
 {
     THROW_HR_IF(E_HANDLE, _hFile.get() == INVALID_HANDLE_VALUE);
 
     auto dispatch = std::make_unique<InteractDispatch>();
-    auto engine = std::make_unique<InputStateMachineEngine>(std::move(dispatch), inheritCursor);
+    auto engine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     _pInputStateMachine = std::make_unique<StateMachine>(std::move(engine));
 }
 
@@ -185,8 +185,14 @@ void VtInputThread::_InputThread()
     return S_OK;
 }
 
+void VtInputThread::CaptureNextCursorPositionReport() const noexcept
+{
+    auto& engine = static_cast<InputStateMachineEngine&>(_pInputStateMachine->Engine());
+    engine.CaptureNextCursorPositionReport();
+}
+
 til::enumset<DeviceAttribute, uint64_t> VtInputThread::WaitUntilDA1(DWORD timeout) const noexcept
 {
-    const auto& engine = static_cast<InputStateMachineEngine&>(_pInputStateMachine->Engine());
+    auto& engine = static_cast<InputStateMachineEngine&>(_pInputStateMachine->Engine());
     return engine.WaitUntilDA1(timeout);
 }

src/host/VtInputThread.hpp

@@ -26,9 +26,10 @@ namespace Microsoft::Console
     class VtInputThread
     {
     public:
-        VtInputThread(_In_ wil::unique_hfile hPipe, const bool inheritCursor);
+        explicit VtInputThread(_In_ wil::unique_hfile hPipe);
 
         [[nodiscard]] HRESULT Start();
+        void CaptureNextCursorPositionReport() const noexcept;
         til::enumset<VirtualTerminal::DeviceAttribute, uint64_t> WaitUntilDA1(DWORD timeout) const noexcept;
 
     private:

src/host/VtIo.cpp

@@ -155,7 +155,7 @@ bool VtIo::IsUsingVt() const
     {
         if (IsValidHandle(_hInput.get()))
         {
-            _pVtInputThread = std::make_unique<VtInputThread>(std::move(_hInput), _lookingForCursorPosition);
+            _pVtInputThread = std::make_unique<VtInputThread>(std::move(_hInput));
         }
     }
     CATCH_RETURN();
@@ -177,7 +177,7 @@ bool VtIo::IsUsingVt() const
             // wait for the DA1 response below and effectively wait for both.
             if (_lookingForCursorPosition)
             {
-                writer.WriteUTF8("\x1b[6n"); // Cursor Position Report (DSR CPR)
+                writer.WriteDSRCPR();
             }
 
             // GH#4999 - Send a sequence to the connected terminal to request
@@ -720,6 +720,19 @@ void VtIo::Writer::WriteASB(bool enabled) const
     _io->_back.append(&buf[0], std::size(buf) - 1);
 }
 
+// DSR CPR: Cursor Position Report
+bool VtIo::Writer::WriteDSRCPR() const
+{
+    if (!_io->_pVtInputThread)
+    {
+        return false;
+    }
+
+    _io->_back.append("\x1b[6n");
+    _io->_pVtInputThread->CaptureNextCursorPositionReport();
+    return true;
+}
+
 void VtIo::Writer::WriteWindowVisibility(bool visible) const
 {
     char buf[] = "\x1b[1t";

src/host/VtIo.hpp

@@ -40,6 +40,7 @@ namespace Microsoft::Console::VirtualTerminal
             void WriteSGR1006(bool enabled) const;
             void WriteDECAWM(bool enabled) const;
             void WriteASB(bool enabled) const;
+            bool WriteDSRCPR() const;
             void WriteWindowVisibility(bool visible) const;
             void WriteWindowTitle(std::wstring_view title) const;
             void WriteAttributes(const TextAttribute& attributes) const;

src/host/screenInfo.cpp

@@ -1404,6 +1404,8 @@ NT_CATCH_RETURN()
 
     if (SUCCEEDED_NTSTATUS(status))
     {
+        SetConptyCursorPositionMayBeWrong();
+
         // Fire off an event to let accessibility apps know the layout has changed.
         if (IsActiveScreenBuffer())
         {
@@ -1422,6 +1424,86 @@ NT_CATCH_RETURN()
     return status;
 }
 
+// If we're ConPTY, our copy of the buffer may be out of sync with the terminal,
+// because our VT, resize reflow, etc., implementation may be different.
+// For some operations we set a flag to indicate the cursor position may be wrong.
+// For GetConsoleScreenBufferInfo(Ex) we then fetch the latest position from the terminal.
+// This fixes some of the most glaring out of sync issues. See GH#18725.
+bool SCREEN_INFORMATION::ConptyCursorPositionMayBeWrong() const noexcept
+{
+    return _conptyCursorPositionMayBeWrong.load(std::memory_order_relaxed);
+}
+
+// This should be called whenever we do something that may desynchronize
+// our cursor position from the terminal's, e.g. a buffer resize.
+// See ConptyCursorPositionMayBeWrong().
+void SCREEN_INFORMATION::SetConptyCursorPositionMayBeWrong() noexcept
+{
+    auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+    assert(gci.IsConsoleLocked());
+
+    if (gci.IsInVtIoMode())
+    {
+        _conptyCursorPositionMayBeWrong.store(true, std::memory_order_relaxed);
+    }
+}
+
+// This should be called whenever we've synchronized our cursor position again.
+// See ConptyCursorPositionMayBeWrong().
+void SCREEN_INFORMATION::ResetConptyCursorPositionMayBeWrong() noexcept
+{
+    _conptyCursorPositionMayBeWrong.store(false, std::memory_order_relaxed);
+    til::atomic_notify_all(_conptyCursorPositionMayBeWrong);
+}
+
+// Call this to synchronously wait until the ConPTY cursor position
+// is known to be correct again. To do so, this emits a DSR CPR sequence
+// and waits for a response from the terminal.
+// See ConptyCursorPositionMayBeWrong().
+void SCREEN_INFORMATION::WaitForConptyCursorPositionToBeSynchronized() noexcept
+{
+    if (!_conptyCursorPositionMayBeWrong.load(std::memory_order_relaxed))
+    {
+        return;
+    }
+
+    auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
+
+    {
+        gci.LockConsole();
+        const auto exit = wil::scope_exit([&] { gci.UnlockConsole(); });
+        auto writer = gci.GetVtWriterForBuffer(this);
+
+        if (!writer || !writer.WriteDSRCPR())
+        {
+            _conptyCursorPositionMayBeWrong.store(false, std::memory_order_relaxed);
+            return;
+        }
+
+        writer.Submit();
+    }
+
+    // If you were to hold the lock, the ConPTY input thread couldn't
+    // process any input and thus couldn't update the cursor position.
+    assert(!gci.IsConsoleLocked());
+
+    for (;;)
+    {
+        if (!_conptyCursorPositionMayBeWrong.load(std::memory_order::relaxed))
+        {
+            break;
+        }
+
+        // atomic_wait() returns false when the timeout expires.
+        // Technically we should decrement the timeout with each iteration,
+        // but I suspect infinite spurious wake-ups are a theoretical problem.
+        if (!til::atomic_wait(_conptyCursorPositionMayBeWrong, true, 500))
+        {
+            break;
+        }
+    }
+}
+
 // Routine Description:
 // - Given a rectangle containing screen buffer coordinates (character-level positioning, not pixel)
 //   This method will trim the rectangle to ensure it is within the buffer.

src/host/screenInfo.hpp

@@ -222,6 +222,11 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console
     [[nodiscard]] NTSTATUS ResizeWithReflow(const til::size coordnewScreenSize);
     [[nodiscard]] NTSTATUS ResizeTraditional(const til::size coordNewScreenSize);
 
+    bool ConptyCursorPositionMayBeWrong() const noexcept;
+    void SetConptyCursorPositionMayBeWrong() noexcept;
+    void ResetConptyCursorPositionMayBeWrong() noexcept;
+    void WaitForConptyCursorPositionToBeSynchronized() noexcept;
+
 private:
     SCREEN_INFORMATION(_In_ Microsoft::Console::Interactivity::IWindowMetrics* pMetrics,
                        const TextAttribute popupAttributes,
@@ -280,6 +285,7 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console
     til::CoordType _virtualBottom;
 
     std::optional<til::size> _deferredPtyResize{ std::nullopt };
+    std::atomic<bool> _conptyCursorPositionMayBeWrong = false;
 
     static void _handleDeferredResize(SCREEN_INFORMATION& siMain);
 

src/server/ApiDispatchers.cpp

@@ -584,6 +584,12 @@ constexpr T saturate(auto val)
     SCREEN_INFORMATION* pObj;
     RETURN_IF_FAILED(pObjectHandle->GetScreenBuffer(GENERIC_READ, &pObj));
 
+    // See ConptyCursorPositionMayBeWrong() for details.
+    if (pObj->ConptyCursorPositionMayBeWrong())
+    {
+        pObj->WaitForConptyCursorPositionToBeSynchronized();
+    }
+
     m->_pApiRoutines->GetConsoleScreenBufferInfoExImpl(*pObj, ex);
 
     a->FullscreenSupported = !!ex.bFullscreenSupported;