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.

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

Implement the rest of the FTCS marks #14341

Merged
19 commits merged into from
Dec 1, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
58 changes: 49 additions & 9 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -821,11 +821,32 @@ bool Terminal::SendCharEvent(const wchar_t ch, const WORD scanCode, const Contro
// Then treat this line like it's a prompt mark.
if (_autoMarkPrompts && vkey == VK_RETURN && !_inAltBuffer())
{
DispatchTypes::ScrollMark mark;
mark.category = DispatchTypes::MarkCategory::Prompt;
// Don't set the color - we'll automatically use the DEFAULT_FOREGROUND
// color for any MarkCategory::Prompt marks without one set.
AddMark(mark);
// * If we have a _currentPrompt:
// - Then we did know that the prompt started, (we may have also
// already gotten a CommandStart sequence). The user has pressed
// enter, and we're treating that like the prompt has now ended.
// - Perform a FTCS_COMMAND_EXECUTED, so that we start marking this
// as output.
// - This enables CMD to have full FTCS support, even though there's
// no point in CMD to insert a "preexec" hook
// * Else: We don't have a prompt. We don't know anything else, but we
// can set the whole like as the prompt, no command, and start the
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
// command_executed now.

if (_currentPrompt)
{
OutputStart();
Copy link
Member

@lhecker lhecker Nov 8, 2022

Choose a reason for hiding this comment

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

nit: Both branches end up calling OutputStart at the end.

}
else
{
DispatchTypes::ScrollMark mark;
mark.category = DispatchTypes::MarkCategory::Prompt;
// Don't set the color - we'll automatically use the DEFAULT_FOREGROUND
// color for any MarkCategory::Prompt marks without one set.
AddMark(mark);
_currentPrompt = &_scrollMarks.back();
OutputStart();
}
}

// Unfortunately, the UI doesn't give us both a character down and a
Expand Down Expand Up @@ -1268,8 +1289,16 @@ void Terminal::_AdjustCursorPosition(const til::point proposedPosition)
{
for (auto& mark : _scrollMarks)
{
// Move the mark up
mark.start.y -= rowsPushedOffTopOfBuffer;

// If the mark had sub-regions, then move those pointers too
if (mark.commandEnd.has_value())
(*mark.commandEnd).y -= rowsPushedOffTopOfBuffer;
if (mark.outputEnd.has_value())
(*mark.outputEnd).y -= rowsPushedOffTopOfBuffer;
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
}

_scrollMarks.erase(std::remove_if(_scrollMarks.begin(),
_scrollMarks.end(),
[](const VirtualTerminal::DispatchTypes::ScrollMark& m) { return m.start.y < 0; }),
Expand Down Expand Up @@ -1559,6 +1588,7 @@ void Terminal::_updateUrlDetection()
}
}

// NOTE: This is the version of AddMark that comes from the UI. The VT api call into this too.
void Terminal::AddMark(const Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark& mark,
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
const til::point& start,
const til::point& end)
Expand All @@ -1577,6 +1607,9 @@ void Terminal::AddMark(const Microsoft::Console::VirtualTerminal::DispatchTypes:
// Tell the control that the scrollbar has somehow changed. Used as a
// workaround to force the control to redraw any scrollbar marks
_NotifyScrollEvent();

// DON'T set _currentPrompt. The VT impl will do that for you. We don't want
// UI-driven marks to set that.
}

void Terminal::ClearMark()
Expand All @@ -1593,13 +1626,19 @@ void Terminal::ClearMark()
start = til::point{ GetSelectionAnchor() };
end = til::point{ GetSelectionEnd() };
}
auto inSelection = [&start, &end](const DispatchTypes::ScrollMark& m) {
Copy link
Member

Choose a reason for hiding this comment

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

(I just realized that the two positions were for a selection!)

return (m.start >= start && m.start <= end) ||
(m.end >= start && m.end <= end);
};

if (_currentPrompt && inSelection(*_currentPrompt))
{
_currentPrompt = nullptr;
}

_scrollMarks.erase(std::remove_if(_scrollMarks.begin(),
_scrollMarks.end(),
[&start, &end](const auto& m) {
return (m.start >= start && m.start <= end) ||
(m.end >= start && m.end <= end);
}),
inSelection),
_scrollMarks.end());

// Tell the control that the scrollbar has somehow changed. Used as a
Expand All @@ -1608,6 +1647,7 @@ void Terminal::ClearMark()
}
void Terminal::ClearAllMarks()
{
_currentPrompt = nullptr;
_scrollMarks.clear();
// Tell the control that the scrollbar has somehow changed. Used as a
// workaround to force the control to redraw any scrollbar marks
Expand Down
5 changes: 4 additions & 1 deletion src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ class Microsoft::Terminal::Core::Terminal final :
void UseMainScreenBuffer() override;

void AddMark(const Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark& mark) override;

void CommandStart() override;
void OutputStart() override;
void CommandFinished(std::optional<unsigned int> error) override;
bool IsConsolePty() const override;
bool IsVtInputEnabled() const override;
void NotifyAccessibilityChange(const til::rect& changedRect) override;
Expand Down Expand Up @@ -403,6 +405,7 @@ class Microsoft::Terminal::Core::Terminal final :
std::optional<KeyEventCodes> _lastKeyEventCodes;

std::vector<Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark> _scrollMarks;
Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark* _currentPrompt{ nullptr };
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved

static WORD _ScanCodeFromVirtualKey(const WORD vkey) noexcept;
static WORD _VirtualKeyFromScanCode(const WORD scanCode) noexcept;
Expand Down
47 changes: 47 additions & 0 deletions src/cascadia/TerminalCore/TerminalApi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,57 @@ void Terminal::UseMainScreenBuffer()
CATCH_LOG();
}

// NOTE: This is the version of AddMark that comes from VT
void Terminal::AddMark(const Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark& mark)
{
const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() };
AddMark(mark, cursorPos, cursorPos);
auto* last = &_scrollMarks.back();
if (last->category == Microsoft::Console::VirtualTerminal::DispatchTypes::MarkCategory::Prompt)
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
{
_currentPrompt = last;
}
}

void Terminal::CommandStart()
{
if (_currentPrompt)
{
const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() };

// Move the end of this mark to the current cursor position. The prompt
// is between [mark.start, mark.end)
_currentPrompt->end = cursorPos;
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
}
}

void Terminal::OutputStart()
{
if (_currentPrompt)
{
const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() };
// Mark the current cursor pos as the the end of the command. The command
// is between [mark.end, mark.commandEnd)
_currentPrompt->commandEnd = cursorPos;
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
}
}

void Terminal::CommandFinished(std::optional<unsigned int> error)
{
if (_currentPrompt)
{
const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() };
// Mark the current cursor pos as the the end of the output. The command
// is between [mark.commandEnd, mark.outputEnd)
_currentPrompt->outputEnd = cursorPos;
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved

if (error.has_value())
{
_currentPrompt->category = *error == 0u ?
DispatchTypes::MarkCategory::Success :
DispatchTypes::MarkCategory::Error;
}
}
}
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved

// Method Description:
Expand Down
15 changes: 15 additions & 0 deletions src/host/outputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -486,3 +486,18 @@ void ConhostInternalGetSet::AddMark(const Microsoft::Console::VirtualTerminal::D
{
// Not implemented for conhost.
}

void ConhostInternalGetSet::CommandStart()
{
// Not implemented for conhost.
}

void ConhostInternalGetSet::OutputStart()
{
// Not implemented for conhost.
}

void ConhostInternalGetSet::CommandFinished(std::optional<unsigned int> /*error*/)
{
// Not implemented for conhost.
}
3 changes: 3 additions & 0 deletions src/host/outputStream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::
void NotifyAccessibilityChange(const til::rect& changedRect) override;

void AddMark(const Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark& mark) override;
void CommandStart() override;
void OutputStart() override;
void CommandFinished(std::optional<unsigned int> error) override;

private:
Microsoft::Console::IIoProvider& _io;
Expand Down
3 changes: 3 additions & 0 deletions src/terminal/adapter/DispatchTypes.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,9 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes
std::optional<til::color> color;
til::point start;
til::point end; // exclusive
std::optional<til::point> commandEnd;
std::optional<til::point> outputEnd;

MarkCategory category{ MarkCategory::Info };
// Other things we may want to think about in the future are listed in
// GH#11000
Expand Down
3 changes: 3 additions & 0 deletions src/terminal/adapter/ITerminalApi.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,8 @@ namespace Microsoft::Console::VirtualTerminal
virtual void NotifyAccessibilityChange(const til::rect& changedRect) = 0;

virtual void AddMark(const Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark& mark) = 0;
virtual void CommandStart() = 0;
virtual void OutputStart() = 0;
virtual void CommandFinished(std::optional<unsigned int> error) = 0;
};
}
29 changes: 29 additions & 0 deletions src/terminal/adapter/adaptDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2674,6 +2674,35 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string)
_api.AddMark(mark);
return true;
}
else if (action == L"B") // FTCS_COMMAND_START
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
{
_api.CommandStart();
return true;
}
else if (action == L"C") // FTCS_COMMAND_EXECUTED
{
_api.OutputStart();
return true;
}
else if (action == L"D") // FTCS_COMMAND_FINISHED
{
std::optional<unsigned int> error = std::nullopt;
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
if (parts.size() >= 2)
{
const auto errorString = til::at(parts, 1);

// If we fail to parse the code, then it was gibberish, or it might
// have just started with "-". Either way, let's just treat it as an
// error and move on.
//
// We know that "0" will be successfully parsed, and that's close enough.
unsigned int parsedError = 0;
error = Utils::StringToUint(errorString, parsedError) ? parsedError :
static_cast<unsigned int>(-1);
}
_api.CommandFinished(error);
return true;
}

// When we add the rest of the FTCS sequences (GH#11000), we should add a
// simple state machine here to track the most recently emitted mark from
Expand Down