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.

Sign up for GitHub

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

Jump to bottom

Copying text from the command window behaves differently than previous versions #1126

Closed
DHowett-MSFT opened this issue Jun 3, 2019 · 4 comments · Fixed by #2831
Closed

Copying text from the command window behaves differently than previous versions #1126

DHowett-MSFT opened this issue Jun 3, 2019 · 4 comments · Fixed by #2831
Assignees
@carlos-zamora
Labels
Area-Interaction Interacting with the vintage console window (as opposed to driving via API or hooks) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
20H1

Comments

@DHowett-MSFT
Copy link
Contributor

Ported from internal bug

When I select some text from a window using Vim.exe and paste to another window, the text is badly formatted.
Also, if double-click text from a command window, it copies/pastes the extra white-space at the end which was not the case in earlier releases.
I also see the problem when copy/pasting to an excel spreadsheel

Tried to repro: space bug does not repro, vim bug does since 19H1
@DHowett-MSFT DHowett-MSFT added Product-Conhost For issues in the Console codebase Area-Interaction Interacting with the vintage console window (as opposed to driving via API or hooks) Issue-Bug It either shouldn't be doing this or needs an investigation. labels Jun 3, 2019
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 3, 2019
@DHowett-MSFT DHowett-MSFT added Help Wanted We encourage anyone to jump in on these. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jun 3, 2019
@DHowett-MSFT
Copy link
Contributor Author

To clarify, this is not #1091 because 1091 is in Terminal and this is in Conhost

@zadjii-msft zadjii-msft added this to the 20H1 milestone Jun 11, 2019
@JeremyTBradshaw
Copy link

Just commenting to add that I see this behavior when it's PowerShell.exe in the terminal. Doesn't behave on the paste into other apps like it does with cmd.exe and PowerShell.exe natively. I assume it's the trailing whitespace on each line as the internal bug's description states.

@ghost
Copy link

ghost commented Jul 21, 2019

Copying text does not trim the trailing whitespace for me in WSL unless I hold down the Alt key while selecting. No issues with PowerShell or cmd.

@carlos-zamora carlos-zamora self-assigned this Sep 13, 2019
@carlos-zamora carlos-zamora removed the Help Wanted We encourage anyone to jump in on these. label Sep 13, 2019
@carlos-zamora carlos-zamora mentioned this issue Sep 20, 2019
Merged
5 tasks
@ghost ghost added the In-PR This issue has a related PR label Sep 20, 2019
DHowett-MSFT pushed a commit that referenced this issue Oct 1, 2019
@carlos-zamora @DHowett
make filling chars (and, thus, erase line/char) unset wrap (#2831)
4dd9f9c 
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

Now we're making this bool a std::optional to allow for a ternary state. This
allows for us to handle the following cases completely. Refer to the table
below:

,- current wrap value
|     ,- are we filling the last cell in the row?
|     |     ,- new wrap value
|     |     |     ,- comments
|--   |--   |--   |
| 0   | 0   | 0   |
| 0   | 1   | 0   |
| 0   | 1   | 1   | THIS CASE WAS HANDLED CORRECTLY
| 1   | 0   | 0   | THIS CASE WAS UNHANDLED
| 1   | 0   | 1   |
| 1   | 1   | 1   |

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 #1126
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Oct 1, 2019
DHowett-MSFT pushed a commit that referenced this issue Oct 3, 2019
@carlos-zamora @DHowett
make filling chars (and, thus, erase line/char) unset wrap (#2831)
f081a95 
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

Now we're making this bool a std::optional to allow for a ternary state. This
allows for us to handle the following cases completely. Refer to the table
below:

,- current wrap value
|     ,- are we filling the last cell in the row?
|     |     ,- new wrap value
|     |     |     ,- comments
|--   |--   |--   |
| 0   | 0   | 0   |
| 0   | 1   | 0   |
| 0   | 1   | 1   | THIS CASE WAS HANDLED CORRECTLY
| 1   | 0   | 0   | THIS CASE WAS UNHANDLED
| 1   | 0   | 1   |
| 1   | 1   | 1   |

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 #1126
@ghost
Copy link

ghost commented Oct 4, 2019

🎉This issue was addressed in #2831, which has now been successfully released as Windows Terminal Preview v0.5.2762.0.:tada:

Handy links:

@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 4, 2019
@DHowett-MSFT DHowett-MSFT added the Priority-2 A description (P2) label Oct 4, 2019
DHowett-MSFT pushed a commit that referenced this issue Oct 17, 2019
@carlos-zamora @DHowett
make filling chars (and, thus, erase line/char) unset wrap (#2831)
b512fcf 
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

Now we're making this bool a std::optional to allow for a ternary state. This
allows for us to handle the following cases completely. Refer to the table
below:

,- current wrap value
|     ,- are we filling the last cell in the row?
|     |     ,- new wrap value
|     |     |     ,- comments
|--   |--   |--   |
| 0   | 0   | 0   |
| 0   | 1   | 0   |
| 0   | 1   | 1   | THIS CASE WAS HANDLED CORRECTLY
| 1   | 0   | 0   | THIS CASE WAS UNHANDLED
| 1   | 0   | 1   |
| 1   | 1   | 1   |

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 #1126

(cherry picked from commit 4dd9f9c)
DHowett-MSFT pushed a commit that referenced this issue Oct 17, 2019
@DHowett
Merged PR 3896152: Merge selected github changes up to dd2fbef
62f5299 
- 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
Assignees

@carlos-zamora carlos-zamora

Labels
Area-Interaction Interacting with the vintage console window (as opposed to driving via API or hooks) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
20H1
Development

Successfully merging a pull request may close this issue.

make filling chars (and, thus, erase line/char) unset wrap microsoft/terminal
4 participants
@carlos-zamora @DHowett-MSFT @JeremyTBradshaw @zadjii-msft