Merge branch 'main' of https://github.com/microsoft/terminal into dev…

…/pabhoj/action_refactor

doc/cascadia/profiles.schema.json

@@ -349,7 +349,7 @@
           "description": "Sets the DWrite font features for the given font. For example, { \"ss01\": 1, \"liga\":0 } will enable ss01 and disable ligatures.",
           "type": "object",
           "patternProperties": {
-            "^(([A-Za-z0-9]){4})$": {
+            "^[\\x20-\\x7E]{4}$": {
               "type": "integer"
             }
           },
@@ -359,7 +359,7 @@
           "description": "Sets the DWrite font axes for the given font. For example, { \"wght\": 200 } will set the font weight to 200.",
           "type": "object",
           "patternProperties": {
-            "^([A-Za-z]{4})$": {
+            "^[\\x20-\\x7E]{4}$": {
               "type": "number"
             }
           },

src/buffer/out/search.cpp

@@ -8,29 +8,22 @@
 
 using namespace Microsoft::Console::Types;
 
-bool Search::ResetIfStale(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool reverse, bool caseInsensitive, std::vector<til::point_span>* prevResults)
+bool Search::IsStale(const Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool caseInsensitive) const noexcept
 {
-    const auto& textBuffer = renderData.GetTextBuffer();
-    const auto lastMutationId = textBuffer.GetLastMutationId();
-
-    if (_renderData == &renderData &&
-        _needle == needle &&
-        _caseInsensitive == caseInsensitive &&
-        _lastMutationId == lastMutationId)
-    {
-        _step = reverse ? -1 : 1;
-        return false;
-    }
+    return _renderData != &renderData ||
+           _needle != needle ||
+           _caseInsensitive != caseInsensitive ||
+           _lastMutationId != renderData.GetTextBuffer().GetLastMutationId();
+}
 
-    if (prevResults)
-    {
-        *prevResults = std::move(_results);
-    }
+bool Search::Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool caseInsensitive, bool reverse)
+{
+    const auto& textBuffer = renderData.GetTextBuffer();
 
     _renderData = &renderData;
     _needle = needle;
     _caseInsensitive = caseInsensitive;
-    _lastMutationId = lastMutationId;
+    _lastMutationId = textBuffer.GetLastMutationId();
 
     _results = textBuffer.SearchText(needle, caseInsensitive);
     _index = reverse ? gsl::narrow_cast<ptrdiff_t>(_results.size()) - 1 : 0;
@@ -98,8 +91,9 @@ void Search::MovePastPoint(const til::point anchor) noexcept
     _index = (index + count) % count;
 }
 
-void Search::FindNext() noexcept
+void Search::FindNext(bool reverse) noexcept
 {
+    _step = reverse ? -1 : 1;
     if (const auto count{ gsl::narrow_cast<ptrdiff_t>(_results.size()) })
     {
         _index = (_index + _step + count) % count;
@@ -141,6 +135,11 @@ const std::vector<til::point_span>& Search::Results() const noexcept
     return _results;
 }
 
+std::vector<til::point_span>&& Search::ExtractResults() noexcept
+{
+    return std::move(_results);
+}
+
 ptrdiff_t Search::CurrentMatch() const noexcept
 {
     return _index;

src/buffer/out/search.h

@@ -25,21 +25,19 @@ class Search final
 public:
     Search() = default;
 
-    bool ResetIfStale(Microsoft::Console::Render::IRenderData& renderData,
-                      const std::wstring_view& needle,
-                      bool reverse,
-                      bool caseInsensitive,
-                      std::vector<til::point_span>* prevResults = nullptr);
+    bool IsStale(const Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool caseInsensitive) const noexcept;
+    bool Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool caseInsensitive, bool reverse);
 
     void MoveToCurrentSelection();
     void MoveToPoint(til::point anchor) noexcept;
     void MovePastPoint(til::point anchor) noexcept;
-    void FindNext() noexcept;
+    void FindNext(bool reverse) noexcept;
 
     const til::point_span* GetCurrent() const noexcept;
     bool SelectCurrent() const;
 
     const std::vector<til::point_span>& Results() const noexcept;
+    std::vector<til::point_span>&& ExtractResults() noexcept;
     ptrdiff_t CurrentMatch() const noexcept;
 
 private:

src/cascadia/TerminalControl/ControlCore.cpp

@@ -959,26 +959,23 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         if (_renderEngine)
         {
-            std::unordered_map<std::wstring_view, uint32_t> featureMap;
-            if (const auto fontFeatures = _settings->FontFeatures())
-            {
-                featureMap.reserve(fontFeatures.Size());
-
-                for (const auto& [tag, param] : fontFeatures)
+            static constexpr auto cloneMap = [](const IFontFeatureMap& map) {
+                std::unordered_map<std::wstring_view, float> clone;
+                if (map)
                 {
-                    featureMap.emplace(tag, param);
+                    clone.reserve(map.Size());
+                    for (const auto& [tag, param] : map)
+                    {
+                        clone.emplace(tag, param);
+                    }
                 }
-            }
-            std::unordered_map<std::wstring_view, float> axesMap;
-            if (const auto fontAxes = _settings->FontAxes())
-            {
-                axesMap.reserve(fontAxes.Size());
+                return clone;
+            };
 
-                for (const auto& [axis, value] : fontAxes)
-                {
-                    axesMap.emplace(axis, value);
-                }
-            }
+            const auto fontFeatures = _settings->FontFeatures();
+            const auto fontAxes = _settings->FontAxes();
+            const auto featureMap = cloneMap(fontFeatures);
+            const auto axesMap = cloneMap(fontAxes);
 
             // TODO: MSFT:20895307 If the font doesn't exist, this doesn't
             //      actually fail. We need a way to gracefully fallback.
@@ -1654,30 +1651,41 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // - text: the text to search
     // - goForward: boolean that represents if the current search direction is forward
     // - caseSensitive: boolean that represents if the current search is case sensitive
+    // - resetOnly: If true, only Reset() will be called, if anything. FindNext() will never be called.
     // Return Value:
     // - <none>
-    SearchResults ControlCore::Search(const std::wstring_view& text, const bool goForward, const bool caseSensitive, const bool reset)
+    SearchResults ControlCore::Search(const std::wstring_view& text, const bool goForward, const bool caseSensitive, const bool resetOnly)
     {
         const auto lock = _terminal->LockForWriting();
+        const auto searchInvalidated = _searcher.IsStale(*_terminal.get(), text, !caseSensitive);
 
-        bool searchInvalidated = false;
-        std::vector<til::point_span> oldResults;
-        if (_searcher.ResetIfStale(*GetRenderData(), text, !goForward, !caseSensitive, &oldResults))
+        if (searchInvalidated || !resetOnly)
         {
-            searchInvalidated = true;
+            std::vector<til::point_span> oldResults;
 
-            _cachedSearchResultRows = {};
-            if (SnapSearchResultToSelection())
+            if (searchInvalidated)
             {
-                _searcher.MoveToCurrentSelection();
-                SnapSearchResultToSelection(false);
+                oldResults = _searcher.ExtractResults();
+                _searcher.Reset(*_terminal.get(), text, !caseSensitive, !goForward);
+
+                if (SnapSearchResultToSelection())
+                {
+                    _searcher.MoveToCurrentSelection();
+                    SnapSearchResultToSelection(false);
+                }
+
+                _terminal->SetSearchHighlights(_searcher.Results());
+            }
+            else
+            {
+                _searcher.FindNext(!goForward);
             }
 
-            _terminal->SetSearchHighlights(_searcher.Results());
-        }
-        else if (!reset)
-        {
-            _searcher.FindNext();
+            if (const auto idx = _searcher.CurrentMatch(); idx >= 0)
+            {
+                _terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx));
+            }
+            _renderer->TriggerSearchHighlight(oldResults);
         }
 
         int32_t totalMatches = 0;
@@ -1686,39 +1694,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         {
             totalMatches = gsl::narrow<int32_t>(_searcher.Results().size());
             currentMatch = gsl::narrow<int32_t>(idx);
-            _terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx));
         }
 
-        _renderer->TriggerSearchHighlight(oldResults);
-
         return {
             .TotalMatches = totalMatches,
             .CurrentMatch = currentMatch,
             .SearchInvalidated = searchInvalidated,
         };
     }
 
-    Windows::Foundation::Collections::IVector<int32_t> ControlCore::SearchResultRows()
+    const std::vector<til::point_span>& ControlCore::SearchResultRows() const noexcept
     {
-        if (!_cachedSearchResultRows)
-        {
-            auto results = std::vector<int32_t>();
-            auto lastRow = til::CoordTypeMin;
-
-            for (const auto& match : _searcher.Results())
-            {
-                const auto row{ match.start.y };
-                if (row != lastRow)
-                {
-                    results.push_back(row);
-                    lastRow = row;
-                }
-            }
-
-            _cachedSearchResultRows = winrt::single_threaded_vector<int32_t>(std::move(results));
-        }
-
-        return _cachedSearchResultRows;
+        return _searcher.Results();
     }
 
     void ControlCore::ClearSearch()
@@ -1728,7 +1715,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _terminal->SetSearchHighlightFocused({});
         _renderer->TriggerSearchHighlight(_searcher.Results());
         _searcher = {};
-        _cachedSearchResultRows = {};
     }
 
     // Method Description:
@@ -2625,12 +2611,27 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         CompletionsChanged.raise(*this, *args);
     }
+
+    // Select the region of text between [s.start, s.end), in buffer space
     void ControlCore::_selectSpan(til::point_span s)
     {
+        // s.end is an _exclusive_ point. We need an inclusive one. But
+        // decrement in bounds wants an inclusive one. If you pass an exclusive
+        // one, then it might assert at you for being out of bounds. So we also
+        // take care of the case that the end point is outside the viewport
+        // manually.
         const auto bufferSize{ _terminal->GetTextBuffer().GetSize() };
-        bufferSize.DecrementInBounds(s.end);
+        til::point inclusiveEnd = s.end;
+        if (s.end.x == bufferSize.Width())
+        {
+            inclusiveEnd = til::point{ std::max(0, s.end.x - 1), s.end.y };
+        }
+        else
+        {
+            bufferSize.DecrementInBounds(inclusiveEnd);
+        }
 
-        _terminal->SelectNewRegion(s.start, s.end);
+        _terminal->SelectNewRegion(s.start, inclusiveEnd);
         _renderer->TriggerSelection();
     }
 

src/cascadia/TerminalControl/ControlCore.h

@@ -220,12 +220,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void SetEndSelectionPoint(const til::point position);
 
         SearchResults Search(const std::wstring_view& text, bool goForward, bool caseSensitive, bool reset);
+        const std::vector<til::point_span>& SearchResultRows() const noexcept;
         void ClearSearch();
         void SnapSearchResultToSelection(bool snap) noexcept;
         bool SnapSearchResultToSelection() const noexcept;
 
-        Windows::Foundation::Collections::IVector<int32_t> SearchResultRows();
-
         void LeftClickOnTerminal(const til::point terminalPosition,
                                  const int numberOfClicks,
                                  const bool altEnabled,
@@ -353,8 +352,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         til::point _contextMenuBufferPosition{ 0, 0 };
 
-        Windows::Foundation::Collections::IVector<int32_t> _cachedSearchResultRows{ nullptr };
-
         void _setupDispatcherAndCallbacks();
 
         bool _setFontSizeUnderLock(float fontSize);

src/cascadia/TerminalControl/ControlCore.idl

@@ -136,7 +136,6 @@ namespace Microsoft.Terminal.Control
 
         SearchResults Search(String text, Boolean goForward, Boolean caseSensitive, Boolean reset);
         void ClearSearch();
-        IVector<Int32> SearchResultRows { get; };
         Boolean SnapSearchResultToSelection;
 
         Microsoft.Terminal.Core.Color ForegroundColor { get; };

src/cascadia/TerminalControl/ControlSettings.h

@@ -10,7 +10,7 @@ Licensed under the MIT license.
 #include <conattrs.hpp>
 #include "ControlAppearance.h"
 
-using IFontFeatureMap = winrt::Windows::Foundation::Collections::IMap<winrt::hstring, uint32_t>;
+using IFontFeatureMap = winrt::Windows::Foundation::Collections::IMap<winrt::hstring, float>;
 using IFontAxesMap = winrt::Windows::Foundation::Collections::IMap<winrt::hstring, float>;
 
 namespace winrt::Microsoft::Terminal::Control::implementation

src/cascadia/TerminalControl/IControlSettings.idl

@@ -40,7 +40,7 @@ namespace Microsoft.Terminal.Control
         Single FontSize { get; };
         Windows.UI.Text.FontWeight FontWeight { get; };
         String Padding { get; };
-        Windows.Foundation.Collections.IMap<String, UInt32> FontFeatures { get; };
+        Windows.Foundation.Collections.IMap<String, Single> FontFeatures { get; };
         Windows.Foundation.Collections.IMap<String, Single> FontAxes { get; };
         Boolean EnableBuiltinGlyphs { get; };
         Boolean EnableColorGlyphs { get; };

src/cascadia/TerminalControl/TermControl.cpp

@@ -495,14 +495,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
             if (_searchBox && _searchBox->Visibility() == Visibility::Visible)
             {
-                if (const auto searchMatches = _core.SearchResultRows())
-                {
-                    const til::color color{ _core.ForegroundColor() };
-                    const auto rightAlignedOffset = (scrollBarWidthInPx - pipWidth) * sizeof(til::color);
+                const auto core = winrt::get_self<ControlCore>(_core);
+                const auto& searchMatches = core->SearchResultRows();
+                const auto color = core->ForegroundColor();
+                const auto rightAlignedOffset = (scrollBarWidthInPx - pipWidth) * sizeof(til::color);
+                til::CoordType lastRow = til::CoordTypeMin;
 
-                    for (const auto row : searchMatches)
+                for (const auto& span : searchMatches)
+                {
+                    if (lastRow != span.start.y)
                     {
-                        const auto base = dataAt(row) + rightAlignedOffset;
+                        lastRow = span.start.y;
+                        const auto base = dataAt(lastRow) + rightAlignedOffset;
                         drawPip(base, color);
                     }
                 }

src/cascadia/TerminalCore/terminalrenderdata.cpp

@@ -199,7 +199,7 @@ til::CoordType Terminal::_ScrollToPoints(const til::point coordStart, const til:
         _NotifyScrollEvent();
     }
 
-    return _scrollOffset;
+    return _VisibleStartIndex();
 }
 
 void Terminal::SelectNewRegion(const til::point coordStart, const til::point coordEnd)