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

Passthrough BEL in conpty #2990

Merged
merged 1 commit into from
Sep 30, 2019
Merged

Passthrough BEL in conpty #2990

merged 1 commit into from
Sep 30, 2019

Conversation

zadjii-msft
Copy link
Member

Summary of the Pull Request

Passthrough the 🔔 in conpty mode

PR Checklist

Detailed Description of the Pull Request / Additional comments

                        🔔
             🔔🔔🔔🔔🔔
        🔔🔔🔔🔔🔔🔔🔔
        🔔🔔🔔🔔🔔🔔🔔
        🔔🔔🔔🔔🔔🔔🔔
        🔔🔔🔔🔔🔔🔔🔔
  🔔🔔🔔🔔🔔🔔🔔🔔🔔
                        🔔

Validation Steps Performed

@zadjii-msft zadjii-msft added Area-VT Virtual Terminal sequence support Product-Conpty For console issues specifically related to conpty labels Sep 30, 2019
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

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

Interesting... it’s not VT, so it doesn’t seem like it should go in the VT engine, but I can’t come up with a good reason why that makes me undercomfortable...

@zadjii-msft
Copy link
Member Author

@DHowett-MSFT I didn't add it to the VT renderer, because then all the renderers would need to have an additional "PaintBell" function that they use to call back into the host (a la PaintTitle for the GDI/DX engine) - that seemed a tad bit like overkill, when we could just catch it right here and send it through.

@DHowett-MSFT
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DHowett-MSFT DHowett-MSFT added the Needs-Second It's a PR that needs another sign-off label Sep 30, 2019
@ghost ghost requested review from miniksa and carlos-zamora September 30, 2019 22:33
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

10/10 on presentation

@DHowett-MSFT DHowett-MSFT merged commit 6831120 into master Sep 30, 2019
@DHowett-MSFT DHowett-MSFT deleted the dev/migrie/b/2952-🔔 branch September 30, 2019 22:38
DHowett-MSFT pushed a commit that referenced this pull request Oct 3, 2019
🔔
[insert Chorus of the Bells here -DHowett]

Fixes #2952.
@ghost
Copy link

ghost commented Oct 4, 2019

🎉Windows Terminal Preview v0.5.2762.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request Oct 4, 2019
DHowett-MSFT pushed a commit that referenced this pull request Oct 17, 2019
🔔
[insert Chorus of the Bells here -DHowett]

Fixes #2952.

(cherry picked from commit 6831120)
DHowett-MSFT pushed a commit that referenced this pull request Oct 17, 2019
- Fix snapping to the cursor in "Terminal Scrolling" mode (GH-2705)

fixes GH-1222

  PSReadline calls SetConsoleCursorPosition on each character. We try to snap.

  However, we'd only ever do this with the visible viewport, the viewport
  defined by `SCREEN_INFORMATION::_viewport`. When there's a virtual viewport in
  Terminal Scrolling mode, we actually need to snap the virtual viewport, so
  that this behavior looks more regular.

(cherry picked from commit 6f4b98a)
- Passthrough BEL in conpty (GH-2990)

fixes GH-2952.

(cherry picked from commit 6831120)
- make filling chars (and, thus, erase line/char) unset wrap (GH-2831)

EraseInLine calls `FillConsoleOutputCharacterW()`. In filling the row with
chars, we were setting the wrap flag. We need to specifically not do this on
ANY _FILL_ operation. Now a fill operation UNSETS the wrap flag if we fill to
the end of the line.

Originally, we had a boolean `setWrap` that would mean...
- **true**: if writing to the end of the row, SET the wrap value to true
- **false**: if writing to the end of the row, DON'T CHANGE the wrap value

,- current wrap value
| ,- fill last cell?
| | ,- new wrap value
| | | ,- comments
|-|-|-|
|1|0|0| THIS CASE WAS UNHANDLED

To handle that special case (1-0-0), we need to UNSET the wrap. So now, we have
~setWrap~ `wrap` mean the following:
- **true**: if writing to the end of the row, SET the wrap value to TRUE
- **false**: if writing to the end of the row, SET the wrap value to FALSE
- **nullopt**: leave the wrap value as it is

Closes GH-1126

(cherry picked from commit 4dd9f9c)
- When reverse indexing, preserve RGB/256 attributes (GH-2987)

(cherry picked from commit 847d6b5)
- do not allow CUU and CUD to move "across" margins. (GH-2996)

If we're moving the cursor up, its vertical movement should be stopped
at the top margin.
Similarly, this applies to moving down and the bottom margin.

Fixes GH-2929.

(cherry picked from commit 0ce08af)
- Prevent the v1 propsheet from zeroing colors, causing black text on black background. (GH-2651)

  fixes GH-2319

(cherry picked from commit b97db63)
- Render the cursor state to VT (GH-2829)

(cherry picked from commit a9f3849)
- Don't put NUL in the buffer in VT mode (GH-3015)

(cherry picked from commit a82d6b8)
- Return to ground when we flush the last char (GH-2823)

The InputStateMachineEngine was incorrectly not returning to the ground
state after flushing the last sequence. That means that something like
alt+backspace would leave us in the Escape state, not the ground state.
This makes sure we return to ground.

Additionally removes the "Parser.UnitTests-common.vcxproj" file, which
was originally used for a theoretical time when we only open-sourced the
parser. It's unnecessary now, and we can get rid of it.

Also includes a small patch to bcz.cmd, to make sure bx works with
projects with a space in their name.

(cherry picked from commit 53c81a0)
- Add support for passing through extended text attributes, like… (GH-2917)
 ...

Related work items: #23678919, #23678920, #23731910, #23731911, #23731912, #23731913, #23731914, #23731915, #23731916, #23731917, #23731918
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 Needs-Second It's a PR that needs another sign-off Product-Conpty For console issues specifically related to conpty
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bell does not work or does not exist
3 participants