Skip to content

Support all the newline operations #3189

Closed

Description

Description of the new feature/enhancement

VT terminals support at least five newline controls that I'm aware of. There are the three control characters: LF (linefeed), VT (vertical tab), and FF (form feed). And then there are also two escape sequences: NEL (Next Line) and IND (Index).

The three control characters are simply aliases for the same operation. When the New Line Mode is reset, they perform a linefeed by itself. When the New Line Mode is set, they perform a carriage return as well as the linefeed.

The NEL and IND sequences are not affected by the mode, though. The IND escape sequence always performs a linefeed by itself, with no carriage return. And the NEL escape sequence always performs a carriage return and linefeed together.

It may not seem as if there's much value in supporting all of these sequences, but they are required to pass the Vttest Test of cursor movements, so it would be nice to have them supported in the Windows console.

Proposed technical implementation details (optional)

I'd ignore the New Line Mode to start with, since that's not essential, and introduces a whole lot more complexity. And that means we could actually implement everything in the OutputStateMachineEngine, simply with calls to the ITermDispatch::Execute method, with either LF, or CR and LF, depending on what was needed.

That said, I think it would be nicer to add a dedicated LineFeed method to the ITermDispatch interface, probably with an enum parameter specifying the required type (i.e. with carriage return, without carriage return, or mode-dependent). Then we can always add a mode condition in there at a later stage if we want to support the New Line Mode.

It may even be worth adding a dedicated method in the ConGetSet interface for implementing the newline functionality directly, instead of going through WriteCharsLegacy. This requires a little more work, but has the advantage that it would not be influenced by the DISABLE_NEWLINE_AUTO_RETURN flag (which can produce the wrong behaviour when not set).

I should also mention there is an additional complication with implementing the IND sequence, which is issue #976. My preference would be to drop those cursor movement operations for now, in favor of getting IND working. And then we can always add them back when a proper VT52 mode is implemented (which is something I still have on the back burner). But it seems pointless keeping them without the rest of the VT52 support, and without the user explicitly requesting VT52 mode.

Activity

  1. added
    Issue-FeatureComplex enough to require an in depth planning process and actual budgeted, scheduled work.
    on Oct 14, 2019
  2. ghost added
    Needs-TriageIt's a new issue that the core contributor team needs to triage at the next triage meeting
    on Oct 14, 2019
  3. ghost added
    Needs-Tag-FixDoesn't match tag requirements
    on Oct 14, 2019
  4. DHowett-MSFT commented on Oct 14, 2019

    @DHowett-MSFT
    Contributor

    This'll be important as we nail down #780. Thanks!

  5. added
    Area-VTVirtual Terminal sequence support
    Issue-TaskIt's a feature request, but it doesn't really need a major design.
    Product-ConhostFor issues in the Console codebase
    and removed
    Issue-FeatureComplex enough to require an in depth planning process and actual budgeted, scheduled work.
    Needs-TriageIt's a new issue that the core contributor team needs to triage at the next triage meeting
    on Oct 14, 2019
  6. ghost removed
    Needs-Tag-FixDoesn't match tag requirements
    on Oct 14, 2019
  7. j4james commented on Oct 15, 2019

    @j4james
    CollaboratorAuthor

    This'll be important as we nail down #780.

    Yes! I was just thinking about this. If you're in VT mode, the control characters should all already be parsed out from the regular text, so it seems really pointless to then forward them on to the WriteCharsLegacy method which has to reparse the content all over again.

    If we got the OutputStateMachineEngine to handle all of the control characters itself, it should be a lot more efficient, and then the WriteCharsLegacy function would no longer need to worry about any special case handling for VT mode.

    It would also mean the PrintString handler could eventually be replaced by something much simpler than WriteCharsLegacy, since it could assume that the content was always printable text that doesn't require control character handling.

  8. j4james commented on Oct 16, 2019

    @j4james
    CollaboratorAuthor

    Dustin L. Howett (MSFT) (@DHowett-MSFT) Is it OK if I start putting together a PR for this? Or is it likely to conflict with the work that is already being done for #780?

  9. DHowett-MSFT commented on Oct 17, 2019

    @DHowett-MSFT
    Contributor

    James Holderness (@j4james) sure is! Michael Niksa (@miniksa) gave me his blessing to give to you, with apologies that he's been busy.

    Cryptically, he said "I only want the deferred one in WriteChars(...)", which I parsed as "the only type of newline WriteChars should know about after #780 is the deferred kind".

    😄

  10. j4james commented on Oct 17, 2019

    @j4james
    CollaboratorAuthor

    That makes sense. And I fully understand how busy you guys are, so please don't feel any pressure to deal with my issues. If I'm stepping on any toes with my PRs, or there's anything you don't like, don't hesitate to reject them. I'm on holiday at the moment, so I may be going a bit overboard with the PR submissions. 😀

  11. ghost added
    In-PRThis issue has a related PR
    on Oct 21, 2019
  12. ghost closed this as completedin #3271on Jan 15, 2020
  13. ghost added a commit that references this issue on Jan 15, 2020
    701b421
  14. ghost removed
    In-PRThis issue has a related PR
    on Jan 15, 2020
  15. ghost added
    Resolution-Fix-CommittedFix is checked in, but it might be 3-4 weeks until a release.
    on Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Area-VTVirtual Terminal sequence supportIssue-TaskIt's a feature request, but it doesn't really need a major design.Product-ConhostFor issues in the Console codebaseResolution-Fix-CommittedFix is checked in, but it might be 3-4 weeks until a release.

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions