Skip to content

Commit

Permalink
Remove the VT52 movement ops for now, to avoid conflicting with the V…
Browse files Browse the repository at this point in the history
…T100 IND control. (#4044)

## Summary of the Pull Request

This removes support for the the VT52 cursor movement operations, in preparation for PR #3271, since the cursor back operation conflicts with the VT100 [`IND`](https://vt100.net/docs/vt510-rm/IND.html) sequence, which we're planning to add. Eventually these ops will be brought back as part of a proper VT52 implementation, when appropriately activated by the [`DECANM`](https://vt100.net/docs/vt510-rm/DECANM.html) mode.

## References

#976 #3271

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

## Detailed Description of the Pull Request / Additional comments

The operations were removed from the `OutputStateMachineEngine`, and their associated test cases were removed from `StateMachineExternalTest`. There is no real loss of functionality here, since these sequences were never valid as implemented.

## Validation Steps Performed

I've just tested manually to confirm that the sequences no longer work.
  • Loading branch information
j4james authored and msftbot[bot] committed Dec 24, 2019
1 parent a322ff0 commit 4daf1d7
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 40 deletions.
16 changes: 0 additions & 16 deletions src/terminal/parser/OutputStateMachineEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,22 +171,6 @@ bool OutputStateMachineEngine::ActionEscDispatch(const wchar_t wch,
{
switch (wch)
{
case VTActionCodes::CUU_CursorUp:
success = _dispatch->CursorUp(1);
TermTelemetry::Instance().Log(TermTelemetry::Codes::CUU);
break;
case VTActionCodes::CUD_CursorDown:
success = _dispatch->CursorDown(1);
TermTelemetry::Instance().Log(TermTelemetry::Codes::CUD);
break;
case VTActionCodes::CUF_CursorForward:
success = _dispatch->CursorForward(1);
TermTelemetry::Instance().Log(TermTelemetry::Codes::CUF);
break;
case VTActionCodes::CUB_CursorBackward:
success = _dispatch->CursorBackward(1);
TermTelemetry::Instance().Log(TermTelemetry::Codes::CUB);
break;
case VTActionCodes::DECSC_CursorSave:
success = _dispatch->CursorSaveState();
TermTelemetry::Instance().Log(TermTelemetry::Codes::DECSC);
Expand Down
24 changes: 0 additions & 24 deletions src/terminal/parser/ut_parser/OutputEngineTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -966,30 +966,6 @@ class StateMachineExternalTest final
return true;
}

void TestEscCursorMovement(wchar_t const wchCommand,
const bool* const pfFlag,
StateMachine& mach,
StatefulDispatch& dispatch)
{
mach.ProcessCharacter(AsciiChars::ESC);
mach.ProcessCharacter(wchCommand);

VERIFY_IS_TRUE(*pfFlag);
VERIFY_ARE_EQUAL(dispatch._cursorDistance, 1u);
}

TEST_METHOD(TestEscCursorMovement)
{
auto dispatch = std::make_unique<StatefulDispatch>();
auto pDispatch = dispatch.get();
auto engine = std::make_unique<OutputStateMachineEngine>(std::move(dispatch));
StateMachine mach(std::move(engine));
TestEscCursorMovement(L'A', &pDispatch->_cursorUp, mach, *pDispatch);
TestEscCursorMovement(L'B', &pDispatch->_cursorDown, mach, *pDispatch);
TestEscCursorMovement(L'C', &pDispatch->_cursorForward, mach, *pDispatch);
TestEscCursorMovement(L'D', &pDispatch->_cursorBackward, mach, *pDispatch);
}

void InsertNumberToMachine(StateMachine* const pMachine, size_t number)
{
static const size_t cchBufferMax = 20;
Expand Down

0 comments on commit 4daf1d7

Please sign in to comment.