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

Copy from terminal doesn't work [Windows/Insiders] #31902

Closed
warpdesign opened this issue Aug 2, 2017 · 9 comments · Fixed by #37290
Closed

Copy from terminal doesn't work [Windows/Insiders] #31902

warpdesign opened this issue Aug 2, 2017 · 9 comments · Fixed by #37290
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug help wanted Issues identified as good community contribution opportunities terminal General terminal issues that don't fall under another label upstream Issue identified as 'upstream' component related (exists outside of VS Code)
Milestone

Comments

@warpdesign
Copy link
Contributor

warpdesign commented Aug 2, 2017

Steps to Reproduce:

  1. open the integrated terminal and have it output some text so that scrolling happens
  2. scroll up a little and select some text (see screenshot1)
  3. press ctrl+c
  4. press ctrl+v

Screenshot1:
copypaste1

What happens: terminal scrolls to the bottom and the text that was previously in the clipboard (here that's not what I copied) is pasted, instead of the one I just copied using ctrl+c

Now, if I don't end the selection (ie: release the left mouse button) before reaching the left side of the screen, it will work:

copypaste2

I am using the latest insider build (211ffdf) on Windows 10 (64bit).

I tried and could not reproduce the problem on a standard cmd prompt.

@vscodebot vscodebot bot added the terminal General terminal issues that don't fall under another label label Aug 2, 2017
@Tyriar
Copy link
Member

Tyriar commented Aug 2, 2017

@warpdesign to clarify, it's only happening when you release the mouse outside of the terminal?

@Tyriar Tyriar added the info-needed Issue requires more information from poster label Aug 2, 2017
@warpdesign
Copy link
Contributor Author

@Tyriar Yes, releasing the mouse outside of <div class="terminal-wrapper active></div> triggers the bug.

As long as I release the mouse inside this div it works as expected.

@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug and removed info-needed Issue requires more information from poster labels Aug 21, 2017
@Tyriar Tyriar added this to the Backlog milestone Aug 21, 2017
@warpdesign
Copy link
Contributor Author

Any idea when this will be fixed? It still happens in latest insiders build and is really annoying.

Does it still need the needs more info tag?

@Tyriar
Copy link
Member

Tyriar commented Oct 30, 2017

Will be fixed when time allows, I'm pretty flooded with other issues atm though. It's open to PRs though. The root cause will likely be in https://github.com/sourcelair/xterm.js

@Tyriar Tyriar added help wanted Issues identified as good community contribution opportunities upstream Issue identified as 'upstream' component related (exists outside of VS Code) labels Oct 30, 2017
@warpdesign
Copy link
Contributor Author

warpdesign commented Oct 31, 2017

I played a little with xterm.js but I cannot reproduce the problem. When selecting text, even if I release the mouse outside of the terminal, term.getSelection() returns the correct selection.

@warpdesign
Copy link
Contributor Author

warpdesign commented Oct 31, 2017

It seems like this action is not ran when the mouse is released outside of the terminal div and ctrl+c is pressed:

actionRegistry.registerWorkbenchAction(new SyncActionDescriptor(CopyTerminalSelectionAction, CopyTerminalSelectionAction.ID, CopyTerminalSelectionAction.LABEL, {
 	primary: KeyMod.CtrlCmd | KeyCode.KEY_C,
 	linux: { primary: KeyMod.CtrlCmd | KeyMod.Shift | KeyCode.KEY_C }
 }, ContextKeyExpr.and(KEYBINDING_CONTEXT_TERMINAL_TEXT_SELECTED, KEYBINDING_CONTEXT_TERMINAL_FOCUS)), 'Terminal: Copy Selection', category);

@warpdesign
Copy link
Contributor Author

warpdesign commented Oct 31, 2017

One more thing I noticed, if I do the following:

  1. select some text and release the mouse outside of the terminal div
  2. hide the terminal panel (with ctrl+ù)
  3. show the terminal panel (with ctrl+ù again)
  4. press ctrl+c
  5. press ctrl+v

The text is pasted as expected: maybe there is something wrong with KEYBINDING_CONTEXT_TERMINAL_FOCUS: it seems terminal kind of loses focus when the mouse is being released outside of the terminal div which causes the action not to be triggered.

@warpdesign
Copy link
Contributor Author

warpdesign commented Oct 31, 2017

Actually, the problem is with KEYBINDING_CONTEXT_TERMINAL_TEXT_SELECTED: commenting it, copy/paste works after releasing the mouse outside of the terminal area.

@warpdesign
Copy link
Contributor Author

warpdesign commented Oct 31, 2017

I think the problem is in terminalInstance.ts:

this._instanceDisposables.push(dom.addDisposableListener(this._xterm.element, 'mouseup', (event: KeyboardEvent) => {
	// Wait until mouseup has propagated through the DOM before
	// evaluating the new selection state.
	setTimeout(() => this._refreshSelectionContextKey(), 0);
}));

The mouseup event isn't triggered if the mouse is released outside of the xterm element, so the selection isn't refreshed in this case, and the CopyTerminalSelectionAction is not executed.

warpdesign added a commit to warpdesign/vscode that referenced this issue Oct 31, 2017
Tyriar added a commit that referenced this issue Dec 27, 2017
copy from terminal doesn't work #31902
@vscodebot vscodebot bot locked and limited conversation to collaborators Feb 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug help wanted Issues identified as good community contribution opportunities terminal General terminal issues that don't fall under another label upstream Issue identified as 'upstream' component related (exists outside of VS Code)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants