starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-04-19 04:31:10 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Fix Copy to Clipboard to preserve visual structure of block selection (#8579)

Browse Source 
There are two issue with copy to clipboard when block is selected:
* We don't add new lines for lines that were wrapped
* We remove trailing whitespaces which is not intuitive in block selection.

Fixed the copy logic to always add newlines and not to remove
whitespaces when block is selected.

Even if shift is pressed!

## Detailed Description of the Pull Request / Additional comments
* Added optional parameter to `TextBuffer::GetText` 
that allows to apply formatting (includeCRLF / trimming) 
to lines that were wrapped
* Changed `Terminal::RetrieveSelectedTextFromBuffer` 
to apply the following parameters when block is selected:
  * includeCRLF = true
  * trimTrailingWhitespaces = false
  * apply the formatting above to all rows, including the ones 
that were wrapped 

## Validation Steps Performed
* Manual tests for both block and standard selection
* Copy with both right-click and command
* Added UT

Closes #6740
This commit is contained in:
Don-Vito
2020-12-15 01:32:44 +02:00
committed by GitHub
parent cae0f9a718
commit a1f42e87a8
4 changed files with 98 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
src/buffer/out/textBuffer.cpp
View File

@@ -1512,12 +1512,14 @@ void TextBuffer::_ExpandTextRow(SMALL_RECT& textRow) const
// - trimTrailingWhitespace - remove the trailing whitespace at the end of each line
// - textRects - the rectangular regions from which the data will be extracted from the buffer (i.e.: selection rects)
// - GetAttributeColors - function used to map TextAttribute to RGB COLORREFs. If null, only extract the text.
// - formatWrappedRows - if set we will apply formatting (CRLF inclusion and whitespace trimming) on wrapped rows
// Return Value:
// - The text, background color, and foreground color data of the selected region of the text buffer.
const TextBuffer::TextAndColor TextBuffer::GetText(const bool includeCRLF,
const bool trimTrailingWhitespace,
const std::vector<SMALL_RECT>& selectionRects,
std::function<std::pair<COLORREF, COLORREF>(const TextAttribute&)> GetAttributeColors) const
std::function<std::pair<COLORREF, COLORREF>(const TextAttribute&)> GetAttributeColors,
const bool formatWrappedRows) const
{
TextAndColor data;
const bool copyTextColor = GetAttributeColors != nullptr;
@@ -1579,12 +1581,12 @@ const TextBuffer::TextAndColor TextBuffer::GetText(const bool includeCRLF,
it++;
}
const bool forcedWrap = GetRowByOffset(iRow).GetCharRow().WasWrapForced();
// We apply formatting to rows if the row was NOT wrapped or formatting of wrapped rows is allowed
const bool shouldFormatRow = formatWrappedRows || !GetRowByOffset(iRow).GetCharRow().WasWrapForced();
if (trimTrailingWhitespace)
{
// if the row was NOT wrapped...
if (!forcedWrap)
if (shouldFormatRow)
{
// remove the spaces at the end (aka trim the trailing whitespace)
while (!selectionText.empty() && selectionText.back() == UNICODE_SPACE)
@@ -1603,8 +1605,7 @@ const TextBuffer::TextAndColor TextBuffer::GetText(const bool includeCRLF,
// a.k.a if we're earlier than the bottom, then apply CR/LF.
if (includeCRLF && i < selectionRects.size() - 1)
{
// if the row was NOT wrapped...
if (!forcedWrap)
if (shouldFormatRow)
{
// then we can assume a CR/LF is proper
selectionText.push_back(UNICODE_CARRIAGERETURN);

5
src/buffer/out/textBuffer.hpp
View File

@@ -156,10 +156,11 @@ public:
std::vector<std::vector<COLORREF>> BkAttr;
};
const TextAndColor GetText(const bool lineSelection,
const TextAndColor GetText(const bool includeCRLF,
const bool trimTrailingWhitespace,
const std::vector<SMALL_RECT>& textRects,
std::function<std::pair<COLORREF, COLORREF>(const TextAttribute&)> GetAttributeColors = nullptr) const;
std::function<std::pair<COLORREF, COLORREF>(const TextAttribute&)> GetAttributeColors = nullptr,
const bool formatWrappedRows = false) const;
static std::string GenHTML(const TextAndColor& rows,
const int fontHeightPoints,

11
src/cascadia/TerminalCore/TerminalSelection.cpp
View File

@@ -255,10 +255,15 @@ const TextBuffer::TextAndColor Terminal::RetrieveSelectedTextFromBuffer(bool sin
const auto GetAttributeColors = std::bind(&Terminal::GetAttributeColors, this, std::placeholders::_1);
return _buffer->GetText(!singleLine,
!singleLine,
// GH#6740: Block selection should preserve the text block as is:
// - No trailing white-spaces should be removed.
// - CRLFs need to be added - so the lines structure is preserved
// - We should apply formatting above to wrapped rows as well (newline should be added).
return _buffer->GetText(!singleLine || _blockSelection,
!singleLine && !_blockSelection,
selectionRects,
GetAttributeColors);
GetAttributeColors,
_blockSelection);
}
// Method Description:

109
src/host/ut_host/TextBufferTests.cpp
View File

@@ -2471,56 +2471,107 @@ void TextBufferTests::GetText()
// |_____|
// simulate a selection from origin to {4,5}
const auto textRects = _buffer->GetTextRects({ 0, 0 }, { 4, 5 });
const auto textRects = _buffer->GetTextRects({ 0, 0 }, { 4, 5 }, blockSelection);
std::wstring result = L"";
const auto textData = _buffer->GetText(includeCRLF, trimTrailingWhitespace, textRects).text;
const auto formatWrappedRows = blockSelection;
const auto textData = _buffer->GetText(includeCRLF, trimTrailingWhitespace, textRects, nullptr, formatWrappedRows).text;
for (auto& text : textData)
{
result += text;
}
std::wstring expectedText = L"";
if (includeCRLF)
if (formatWrappedRows)
{
if (trimTrailingWhitespace)
if (includeCRLF)
{
Log::Comment(L"Standard Copy to Clipboard");
expectedText += L"12345";
expectedText += L"67\r\n";
expectedText += L" 345\r\n";
expectedText += L"123 \r\n";
if (trimTrailingWhitespace)
{
Log::Comment(L"UNDEFINED");
expectedText += L"12345\r\n";
expectedText += L"67\r\n";
expectedText += L" 345\r\n";
expectedText += L"123\r\n";
expectedText += L"\r\n";
}
else
{
Log::Comment(L"Copy block selection to Clipboard");
expectedText += L"12345\r\n";
expectedText += L"67 \r\n";
expectedText += L" 345\r\n";
expectedText += L"123 \r\n";
expectedText += L" \r\n";
expectedText += L" ";
}
}
else
{
Log::Comment(L"UI Automation");
expectedText += L"12345";
expectedText += L"67 \r\n";
expectedText += L" 345\r\n";
expectedText += L"123 ";
expectedText += L" \r\n";
expectedText += L" ";
if (trimTrailingWhitespace)
{
Log::Comment(L"UNDEFINED");
expectedText += L"12345";
expectedText += L"67";
expectedText += L" 345";
expectedText += L"123";
}
else
{
Log::Comment(L"UNDEFINED");
expectedText += L"12345";
expectedText += L"67 ";
expectedText += L" 345";
expectedText += L"123 ";
expectedText += L" ";
expectedText += L" ";
}
}
}
else
{
if (trimTrailingWhitespace)
if (includeCRLF)
{
Log::Comment(L"UNDEFINED");
expectedText += L"12345";
expectedText += L"67";
expectedText += L" 345";
expectedText += L"123 ";
if (trimTrailingWhitespace)
{
Log::Comment(L"Standard Copy to Clipboard");
expectedText += L"12345";
expectedText += L"67\r\n";
expectedText += L" 345\r\n";
expectedText += L"123 \r\n";
}
else
{
Log::Comment(L"UI Automation");
expectedText += L"12345";
expectedText += L"67 \r\n";
expectedText += L" 345\r\n";
expectedText += L"123 ";
expectedText += L" \r\n";
expectedText += L" ";
}
}
else
{
Log::Comment(L"Shift+Copy to Clipboard");
expectedText += L"12345";
expectedText += L"67 ";
expectedText += L" 345";
expectedText += L"123 ";
expectedText += L" ";
expectedText += L" ";
if (trimTrailingWhitespace)
{
Log::Comment(L"UNDEFINED");
expectedText += L"12345";
expectedText += L"67";
expectedText += L" 345";
expectedText += L"123 ";
}
else
{
Log::Comment(L"Shift+Copy to Clipboard");
expectedText += L"12345";
expectedText += L"67 ";
expectedText += L" 345";
expectedText += L"123 ";
expectedText += L" ";
expectedText += L" ";
}
}
}