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

Add support for LNM (Line Feed/New Line Mode) #15167

Closed
j4james opened this issue Apr 11, 2023 · 2 comments · Fixed by #15261
Closed

Add support for LNM (Line Feed/New Line Mode) #15167

j4james opened this issue Apr 11, 2023 · 2 comments · Fixed by #15261
Labels
Area-VT Virtual Terminal sequence support In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase
Milestone

Comments

@j4james
Copy link
Collaborator

j4james commented Apr 11, 2023

Description of the new feature/enhancement

When LNM is set, the linefeed controls (LF, FF, and VT) will also trigger a carriage return. When reset they, they won't. This is essentially the inverse of our existing console mode, DISABLE_NEWLINE_AUTO_RETURN.

However, LNM has an additional effect on input. When set, the Return key generates both a carriage return and a line feed. When reset it only generates the carriage return.

It's quite widely implemented, but I don't think VTE supports it, which suggests it's probably not that widely used. However, it is a requirement for meeting VT level 1 conformance, which is why I would like for us to support it.

Proposed technical implementation details (optional)

I had originally thought we could map it directly to the DISABLE_NEWLINE_AUTO_RETURN mode, since we're already using that to determine how linefeed controls are interpreted. However, when a Windows console app has DISABLE_NEWLINE_AUTO_RETURN reset (which is the equivalent of LNM being set), we don't typically want the Return key to behave differently.

So my idea was this: We add a new input mode that specifically handles the LNM behavior for the Return key, which by default is disabled. And we treat the DISABLE_NEWLINE_AUTO_RETURN mode as an inverse alias for the LNM output behavior (as we already do).

When those two match, we're in a valid VT state, and we can use the LNM mode to toggle them both at the same time. But when they're out of sync (which is the default state for a Windows console app), we just act as if the LNM mode is not supported, i.e. we don't respond to any attempts to change it, and DECRQM reports the mode as unknown.

Does that seem like a reasonable approach to take?

@j4james j4james added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Apr 11, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Apr 11, 2023
@lhecker
Copy link
Member

lhecker commented Apr 13, 2023

Isn't LNM basically a superset of the DISABLE_NEWLINE_AUTO_RETURN functionality? If so, why can't we have LNM override the DISABLE_NEWLINE_AUTO_RETURN setting entirely instead of checking if they're in sync?

@j4james
Copy link
Collaborator Author

j4james commented Apr 13, 2023

The current default behavior for Windows console apps (i.e. when DISABLE_NEWLINE_AUTO_RETURN is reset) is to execute both a carriage and a line feed when LF is output, and to generate just CR when the Return key is pressed. This behavior does not match either of the LNM states.

When LNM is set, a Return key will generate both CR and LF, which a lot of console apps will register as a double key press. And when LNM is reset, you won't automatically get carriage returns when outputting *nix-style text content with \n line endings (I'm guessing that was the main reason the console added the auto-return functionality in the first place).

@zadjii-msft zadjii-msft added Product-Conhost For issues in the Console codebase Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Apr 17, 2023
@zadjii-msft zadjii-msft added this to the Backlog milestone Apr 17, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed Needs-Tag-Fix Doesn't match tag requirements labels Apr 17, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Apr 28, 2023
DHowett pushed a commit that referenced this issue May 12, 2023
This PR adds support for the ANSI Line Feed/New Line mode (`LNM`), which
determines whether outputting a linefeed control should also trigger a
carriage return, and whether the `Return` key should generate an `LF` in
addition to `CR`.

## Detailed Description of the Pull Request / Additional comments

In ConHost, there was already a console mode which handled the output
side of things, but I've now also added a `TerminalInput` mode that
controls the behavior of the `Return` key. When `LNM` is set, both the
output and input modes are enabled, and when reset, they're disabled.

If they're not already matching, then `LNM` has no effect, and will be
reported as unknown when queried. This is the typical state for legacy
console applications, which expect a linefeed to trigger a carriage
return, but wouldn't want the `Return` key generating both `CR`+`LF`.

As part of this PR, I've also refactored the `ITerminalApi` interface to
consolidate what I'm now calling the "system" modes: bracketed paste,
auto wrap, and the new line feed mode. This closes another gap between
Terminal and ConHost, so both auto wrap, and line feed mode will now be
supported for conpty pass through.

## Validation Steps Performed

I've added an `LNM` test that checks the escape sequence is triggering
both of the expected mode changes, and added an additional `DECRQM` test
covering the currently implemented standard modes: the new `LNM`, and
the existing `IRM` (which wasn't previously tested). I've also extended
the `DECRQM` private mode test to cover `DECAWM` and Bracketed Paste
(which we also weren't previously testing).

I've manually tested `LNM` in Vttest to confirm the keyboard is working
as expected.

Closes #15167
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label May 12, 2023
DHowett pushed a commit that referenced this issue May 15, 2023
This PR adds support for the ANSI Line Feed/New Line mode (`LNM`), which
determines whether outputting a linefeed control should also trigger a
carriage return, and whether the `Return` key should generate an `LF` in
addition to `CR`.

## Detailed Description of the Pull Request / Additional comments

In ConHost, there was already a console mode which handled the output
side of things, but I've now also added a `TerminalInput` mode that
controls the behavior of the `Return` key. When `LNM` is set, both the
output and input modes are enabled, and when reset, they're disabled.

If they're not already matching, then `LNM` has no effect, and will be
reported as unknown when queried. This is the typical state for legacy
console applications, which expect a linefeed to trigger a carriage
return, but wouldn't want the `Return` key generating both `CR`+`LF`.

As part of this PR, I've also refactored the `ITerminalApi` interface to
consolidate what I'm now calling the "system" modes: bracketed paste,
auto wrap, and the new line feed mode. This closes another gap between
Terminal and ConHost, so both auto wrap, and line feed mode will now be
supported for conpty pass through.

## Validation Steps Performed

I've added an `LNM` test that checks the escape sequence is triggering
both of the expected mode changes, and added an additional `DECRQM` test
covering the currently implemented standard modes: the new `LNM`, and
the existing `IRM` (which wasn't previously tested). I've also extended
the `DECRQM` private mode test to cover `DECAWM` and Bracketed Paste
(which we also weren't previously testing).

I've manually tested `LNM` in Vttest to confirm the keyboard is working
as expected.

Closes #15167

(cherry picked from commit 3d73721)
Service-Card-Id: 89180225
Service-Version: 1.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants