Repository: microsoft/terminal (language: cpp)

Issue:
Make double-click at the edge select across the edge
<!-- 
🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨

I ACKNOWLEDGE THE FOLLOWING BEFORE PROCEEDING:
1. If I delete this entire template and go my own path, the core team may close my issue without further explanation or engagement.
2. If I list multiple bugs/concerns in this one issue, the core team may close my issue without further explanation or engagement.
3. If I write an issue that has many duplicates, the core team may close my issue without further explanation or engagement (and without necessarily spending time to find the exact duplicate ID number).
4. If I leave the title incomplete when filing the issue, the core team may close my issue without further explanation or engagement.
5. If I file something completely blank in the body, the core team may close my issue without further explanation or engagement.

All good? Then proceed!
-->

<!--
This bug tracker is monitored by Windows Terminal development team and other technical folks.

**Important: When reporting BSODs or security issues, DO NOT attach memory dumps, logs, or traces to Github issues**.
Instead, send dumps/traces to secure@microsoft.com, referencing this GitHub issue.

If this is an application crash, please also provide a Feedback Hub submission link so we can find your diagnostic data on the backend. Use the category "Apps > Windows Terminal (Preview)" and choose "Share My Feedback" after submission to get the link.

Please use this form and describe your issue, concisely but precisely, with as much detail as possible.

-->

# Environment

Windows build number: 19041.1
Windows Terminal version (if applicable): 0.7.3451.0

# Steps to reproduce

<!-- A description of how to trigger this bug. -->
- You may need to configure `wordDelimiters` to equal `" \\()\"',;<>~!@#$^*|+[]{}~│"`
- Open Windows Terminal at default window width, using Ubuntu
- Run `echo 'really long string of text, really particularly long indeed, typing typing typing typing, also this url: https://example.com'`
- Double click on the URL

# Expected behavior
 
<!-- A description of what you're expecting, possibly containing screenshots or reference material. -->
The entirety of the URL should be selected.

# Actual behavior

<!-- What's actually happening? -->
Depending on whether you double click on the URL before or after the line wrap, either just `https://example` or `.com` is selected.



Discussion / hints:
This is a nontrivial amount of work because Terminal doesn't exactly know when a word was wrapped. I think we have another task, which would be a prerequisite of this, lying around to fix pty wrapping. Marked appropriately.
This is especially nasty when some long file path is wrapped, and that's exactly when you want it be selected easily
> **Note**
> ## Walkthrough
 
I'd start by looking at `TextBuffer::_GetWordEndForSelection` in `textBuffer.cpp`. That's the code that _should_ ultimately handle double-click selection. Looks to me like that `while` loop only iterates over a single row (the `result.x < bufferSize.RightInclusive()` check). I bet there's a better way to not do that (but also not explode if the user double clicks a word that runs right up to the end of the row in the _last_ row of a totally full buffer). 

Probably possible to write tests in `TextBufferTests.cpp`, but shockingly, there aren't any selection tests in there..?
This is important.

Cygwin does it, why not terminal?

Thanks.
+1 for multiline selection

extremely important.

Fix patch:
diff --git a/.github/actions/spelling/expect/04cdb9b77d6827c0202f51acd4205b017015bfff.txt b/.github/actions/spelling/expect/04cdb9b77d6827c0202f51acd4205b017015bfff.txt
new file mode 100644
index 00000000000..f117f5081da
--- /dev/null
+++ b/.github/actions/spelling/expect/04cdb9b77d6827c0202f51acd4205b017015bfff.txt
@@ -0,0 +1,5 @@
+EOB
+swrapped
+wordi
+wordiswrapped
+wrappe
diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp
index 52a5c6e3b72..528d8fbf4d3 100644
--- a/src/buffer/out/textBuffer.cpp
+++ b/src/buffer/out/textBuffer.cpp
@@ -1329,36 +1329,32 @@ til::point TextBuffer::_GetWordStartForAccessibility(const til::point target, co
 {
     auto result = target;
     const auto bufferSize = GetSize();
-    auto stayAtOrigin = false;
 
     // ignore left boundary. Continue until readable text found
     while (_GetDelimiterClassAt(result, wordDelimiters) != DelimiterClass::RegularChar)
     {
-        if (!bufferSize.DecrementInBounds(result))
+        if (result == bufferSize.Origin())
         {
-            // first char in buffer is a DelimiterChar or ControlChar
-            // we can't move any further back
-            stayAtOrigin = true;
-            break;
+            //looped around and hit origin (no word between origin and target)
+            return result;
         }
+        bufferSize.DecrementInBounds(result);
     }
 
     // make sure we expand to the left boundary or the beginning of the word
     while (_GetDelimiterClassAt(result, wordDelimiters) == DelimiterClass::RegularChar)
     {
-        if (!bufferSize.DecrementInBounds(result))
+        if (result == bufferSize.Origin())
         {
             // first char in buffer is a RegularChar
             // we can't move any further back
-            break;
+            return result;
         }
+        bufferSize.DecrementInBounds(result);
     }
 
-    // move off of delimiter and onto word start
-    if (!stayAtOrigin && _GetDelimiterClassAt(result, wordDelimiters) != DelimiterClass::RegularChar)
-    {
-        bufferSize.IncrementInBounds(result);
-    }
+    // move off of delimiter
+    bufferSize.IncrementInBounds(result);
 
     return result;
 }
@@ -1376,10 +1372,16 @@ til::point TextBuffer::_GetWordStartForSelection(const til::point target, const
     const auto bufferSize = GetSize();
 
     const auto initialDelimiter = _GetDelimiterClassAt(result, wordDelimiters);
+    const bool isControlChar = initialDelimiter == DelimiterClass::ControlChar;
 
     // expand left until we hit the left boundary or a different delimiter class
-    while (result.x > bufferSize.Left() && (_GetDelimiterClassAt(result, wordDelimiters) == initialDelimiter))
+    while (result != bufferSize.Origin() && _GetDelimiterClassAt(result, wordDelimiters) == initialDelimiter)
     {
+        //prevent selection wrapping on whitespace selection
+        if (isControlChar && result.x == bufferSize.Left())
+        {
+            break;
+        }
         bufferSize.DecrementInBounds(result);
     }
 
@@ -1457,25 +1459,21 @@ til::point TextBuffer::_GetWordEndForAccessibility(const til::point target, cons
     }
     else
     {
-        auto iter{ GetCellDataAt(result, bufferSize) };
-        while (iter && iter.Pos() != limit && _GetDelimiterClassAt(iter.Pos(), wordDelimiters) == DelimiterClass::RegularChar)
+        while (result != limit && result != bufferSize.BottomRightInclusive() && _GetDelimiterClassAt(result, wordDelimiters) == DelimiterClass::RegularChar)
         {
             // Iterate through readable text
-            ++iter;
+            bufferSize.IncrementInBounds(result);
         }
 
-        while (iter && iter.Pos() != limit && _GetDelimiterClassAt(iter.Pos(), wordDelimiters) != DelimiterClass::RegularChar)
+        while (result != limit && result != bufferSize.BottomRightInclusive() && _GetDelimiterClassAt(result, wordDelimiters) != DelimiterClass::RegularChar)
         {
             // expand to the beginning of the NEXT word
-            ++iter;
+            bufferSize.IncrementInBounds(result);
         }
 
-        result = iter.Pos();
-
-        // Special case: we tried to move one past the end of the buffer,
-        // but iter prevented that (because that pos doesn't exist).
+        // Special case: we tried to move one past the end of the buffer
         // Manually increment onto the EndExclusive point.
-        if (!iter)
+        if (result == bufferSize.BottomRightInclusive())
         {
             bufferSize.IncrementInBounds(result, true);
         }
@@ -1495,19 +1493,18 @@ til::point TextBuffer::_GetWordEndForSelection(const til::point target, const st
 {
     const auto bufferSize = GetSize();
 
-    // can't expand right
-    if (target.x == bufferSize.RightInclusive())
-    {
-        return target;
-    }
-
     auto result = target;
     const auto initialDelimiter = _GetDelimiterClassAt(result, wordDelimiters);
+    const bool isControlChar = initialDelimiter == DelimiterClass::ControlChar;
 
-    // expand right until we hit the right boundary or a different delimiter class
-    while (result.x < bufferSize.RightInclusive() && (_GetDelimiterClassAt(result, wordDelimiters) == initialDelimiter))
+    // expand right until we hit the right boundary as a ControlChar or a different delimiter class
+    while (result != bufferSize.BottomRightInclusive() && _GetDelimiterClassAt(result, wordDelimiters) == initialDelimiter)
     {
-        bufferSize.IncrementInBounds(result);
+        if (isControlChar && result.x == bufferSize.RightInclusive())
+        {
+            break;
+        }
+        bufferSize.IncrementInBoundsCircular(result);
     }
 
     if (_GetDelimiterClassAt(result, wordDelimiters) != initialDelimiter)
diff --git a/src/host/ut_host/TextBufferTests.cpp b/src/host/ut_host/TextBufferTests.cpp
index d1c7395635f..9b330e91d86 100644
--- a/src/host/ut_host/TextBufferTests.cpp
+++ b/src/host/ut_host/TextBufferTests.cpp
@@ -2118,10 +2118,11 @@ void TextBufferTests::TestAppendRTFText()
 void TextBufferTests::WriteLinesToBuffer(const std::vector<std::wstring>& text, TextBuffer& buffer)
 {
     const auto bufferSize = buffer.GetSize();
-
+    int rowsWrapped{};
     for (size_t row = 0; row < text.size(); ++row)
     {
         auto line = text[row];
+
         if (!line.empty())
         {
             // TODO GH#780: writing up to (but not past) the end of the line
@@ -2133,7 +2134,12 @@ void TextBufferTests::WriteLinesToBuffer(const std::vector<std::wstring>& text,
             }
 
             OutputCellIterator iter{ line };
-            buffer.Write(iter, { 0, gsl::narrow<til::CoordType>(row) }, wrap);
+            buffer.Write(iter, { 0, gsl::narrow<til::CoordType>(row + rowsWrapped) }, wrap);
+            //prevent bug that overwrites wrapped rows
+            if (line.size() > static_cast<size_t>(bufferSize.RightExclusive()))
+            {
+                rowsWrapped += static_cast<int>(line.size()) / bufferSize.RightExclusive();
+            }
         }
     }
 }
@@ -2245,6 +2251,88 @@ void TextBufferTests::GetWordBoundaries()
         const auto expected = accessibilityMode ? test.expected.accessibilityModeEnabled : test.expected.accessibilityModeDisabled;
         VERIFY_ARE_EQUAL(expected, result);
     }
+
+    _buffer->Reset();
+    _buffer->ResizeTraditional({ 10, 5 });
+    const std::vector<std::wstring> secondText = { L"this wordiswrapped",
+                                                   L"spaces        wrapped reachEOB" };
+    //Buffer looks like:
+    //  this wordi
+    //  swrapped
+    //  spaces
+    //      wrappe
+    //  d reachEOB
+    WriteLinesToBuffer(secondText, *_buffer);
+    testData = {
+        { { 0, 0 }, { { 0, 0 }, { 0, 0 } } },
+        { { 1, 0 }, { { 0, 0 }, { 0, 0 } } },
+        { { 4, 0 }, { { 4, 0 }, { 0, 0 } } },
+        { { 5, 0 }, { { 5, 0 }, { 5, 0 } } },
+        { { 7, 0 }, { { 5, 0 }, { 5, 0 } } },
+
+        { { 4, 1 }, { { 5, 0 }, { 5, 0 } } },
+        { { 7, 1 }, { { 5, 0 }, { 5, 0 } } },
+        { { 9, 1 }, { { 8, 1 }, { 5, 0 } } },
+
+        { { 0, 2 }, { { 0, 2 }, { 0, 2 } } },
+        { { 7, 2 }, { { 6, 2 }, { 0, 2 } } },
+
+        { { 1, 3 }, { { 0, 3 }, { 0, 2 } } },
+        { { 4, 3 }, { { 4, 3 }, { 4, 3 } } },
+        { { 8, 3 }, { { 4, 3 }, { 4, 3 } } },
+
+        { { 0, 4 }, { { 4, 3 }, { 4, 3 } } },
+        { { 1, 4 }, { { 1, 4 }, { 4, 3 } } },
+        { { 9, 4 }, { { 2, 4 }, { 2, 4 } } },
+    };
+    for (const auto& test : testData)
+    {
+        Log::Comment(NoThrowString().Format(L"Testing til::point (%hd, %hd)", test.startPos.x, test.startPos.y));
+        const auto result = _buffer->GetWordStart(test.startPos, delimiters, accessibilityMode);
+        const auto expected = accessibilityMode ? test.expected.accessibilityModeEnabled : test.expected.accessibilityModeDisabled;
+        VERIFY_ARE_EQUAL(expected, result);
+    }
+
+    //GetWordEnd for Wrapping Text
+    //Buffer looks like:
+    //  this wordi
+    //  swrapped
+    //  spaces
+    //      wrappe
+    //  d reachEOB
+    testData = {
+        // tests for first line of text
+        { { 0, 0 }, { { 3, 0 }, { 5, 0 } } },
+        { { 1, 0 }, { { 3, 0 }, { 5, 0 } } },
+        { { 4, 0 }, { { 4, 0 }, { 5, 0 } } },
+        { { 5, 0 }, { { 7, 1 }, { 0, 2 } } },
+        { { 7, 0 }, { { 7, 1 }, { 0, 2 } } },
+
+        { { 4, 1 }, { { 7, 1 }, { 0, 2 } } },
+        { { 7, 1 }, { { 7, 1 }, { 0, 2 } } },
+        { { 9, 1 }, { { 9, 1 }, { 0, 2 } } },
+
+        { { 0, 2 }, { { 5, 2 }, { 4, 3 } } },
+        { { 7, 2 }, { { 9, 2 }, { 4, 3 } } },
+
+        { { 1, 3 }, { { 3, 3 }, { 4, 3 } } },
+        { { 4, 3 }, { { 0, 4 }, { 2, 4 } } },
+        { { 8, 3 }, { { 0, 4 }, { 2, 4 } } },
+
+        { { 0, 4 }, { { 0, 4 }, { 2, 4 } } },
+        { { 1, 4 }, { { 1, 4 }, { 2, 4 } } },
+        { { 4, 4 }, { { 9, 4 }, { 0, 5 } } },
+        { { 9, 4 }, { { 9, 4 }, { 0, 5 } } },
+    };
+    // clang-format on
+
+    for (const auto& test : testData)
+    {
+        Log::Comment(NoThrowString().Format(L"TestEnd til::point (%hd, %hd)", test.startPos.x, test.startPos.y));
+        auto result = _buffer->GetWordEnd(test.startPos, delimiters, accessibilityMode);
+        const auto expected = accessibilityMode ? test.expected.accessibilityModeEnabled : test.expected.accessibilityModeDisabled;
+        VERIFY_ARE_EQUAL(expected, result);
+    }
 }
 
 void TextBufferTests::MoveByWord()
@@ -2571,9 +2659,7 @@ void TextBufferTests::GetText()
 
         // Setup: Write lines of text to the buffer
         const std::vector<std::wstring> bufferText = { L"1234567",
-                                                       L"",
-                                                       L"  345",
-                                                       L"123    ",
+                                                       L"  345123    ",
                                                        L"" };
         WriteLinesToBuffer(bufferText, *_buffer);
         // buffer should look like this:
@@ -2653,7 +2739,7 @@ void TextBufferTests::GetText()
                     Log::Comment(L"Standard Copy to Clipboard");
                     expectedText += L"12345";
                     expectedText += L"67\r\n";
-                    expectedText += L"  345\r\n";
+                    expectedText += L"  345";
                     expectedText += L"123  \r\n";
                 }
                 else
@@ -2661,7 +2747,7 @@ void TextBufferTests::GetText()
                     Log::Comment(L"UI Automation");
                     expectedText += L"12345";
                     expectedText += L"67   \r\n";
-                    expectedText += L"  345\r\n";
+                    expectedText += L"  345";
                     expectedText += L"123  ";
                     expectedText += L"     \r\n";
                     expectedText += L"     ";


Test patch: