jprdonnelly/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-04-30 16:00:26 -04:00
Code Issues Projects Releases Wiki Activity

Fix redundant CR in formatted text copy (#4190)

Browse Source 
## Summary of the Pull Request

When `GenHTML` or `GenRTF` encountered an empty line, they assumed that `CR` is the last character of the row and wrote it, even though in general `CR` and `LF` just break the line and instead of them either `<BR>` in HTML or `\line` in RTF is written. Don't know how I missed that in #2038.

Another question is whether the `TextAndColor` structure which these methods receive and which is generated by `TextBuffer::GetTextForClipboard` should really contain `\r\n` at the end of each row. I think it'd be cleaner if it didn't esp. that afaik these last 2 characters don't have associated valid color information.

## References

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [X] Closes #4187
* [X] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed - there aren't any related tests, right?
* [ ] Requires documentation to be updated
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #4147 

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Copied various terminal states and verified the generated HTML.
This commit is contained in:
mcpiroman
2020-01-14 18:07:06 +01:00
committed by msftbot[bot]
parent bf86a961f0
commit 1ca29128d4
1 changed files with 46 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1243,26 +1243,6 @@ std::string TextBuffer::GenHTML(const TextAndColor& rows, const int fontHeightPo
for (size_t col = 0; col < rows.text.at(row).length(); col++)
{
// do not include \r nor \n as they don't have attributes
// and are not HTML friendly. For line break use '<BR>' instead.
const bool isLastCharInRow =
col == rows.text.at(row).length() - 1 ||
rows.text.at(row).at(col + 1) == '\r' ||
rows.text.at(row).at(col + 1) == '\n';
bool colorChanged = false;
if (!fgColor.has_value() || rows.FgAttr.at(row).at(col) != fgColor.value())
{
fgColor = rows.FgAttr.at(row).at(col);
colorChanged = true;
}
if (!bkColor.has_value() || rows.BkAttr.at(row).at(col) != bkColor.value())
{
bkColor = rows.BkAttr.at(row).at(col);
colorChanged = true;
}
const auto writeAccumulatedChars = [&](bool includeCurrent) {
if (col >= startOffset)
{
@@ -1289,6 +1269,27 @@ std::string TextBuffer::GenHTML(const TextAndColor& rows, const int fontHeightPo
}
};
if (rows.text.at(row).at(col) == '\r' || rows.text.at(row).at(col) == '\n')
{
// do not include \r nor \n as they don't have color attributes
// and are not HTML friendly. For line break use '<BR>' instead.
writeAccumulatedChars(false);
break;
}
bool colorChanged = false;
if (!fgColor.has_value() || rows.FgAttr.at(row).at(col) != fgColor.value())
{
fgColor = rows.FgAttr.at(row).at(col);
colorChanged = true;
}
if (!bkColor.has_value() || rows.BkAttr.at(row).at(col) != bkColor.value())
{
bkColor = rows.BkAttr.at(row).at(col);
colorChanged = true;
}
if (colorChanged)
{
writeAccumulatedChars(false);
@@ -1310,10 +1311,10 @@ std::string TextBuffer::GenHTML(const TextAndColor& rows, const int fontHeightPo
hasWrittenAnyText = true;
if (isLastCharInRow)
// if this is the last character in the row, flush the whole row
if (col == rows.text.at(row).length() - 1)
{
writeAccumulatedChars(true);
break;
}
}
}
@@ -1430,24 +1431,6 @@ std::string TextBuffer::GenRTF(const TextAndColor& rows, const int fontHeightPoi
for (size_t col = 0; col < rows.text.at(row).length(); ++col)
{
const bool isLastCharInRow =
col == rows.text.at(row).length() - 1 ||
rows.text.at(row).at(col + 1) == '\r' ||
rows.text.at(row).at(col + 1) == '\n';
bool colorChanged = false;
if (!fgColor.has_value() || rows.FgAttr.at(row).at(col) != fgColor.value())
{
fgColor = rows.FgAttr.at(row).at(col);
colorChanged = true;
}
if (!bkColor.has_value() || rows.BkAttr.at(row).at(col) != bkColor.value())
{
bkColor = rows.BkAttr.at(row).at(col);
colorChanged = true;
}
const auto writeAccumulatedChars = [&](bool includeCurrent) {
if (col >= startOffset)
{
@@ -1470,6 +1453,27 @@ std::string TextBuffer::GenRTF(const TextAndColor& rows, const int fontHeightPoi
}
};
if (rows.text.at(row).at(col) == '\r' || rows.text.at(row).at(col) == '\n')
{
// do not include \r nor \n as they don't have color attributes.
// For line break use \line instead.
writeAccumulatedChars(false);
break;
}
bool colorChanged = false;
if (!fgColor.has_value() || rows.FgAttr.at(row).at(col) != fgColor.value())
{
fgColor = rows.FgAttr.at(row).at(col);
colorChanged = true;
}
if (!bkColor.has_value() || rows.BkAttr.at(row).at(col) != bkColor.value())
{
bkColor = rows.BkAttr.at(row).at(col);
colorChanged = true;
}
if (colorChanged)
{
writeAccumulatedChars(false);
@@ -1513,10 +1517,10 @@ std::string TextBuffer::GenRTF(const TextAndColor& rows, const int fontHeightPoi
<< " ";
}
if (isLastCharInRow)
// if this is the last character in the row, flush the whole row
if (col == rows.text.at(row).length() - 1)
{
writeAccumulatedChars(true);
break;
}
}
}