Skip to content

Commit

Permalink
Reimplement the VT tab stop functionality (#5173)
Browse files Browse the repository at this point in the history 
## Summary of the Pull Request

This is essentially a rewrite of the VT tab stop functionality, implemented entirely within the `AdaptDispatch` class. This significantly simplifies the `ConGetSet` interface, and should hopefully make it easier to share the functionality with the Windows Terminal VT implementation in the future.

By removing the dependence on the `SCREEN_INFORMATION` class, it fixes the problem of the the tab state not being preserved when switching between the main and alternate buffers. And the new architecture also fixes problems with the tabs not being correctly initialized when the screen is resized.

## References

This fixes one aspect of issue #3545.
It also supersedes the fix for #411 (PR #2816).
I'm hoping the simplification of `ConGetSet` will help with #3849.

## PR Checklist
* [x] Closes #4669
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] 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: #xxx

## Detailed Description of the Pull Request / Additional comments

In the new tab architecture, there is now a `vector<bool>` (__tabStopColumns_), which tracks whether any particular column is a tab stop or not. There is also a __initDefaultTabStops_ flag indicating whether the default tab stop positions need to be initialised when the screen is resized.

The way this works, the vector is initially empty, and only initialized (to the current width of the screen) when it needs to be used. When the vector grows in size, the __initDefaultTabStops_ flag determines whether the new columns are set to false, or if every 8th column is set to true.

By default we want the latter behaviour - newly revealed columns should have default tab stops assigned to them - so __initDefaultTabStops_ is set to true. However, after a `TBC 3` operation (i.e. we've cleared all tab stops), there should be no tab stops in any newly revealed columns, so __initDefaultTabStops_ is set to false.

Note that the __tabStopColumns_ vector is never made smaller when the window is shrunk, and that way it can preserve the state of tab stops that are off screen, but which may come into range if the window is made bigger again.

However, we can can still reset the vector completely after an `RIS` or `TBC 3` operation, since the state can then be reconstructed automatically based on just the __initDefaultTabStops_ flag.

## Validation Steps Performed

The original screen buffer tests had to be rewritten to set and query the tab stop state using escape sequences rather than interacting with the `SCREEN_INFORMATION` class directly, but otherwise the structure of most tests remained largely the same.

However, the alt buffer test was significantly rewritten, since the original behaviour was incorrect, and the initialization test was dropped completely, since it was no longer applicable. The adapter tests were also dropped, since they were testing the `ConGetSet` interface which has now been removed.

I also had to make an addition to the method setup of the screen buffer tests (making sure the viewport was appropriately initialized), since there were some tests (unrelated to tab stops) that were previously dependent on the state being set in the tab initialization test which has now been removed.

I've manually tested the issue described in #4669 and confirmed that the tabs now produce the correct spacing after a resize.
  • Loading branch information
@donno2048
donno2048 committed Apr 1, 2020
1 parent f4fa0a7 commit 7190f02
Show file tree
Hide file tree
Showing 11 changed files with 344 additions and 602 deletions.
107 changes: 0 additions & 107 deletions src/host/getset.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,13 +404,6 @@ void ApiRoutines::GetNumberOfConsoleMouseButtonsImpl(ULONG& buttons) noexcept
{
// jiggle the handle
screenInfo.GetStateMachine().ResetState();
screenInfo.ClearTabStops();
}
// if we're moving from VT off->on
else if (WI_IsFlagSet(dwNewMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING) &&
WI_IsFlagClear(dwOldMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING))
{
screenInfo.SetDefaultVtTabStops();
}

gci.SetVirtTermLevel(WI_IsFlagSet(dwNewMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING) ? 1 : 0);
Expand Down Expand Up @@ -1522,99 +1515,6 @@ void DoSrvPrivateUseMainScreenBuffer(SCREEN_INFORMATION& screenInfo)
screenInfo.GetActiveBuffer().UseMainScreenBuffer();
}

// Routine Description:
// - A private API call for setting a VT tab stop in the cursor's current column.
// Parameters:
// <none>
// Return value:
// - STATUS_SUCCESS if handled successfully. Otherwise, an appropriate status code indicating the error.
[[nodiscard]] NTSTATUS DoSrvPrivateHorizontalTabSet()
{
CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
SCREEN_INFORMATION& _screenBuffer = gci.GetActiveOutputBuffer().GetActiveBuffer();

const COORD cursorPos = _screenBuffer.GetTextBuffer().GetCursor().GetPosition();
try
{
_screenBuffer.AddTabStop(cursorPos.X);
}
catch (...)
{
return NTSTATUS_FROM_HRESULT(wil::ResultFromCaughtException());
}
return STATUS_SUCCESS;
}

// Routine Description:
// - A private helper for executing a number of tabs.
// Parameters:
// sNumTabs - The number of tabs to execute
// fForward - whether to tab forward or backwards
// Return value:
// - STATUS_SUCCESS if handled successfully. Otherwise, an appropriate status code indicating the error.
[[nodiscard]] NTSTATUS DoPrivateTabHelper(const SHORT sNumTabs, _In_ bool fForward)
{
CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
SCREEN_INFORMATION& _screenBuffer = gci.GetActiveOutputBuffer().GetActiveBuffer();
COORD cursorPos = _screenBuffer.GetTextBuffer().GetCursor().GetPosition();

FAIL_FAST_IF(!(sNumTabs >= 0));
for (SHORT sTabsExecuted = 0; sTabsExecuted < sNumTabs; sTabsExecuted++)
{
cursorPos = (fForward) ? _screenBuffer.GetForwardTab(cursorPos) : _screenBuffer.GetReverseTab(cursorPos);
}

return AdjustCursorPosition(_screenBuffer, cursorPos, TRUE, nullptr);
}

// Routine Description:
// - A private API call for performing a forwards tab. This will take the
// cursor to the tab stop following its current location. If there are no
// more tabs in this row, it will take it to the right side of the window.
// If it's already in the last column of the row, it will move it to the next line.
// Parameters:
// - sNumTabs - The number of tabs to perform.
// Return value:
// - STATUS_SUCCESS if handled successfully. Otherwise, an appropriate status code indicating the error.
[[nodiscard]] NTSTATUS DoSrvPrivateForwardTab(const SHORT sNumTabs)
{
return DoPrivateTabHelper(sNumTabs, true);
}

// Routine Description:
// - A private API call for performing a backwards tab. This will take the
// cursor to the tab stop previous to its current location. It will not reverse line feed.
// Parameters:
// - sNumTabs - The number of tabs to perform.
// Return value:
// - STATUS_SUCCESS if handled successfully. Otherwise, an appropriate status code indicating the error.
[[nodiscard]] NTSTATUS DoSrvPrivateBackwardsTab(const SHORT sNumTabs)
{
return DoPrivateTabHelper(sNumTabs, false);
}

// Routine Description:
// - A private API call for clearing the VT tabs that have been set.
// Parameters:
// - fClearAll - If false, only clears the tab in the current column (if it exists)
// otherwise clears all set tabs. (and reverts to legacy 8-char tabs behavior.)
// Return value:
// - None
void DoSrvPrivateTabClear(const bool fClearAll)
{
CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
SCREEN_INFORMATION& screenBuffer = gci.GetActiveOutputBuffer().GetActiveBuffer();
if (fClearAll)
{
screenBuffer.ClearTabStops();
}
else
{
const COORD cursorPos = screenBuffer.GetTextBuffer().GetCursor().GetPosition();
screenBuffer.ClearTabStop(cursorPos.X);
}
}

// Routine Description:
// - A private API call for enabling VT200 style mouse mode.
// Parameters:
Expand Down Expand Up @@ -2080,13 +1980,6 @@ void DoSrvIsConsolePty(bool& isPty)
isPty = gci.IsInVtIoMode();
}

// Routine Description:
// - a private API call for setting the default tab stops in the active screen buffer.
void DoSrvPrivateSetDefaultTabStops()
{
ServiceLocator::LocateGlobals().getConsoleInformation().GetActiveOutputBuffer().GetActiveBuffer().SetDefaultVtTabStops();
}

// Routine Description:
// - internal logic for adding or removing lines in the active screen buffer
// this also moves the cursor to the left margin, which is expected behavior for IL and DL
Expand Down
6 changes: 0 additions & 6 deletions src/host/getset.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@ void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool
[[nodiscard]] NTSTATUS DoSrvPrivateUseAlternateScreenBuffer(SCREEN_INFORMATION& screenInfo);
void DoSrvPrivateUseMainScreenBuffer(SCREEN_INFORMATION& screenInfo);

[[nodiscard]] NTSTATUS DoSrvPrivateHorizontalTabSet();
[[nodiscard]] NTSTATUS DoSrvPrivateForwardTab(const SHORT sNumTabs);
[[nodiscard]] NTSTATUS DoSrvPrivateBackwardsTab(const SHORT sNumTabs);
void DoSrvPrivateTabClear(const bool fClearAll);

void DoSrvPrivateEnableVT200MouseMode(const bool fEnable);
void DoSrvPrivateEnableUTF8ExtendedMouseMode(const bool fEnable);
void DoSrvPrivateEnableSGRExtendedMouseMode(const bool fEnable);
Expand Down Expand Up @@ -84,7 +79,6 @@ void DoSrvGetConsoleOutputCodePage(unsigned int& codepage);

void DoSrvIsConsolePty(bool& isPty);

void DoSrvPrivateSetDefaultTabStops();
void DoSrvPrivateDeleteLines(const size_t count);
void DoSrvPrivateInsertLines(const size_t count);

Expand Down
74 changes: 0 additions & 74 deletions src/host/outputStream.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -491,80 +491,6 @@ bool ConhostInternalGetSet::PrivateUseMainScreenBuffer()
return true;
}

// - Connects the PrivateHorizontalTabSet call directly into our Driver Message servicing call inside Conhost.exe
// PrivateHorizontalTabSet is an internal-only "API" call that the vt commands can execute,
// but it is not represented as a function call on out public API surface.
// Arguments:
// <none>
// Return Value:
// - true if successful (see PrivateHorizontalTabSet). false otherwise.
bool ConhostInternalGetSet::PrivateHorizontalTabSet()
{
return NT_SUCCESS(DoSrvPrivateHorizontalTabSet());
}

// Routine Description:
// - Connects the PrivateForwardTab call directly into our Driver Message servicing call inside Conhost.exe
// PrivateForwardTab is an internal-only "API" call that the vt commands can execute,
// but it is not represented as a function call on out public API surface.
// Arguments:
// - sNumTabs - the number of tabs to execute
// Return Value:
// - true if successful (see PrivateForwardTab). false otherwise.
bool ConhostInternalGetSet::PrivateForwardTab(const size_t numTabs)
{
SHORT tabs;
if (FAILED(SizeTToShort(numTabs, &tabs)))
{
return false;
}

return NT_SUCCESS(DoSrvPrivateForwardTab(tabs));
}

// Routine Description:
// - Connects the PrivateBackwardsTab call directly into our Driver Message servicing call inside Conhost.exe
// PrivateBackwardsTab is an internal-only "API" call that the vt commands can execute,
// but it is not represented as a function call on out public API surface.
// Arguments:
// - numTabs - the number of tabs to execute
// Return Value:
// - true if successful (see PrivateBackwardsTab). false otherwise.
bool ConhostInternalGetSet::PrivateBackwardsTab(const size_t numTabs)
{
SHORT tabs;
if (FAILED(SizeTToShort(numTabs, &tabs)))
{
return false;
}

return NT_SUCCESS(DoSrvPrivateBackwardsTab(tabs));
}

// Routine Description:
// - Connects the PrivateTabClear call directly into our Driver Message servicing call inside Conhost.exe
// PrivateTabClear is an internal-only "API" call that the vt commands can execute,
// but it is not represented as a function call on out public API surface.
// Arguments:
// - clearAll - set to true to enable blinking, false to disable
// Return Value:
// - true if successful (see PrivateTabClear). false otherwise.
bool ConhostInternalGetSet::PrivateTabClear(const bool clearAll)
{
DoSrvPrivateTabClear(clearAll);
return true;
}

// Routine Description:
// - Connects the PrivateSetDefaultTabStops call directly into the private api point
// Return Value:
// - true
bool ConhostInternalGetSet::PrivateSetDefaultTabStops()
{
DoSrvPrivateSetDefaultTabStops();
return true;
}

// Routine Description:
// - Connects the PrivateEnableVT200MouseMode call directly into our Driver Message servicing call inside Conhost.exe
// PrivateEnableVT200MouseMode is an internal-only "API" call that the vt commands can execute,
Expand Down
6 changes: 0 additions & 6 deletions src/host/outputStream.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,6 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::

bool PrivateUseMainScreenBuffer() override;

bool PrivateHorizontalTabSet();
bool PrivateForwardTab(const size_t numTabs) override;
bool PrivateBackwardsTab(const size_t numTabs) override;
bool PrivateTabClear(const bool clearAll) override;
bool PrivateSetDefaultTabStops() override;

bool PrivateEnableVT200MouseMode(const bool enabled) override;
bool PrivateEnableUTF8ExtendedMouseMode(const bool enabled) override;
bool PrivateEnableSGRExtendedMouseMode(const bool enabled) override;
Expand Down
Loading

0 comments on commit 7190f02

Please sign in to comment.