Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
1 commit merged into from
Dec 14, 2020
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions src/buffer/out/textBuffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions src/buffer/out/textBuffer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,11 @@ class TextBuffer final
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,
Expand Down
11 changes: 8 additions & 3 deletions src/cascadia/TerminalCore/TerminalSelection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
109 changes: 80 additions & 29 deletions src/host/ut_host/TextBufferTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" ";
}
}
}

Expand Down