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

Replace MarkKind::None with MarkKind::Output #18291

Closed
wants to merge 1 commit into from
Closed

Replace MarkKind::None with MarkKind::Output #18291

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
2 changes: 1 addition & 1 deletion src/buffer/out/TextAttribute.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,5 +393,5 @@ void TextAttribute::SetStandardErase() noexcept
{
_attrs = CharacterAttributes::Normal;
_hyperlinkId = 0;
_markKind = MarkKind::None;
_markKind = MarkKind::Output;
}
11 changes: 5 additions & 6 deletions src/buffer/out/TextAttribute.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,9 @@ enum class UnderlineStyle
// We only need a few bits, but uint8_t apparently doesn't satisfy std::has_unique_object_representations_v
enum class MarkKind : uint16_t
{
None = 0,
Output = 0,
Prompt = 1,
Command = 2,
Output = 3,
};

class TextAttribute final
Expand All @@ -56,7 +55,7 @@ class TextAttribute final
_background{},
_hyperlinkId{ 0 },
_underlineColor{},
_markKind{ MarkKind::None }
_markKind{ MarkKind::Output }
{
}

Expand All @@ -66,7 +65,7 @@ class TextAttribute final
_background{ gsl::at(s_legacyBackgroundColorMap, (wLegacyAttr & BG_ATTRS) >> 4) },
_hyperlinkId{ 0 },
_underlineColor{},
_markKind{ MarkKind::None }
_markKind{ MarkKind::Output }
{
}

Expand All @@ -78,7 +77,7 @@ class TextAttribute final
_background{ rgbBackground },
_hyperlinkId{ 0 },
_underlineColor{ rgbUnderline },
_markKind{ MarkKind::None }
_markKind{ MarkKind::Output }
{
}

Expand All @@ -88,7 +87,7 @@ class TextAttribute final
_background{ background },
_hyperlinkId{ hyperlinkId },
_underlineColor{ underlineColor },
_markKind{ MarkKind::None }
_markKind{ MarkKind::Output }
{
}

Expand Down
9 changes: 3 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 @@ -3221,7 +3221,7 @@ void TextBuffer::ClearMarksInRange(
row.SetScrollbarData(std::nullopt);
for (auto& [attr, length] : runs)
{
attr.SetMarkAttributes(MarkKind::None);
attr.SetMarkAttributes(MarkKind::Output);
}
}
}
Expand All @@ -3246,7 +3246,6 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset,
bool startedPrompt = false;
bool startedCommand = false;
bool startedOutput = false;
MarkKind lastMarkKind = MarkKind::Output;

const auto endThisMark = [&](auto x, auto y) {
if (startedOutput)
Expand Down Expand Up @@ -3280,7 +3279,7 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset,
const auto nextX = gsl::narrow_cast<uint16_t>(x + length);
const auto markKind{ attr.GetMarkAttributes() };

if (markKind != MarkKind::None)
if (markKind != MarkKind::Output)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know, entirely removing this conditional seems to work, form 1 minute of playing with it

{
lastMarkedText = { nextX, y };

Expand Down Expand Up @@ -3316,8 +3315,6 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset,

endThisMark(lastMarkedText.x, lastMarkedText.y);
}
// Otherwise, we've changed from any state -> any state, and it doesn't really matter.
lastMarkKind = markKind;
Copy link
Member Author

@lhecker lhecker Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lastMarkKind variable is only ever written to but never read.
The condition above, if (markKind != MarkKind::None), also seems wrong to me. Is it supposed to test lastMarkKind?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @zadjii-msft in case he has any thoughts here

}
// advance to next run of text
x = nextX;
Expand Down Expand Up @@ -3510,7 +3507,7 @@ bool TextBuffer::StartOutput()
// the exit code on that row's scroll mark.
void TextBuffer::EndCurrentCommand(std::optional<unsigned int> error)
{
_currentAttributes.SetMarkAttributes(MarkKind::None);
_currentAttributes.SetMarkAttributes(MarkKind::Output);

for (auto y = GetCursor().GetPosition().y; y >= 0; y--)
{
Expand Down
Loading