Skip to content

Commit

Permalink
Merged PR 3896152: Merge selected github changes up to dd2fbef
Browse files Browse the repository at this point in the history
- Fix snapping to the cursor in "Terminal Scrolling" mode (GH-2705)

fixes GH-1222

  PSReadline calls SetConsoleCursorPosition on each character. We try to snap.

  However, we'd only ever do this with the visible viewport, the viewport
  defined by `SCREEN_INFORMATION::_viewport`. When there's a virtual viewport in
  Terminal Scrolling mode, we actually need to snap the virtual viewport, so
  that this behavior looks more regular.

(cherry picked from commit 6f4b98a)
- Passthrough BEL in conpty (GH-2990)

fixes GH-2952.

(cherry picked from commit 6831120)
- make filling chars (and, thus, erase line/char) unset wrap (GH-2831)

EraseInLine calls `FillConsoleOutputCharacterW()`. In filling the row with
chars, we were setting the wrap flag. We need to specifically not do this on
ANY _FILL_ operation. Now a fill operation UNSETS the wrap flag if we fill to
the end of the line.

Originally, we had a boolean `setWrap` that would mean...
- **true**: if writing to the end of the row, SET the wrap value to true
- **false**: if writing to the end of the row, DON'T CHANGE the wrap value

,- current wrap value
| ,- fill last cell?
| | ,- new wrap value
| | | ,- comments
|-|-|-|
|1|0|0| THIS CASE WAS UNHANDLED

To handle that special case (1-0-0), we need to UNSET the wrap. So now, we have
~setWrap~ `wrap` mean the following:
- **true**: if writing to the end of the row, SET the wrap value to TRUE
- **false**: if writing to the end of the row, SET the wrap value to FALSE
- **nullopt**: leave the wrap value as it is

Closes GH-1126

(cherry picked from commit 4dd9f9c)
- When reverse indexing, preserve RGB/256 attributes (GH-2987)

(cherry picked from commit 847d6b5)
- do not allow CUU and CUD to move "across" margins. (GH-2996)

If we're moving the cursor up, its vertical movement should be stopped
at the top margin.
Similarly, this applies to moving down and the bottom margin.

Fixes GH-2929.

(cherry picked from commit 0ce08af)
- Prevent the v1 propsheet from zeroing colors, causing black text on black background. (GH-2651)

  fixes GH-2319

(cherry picked from commit b97db63)
- Render the cursor state to VT (GH-2829)

(cherry picked from commit a9f3849)
- Don't put NUL in the buffer in VT mode (GH-3015)

(cherry picked from commit a82d6b8)
- Return to ground when we flush the last char (GH-2823)

The InputStateMachineEngine was incorrectly not returning to the ground
state after flushing the last sequence. That means that something like
alt+backspace would leave us in the Escape state, not the ground state.
This makes sure we return to ground.

Additionally removes the "Parser.UnitTests-common.vcxproj" file, which
was originally used for a theoretical time when we only open-sourced the
parser. It's unnecessary now, and we can get rid of it.

Also includes a small patch to bcz.cmd, to make sure bx works with
projects with a space in their name.

(cherry picked from commit 53c81a0)
- Add support for passing through extended text attributes, like… (GH-2917)
 ...

Related work items: #23678919, #23678920, #23731910, #23731911, #23731912, #23731913, #23731914, #23731915, #23731916, #23731917, #23731918
  • Loading branch information
DHowett committed Oct 17, 2019
2 parents 476c8c1 + 361e773 commit 62f5299
Show file tree
Hide file tree
Showing 60 changed files with 2,094 additions and 331 deletions.
15 changes: 10 additions & 5 deletions src/buffer/out/Row.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,11 @@ const UnicodeStorage& ROW::GetUnicodeStorage() const noexcept
// Arguments:
// - it - custom console iterator to use for seeking input data. bool() false when it becomes invalid while seeking.
// - index - column in row to start writing at
// - setWrap - set the wrap flags if we hit the end of the row while writing and there's still more data in the iterator.
// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data in the iterator.
// - limitRight - right inclusive column ID for the last write in this row. (optional, will just write to the end of row if nullopt)
// Return Value:
// - iterator to first cell that was not written to this row.
OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, const bool setWrap, std::optional<size_t> limitRight)
OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, const std::optional<bool> wrap, std::optional<size_t> limitRight)
{
THROW_HR_IF(E_INVALIDARG, index >= _charRow.size());
THROW_HR_IF(E_INVALIDARG, limitRight.value_or(0) >= _charRow.size());
Expand Down Expand Up @@ -202,10 +202,15 @@ OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, co
++it;
}

// If we're asked to set the wrap status and we just filled the last column with some text, set wrap status on the row.
if (setWrap && fillingLastColumn)
// If we're asked to (un)set the wrap status and we just filled the last column with some text...
// NOTE:
// - wrap = std::nullopt --> don't change the wrap value
// - wrap = true --> we're filling cells as a steam, consider this a wrap
// - wrap = false --> we're filling cells as a block, unwrap
if (wrap.has_value() && fillingLastColumn)
{
_charRow.SetWrapForced(true);
// set wrap status on the row to parameter's value.
_charRow.SetWrapForced(wrap.value());
}
}
else
Expand Down
2 changes: 1 addition & 1 deletion src/buffer/out/Row.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class ROW final
UnicodeStorage& GetUnicodeStorage() noexcept;
const UnicodeStorage& GetUnicodeStorage() const noexcept;

OutputCellIterator WriteCells(OutputCellIterator it, const size_t index, const bool setWrap, std::optional<size_t> limitRight = std::nullopt);
OutputCellIterator WriteCells(OutputCellIterator it, const size_t index, const std::optional<bool> wrap = std::nullopt, std::optional<size_t> limitRight = std::nullopt);

friend bool operator==(const ROW& a, const ROW& b) noexcept;

Expand Down
14 changes: 7 additions & 7 deletions src/buffer/out/TextAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ COLORREF TextAttribute::CalculateRgbBackground(std::basic_string_view<COLORREF>
COLORREF TextAttribute::_GetRgbForeground(std::basic_string_view<COLORREF> colorTable,
COLORREF defaultColor) const noexcept
{
return _foreground.GetColor(colorTable, defaultColor, _isBold);
return _foreground.GetColor(colorTable, defaultColor, IsBold());
}

// Routine Description:
Expand Down Expand Up @@ -155,11 +155,6 @@ void TextAttribute::SetColor(const COLORREF rgbColor, const bool fIsForeground)
}
}

bool TextAttribute::IsBold() const noexcept
{
return _isBold;
}

bool TextAttribute::_IsReverseVideo() const noexcept
{
return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO);
Expand Down Expand Up @@ -215,6 +210,11 @@ void TextAttribute::Debolden() noexcept
_SetBoldness(false);
}

void TextAttribute::SetExtendedAttributes(const ExtendedAttributes attrs) noexcept
{
_extendedAttrs = attrs;
}

// Routine Description:
// - swaps foreground and background color
void TextAttribute::Invert() noexcept
Expand All @@ -224,7 +224,7 @@ void TextAttribute::Invert() noexcept

void TextAttribute::_SetBoldness(const bool isBold) noexcept
{
_isBold = isBold;
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Bold, isBold);
}

void TextAttribute::SetDefaultForeground() noexcept
Expand Down
36 changes: 28 additions & 8 deletions src/buffer/out/TextAttribute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,24 @@ Revision History:
#include "WexTestClass.h"
#endif

#pragma pack(push, 1)

class TextAttribute final
{
public:
constexpr TextAttribute() noexcept :
_wAttrLegacy{ 0 },
_foreground{},
_background{},
_isBold{ false }
_extendedAttrs{ ExtendedAttributes::Normal }
{
}

constexpr TextAttribute(const WORD wLegacyAttr) noexcept :
_wAttrLegacy{ gsl::narrow_cast<WORD>(wLegacyAttr & META_ATTRS) },
_foreground{ gsl::narrow_cast<BYTE>(wLegacyAttr & FG_ATTRS) },
_background{ gsl::narrow_cast<BYTE>((wLegacyAttr & BG_ATTRS) >> 4) },
_isBold{ false }
_extendedAttrs{ ExtendedAttributes::Normal }
{
// If we're given lead/trailing byte information with the legacy color, strip it.
WI_ClearAllFlags(_wAttrLegacy, COMMON_LVB_SBCSDBCS);
Expand All @@ -53,7 +55,7 @@ class TextAttribute final
_wAttrLegacy{ 0 },
_foreground{ rgbForeground },
_background{ rgbBackground },
_isBold{ false }
_extendedAttrs{ ExtendedAttributes::Normal }
{
}

Expand All @@ -62,7 +64,7 @@ class TextAttribute final
const BYTE fg = (_foreground.GetIndex() & FG_ATTRS);
const BYTE bg = (_background.GetIndex() << 4) & BG_ATTRS;
const WORD meta = (_wAttrLegacy & META_ATTRS);
return (fg | bg | meta) | (_isBold ? FOREGROUND_INTENSITY : 0);
return (fg | bg | meta) | (IsBold() ? FOREGROUND_INTENSITY : 0);
}

// Method Description:
Expand All @@ -85,7 +87,7 @@ class TextAttribute final
const BYTE fg = (fgIndex & FG_ATTRS);
const BYTE bg = (bgIndex << 4) & BG_ATTRS;
const WORD meta = (_wAttrLegacy & META_ATTRS);
return (fg | bg | meta) | (_isBold ? FOREGROUND_INTENSITY : 0);
return (fg | bg | meta) | (IsBold() ? FOREGROUND_INTENSITY : 0);
}

COLORREF CalculateRgbForeground(std::basic_string_view<COLORREF> colorTable,
Expand Down Expand Up @@ -131,7 +133,18 @@ class TextAttribute final
friend constexpr bool operator!=(const WORD& legacyAttr, const TextAttribute& attr) noexcept;

bool IsLegacy() const noexcept;
bool IsBold() const noexcept;

constexpr bool IsBold() const noexcept
{
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Bold);
}

constexpr ExtendedAttributes GetExtendedAttributes() const noexcept
{
return _extendedAttrs;
}

void SetExtendedAttributes(const ExtendedAttributes attrs) noexcept;

void SetForeground(const COLORREF rgbForeground) noexcept;
void SetBackground(const COLORREF rgbBackground) noexcept;
Expand Down Expand Up @@ -159,7 +172,7 @@ class TextAttribute final
WORD _wAttrLegacy;
TextColor _foreground;
TextColor _background;
bool _isBold;
ExtendedAttributes _extendedAttrs;

#ifdef UNIT_TESTING
friend class TextBufferTests;
Expand All @@ -169,6 +182,13 @@ class TextAttribute final
#endif
};

#pragma pack(pop)
// 2 for _wAttrLegacy
// 4 for _foreground
// 4 for _background
// 1 for _extendedAttrs
static_assert(sizeof(TextAttribute) <= 11 * sizeof(BYTE), "We should only need 11B for an entire TextColor. Any more than that is just waste");

enum class TextAttributeBehavior
{
Stored, // use contained text attribute
Expand All @@ -181,7 +201,7 @@ constexpr bool operator==(const TextAttribute& a, const TextAttribute& b) noexce
return a._wAttrLegacy == b._wAttrLegacy &&
a._foreground == b._foreground &&
a._background == b._background &&
a._isBold == b._isBold;
a._extendedAttrs == b._extendedAttrs;
}

constexpr bool operator!=(const TextAttribute& a, const TextAttribute& b) noexcept
Expand Down
13 changes: 8 additions & 5 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -318,10 +318,12 @@ OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt)
// Arguments:
// - givenIt - Iterator representing output cell data to write
// - target - the row/column to start writing the text to
// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data
// Return Value:
// - The final position of the iterator
OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt,
const COORD target)
const COORD target,
const std::optional<bool> wrap)
{
// Make mutable copy so we can walk.
auto it = givenIt;
Expand All @@ -336,7 +338,8 @@ OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt,
while (it && size.IsInBounds(lineTarget))
{
// Attempt to write as much data as possible onto this line.
it = WriteLine(it, lineTarget, true);
// NOTE: if wrap = true/false, we want to set the line's wrap to true/false (respectively) if we reach the end of the line
it = WriteLine(it, lineTarget, wrap);

// Move to the next line down.
lineTarget.X = 0;
Expand All @@ -351,13 +354,13 @@ OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt,
// Arguments:
// - givenIt - The iterator that will dereference into cell data to insert
// - target - Coordinate targeted within output buffer
// - setWrap - Whether we should try to set the wrap flag if we write up to the end of the line and have more data
// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data in the iterator.
// - limitRight - Optionally restrict the right boundary for writing (e.g. stop writing earlier than the end of line)
// Return Value:
// - The iterator, but advanced to where we stopped writing. Use to find input consumed length or cells written length.
OutputCellIterator TextBuffer::WriteLine(const OutputCellIterator givenIt,
const COORD target,
const bool setWrap,
const std::optional<bool> wrap,
std::optional<size_t> limitRight)
{
// If we're not in bounds, exit early.
Expand All @@ -368,7 +371,7 @@ OutputCellIterator TextBuffer::WriteLine(const OutputCellIterator givenIt,

// Get the row and write the cells
ROW& row = GetRowByOffset(target.Y);
const auto newIt = row.WriteCells(givenIt, target.X, setWrap, limitRight);
const auto newIt = row.WriteCells(givenIt, target.X, wrap, limitRight);

// Take the cell distance written and notify that it needs to be repainted.
const auto written = newIt.GetCellDistance(givenIt);
Expand Down
5 changes: 3 additions & 2 deletions src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,12 @@ class TextBuffer final
OutputCellIterator Write(const OutputCellIterator givenIt);

OutputCellIterator Write(const OutputCellIterator givenIt,
const COORD target);
const COORD target,
const std::optional<bool> wrap = true);

OutputCellIterator WriteLine(const OutputCellIterator givenIt,
const COORD target,
const bool setWrap = false,
const std::optional<bool> setWrap = std::nullopt,
const std::optional<size_t> limitRight = std::nullopt);

bool InsertCharacter(const wchar_t wch, const DbcsAttribute dbcsAttribute, const TextAttribute attr);
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ bool TerminalDispatch::_SetRgbColorsHelper(_In_reads_(cOptions) const DispatchTy
isForeground = false;
}

if (typeOpt == DispatchTypes::GraphicsOptions::RGBColor && cOptions >= 5)
if (typeOpt == DispatchTypes::GraphicsOptions::RGBColorOrFaint && cOptions >= 5)
{
*pcOptionsConsumed = 5;
// ensure that each value fits in a byte
Expand All @@ -115,7 +115,7 @@ bool TerminalDispatch::_SetRgbColorsHelper(_In_reads_(cOptions) const DispatchTy

fSuccess = _terminalApi.SetTextRgbColor(color, isForeground);
}
else if (typeOpt == DispatchTypes::GraphicsOptions::Xterm256Index && cOptions >= 3)
else if (typeOpt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index && cOptions >= 3)
{
*pcOptionsConsumed = 3;
if (rgOptions[2] <= 255) // ensure that the provided index is on the table
Expand Down
5 changes: 4 additions & 1 deletion src/host/_output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,10 @@ void WriteToScreen(SCREEN_INFORMATION& screenInfo, const Viewport& region)
try
{
const OutputCellIterator it(character, lengthToWrite);
const auto done = screenInfo.Write(it, startingCoordinate);

// when writing to the buffer, specifically unset wrap if we get to the last column.
// a fill operation should UNSET wrap in that scenario. See GH #1126 for more details.
const auto done = screenInfo.Write(it, startingCoordinate, false);
cellsModified = done.GetInputDistance(it);

// Notify accessibility
Expand Down
Loading

0 comments on commit 62f5299

Please sign in to comment.