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

Integrated terminal: Selection not on the same position as displayed text when text is bold (causing copying more/different text than what appears selected) #29402

Closed
CherryDT opened this issue Jun 25, 2017 · 11 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug help wanted Issues identified as good community contribution opportunities terminal Integrated terminal issues upstream Issue identified as 'upstream' component related (exists outside of VS Code) verified Verification succeeded
Milestone

Comments

@CherryDT
Copy link

CherryDT commented Jun 25, 2017

Version 1.14.0-insider
Commit 779a7df
Date 2017-06-23T05:59:57.891Z
Shell 1.6.6
Renderer 56.0.2924.87
Node 7.4.0

image

EDIT: It is worse than that, because I now realized that what has been copied is this:

https://suggest.unbubble.eu/?q={searchTerms}",

Note the two extra characters at the end which I didn't intend to copy.

@vscodebot vscodebot bot added bug Issue identified by VS Code Team member as probable bug terminal Integrated terminal issues labels Jun 25, 2017
@CherryDT CherryDT changed the title Start of selection in integrated terminal can confusingly overlap the previous character Start of selection in integrated terminal can confusingly overlap the previous character, and more characters at the end are copied than I selected Jun 25, 2017
@Tyriar
Copy link
Member

Tyriar commented Jun 25, 2017

@CherryDT what font is that and what OS/version are you on?

@Tyriar Tyriar added this to the June 2017 milestone Jun 25, 2017
@CherryDT
Copy link
Author

CherryDT commented Jun 25, 2017

@Tyriar Windows 10 Build 15063, and this is Ubuntu Mono - sorry, I forgot that the font was non-standard.

In this case it's also bold (this is actually another bug I yet have to open - it exists for a long time, that after certain - usually colored - output, everything else becomes bold; for example using Git bash and then starting node REPL and then entering something (e.g. 123) and pressing Enter would trigger it)

@Tyriar
Copy link
Member

Tyriar commented Jun 25, 2017

@CherryDT I can't seem to repro. Some follow ups:

  • What's in your settings.json?
  • Does it repro on all lines there? Can you paste in the text that you can reproduce it with?
  • Does it happen consistently when you restart VS Code?
  • Does the problem fix itself when you resize the window?

@CherryDT
Copy link
Author

CherryDT commented Jun 26, 2017

I realized now that the issue is actually in the rendering of the bold font.

image

As you can see here, as soon as the font is bold (the reason for which is another bug, #29439), it doesn't fit into the grid correctly anymore. And then the selection uses a different grid than the text you are trying to select (and you end up copying more than you intend to).

My settings.json file is:

// Place your settings in this file to overwrite the default settings
{
	"git.confirmSync": false,
    "editor.renderWhitespace": "boundary",
	"editor.insertSpaces": false,
	"editor.fontFamily": "Ubuntu Mono",
	"editor.wrappingIndent": "indent",
	"editor.renderIndentGuides": true,
	"window.reopenWindows": "all",
	"files.eol": "\n",
	"explorer.openEditors.visible": 10,
	"css.lint.ieHack": "warning",
	"scss.lint.ieHack": "warning",
	"less.lint.ieHack": "warning",
	"javascript.format.insertSpaceAfterKeywordsInControlFlowStatements": false,
	"javascript.format.insertSpaceAfterFunctionKeywordForAnonymousFunctions": false,
	"typescript.format.insertSpaceAfterKeywordsInControlFlowStatements": false,
	"typescript.format.insertSpaceAfterFunctionKeywordForAnonymousFunctions": false,
	"extensions.autoUpdate": true,
	"terminal.integrated.shell.windows": "C:\\Windows\\sysnative\\bash.exe",
	"terminal.integrated.shellArgs.windows": ["--login"],
	"telemetry.enableTelemetry": false,
	"ecdc.collectTelemetry": false,
	"php.validate.executablePath": "C:\\Users\\david\\WTServer\\Bin\\php.bat",
	"terminal.integrated.commandsToSkipShell": [
		"editor.action.toggleTabFocusMode",
		"workbench.action.debug.continue",
		"workbench.action.debug.restart",
		"workbench.action.debug.run",
		"workbench.action.debug.start",
		"workbench.action.debug.stop",
		"workbench.action.openNextRecentlyUsedEditorInGroup",
		"workbench.action.openPreviousRecentlyUsedEditorInGroup",
		/*"workbench.action.quickOpen",*/
		"workbench.action.showCommands",
		"workbench.action.terminal.clear",
		"workbench.action.terminal.copySelection",
		"workbench.action.terminal.focus",
		"workbench.action.terminal.focusNext",
		"workbench.action.terminal.focusPrevious",
		"workbench.action.terminal.kill",
		"workbench.action.terminal.new",
		"workbench.action.terminal.paste",
		"workbench.action.terminal.runSelectedText",
		"workbench.action.terminal.scrollDown",
		"workbench.action.terminal.scrollDownPage",
		"workbench.action.terminal.scrollToBottom",
		"workbench.action.terminal.scrollToTop",
		"workbench.action.terminal.scrollUp",
		"workbench.action.terminal.scrollUpPage",
		"workbench.action.terminal.toggleTerminal"
	],
	"files.associations": {
		"*.tag": "htmltag"
	}
	,
	"sync.gist": "",
	"sync.lastUpload": "",
	"sync.autoDownload": false,
	"sync.autoUpload": false,
	"sync.lastDownload": "",
	"sync.version": 262,
	"sync.showSummary": true,
	"sync.forceDownload": false,
	"window.zoomLevel": 0,
	"sync.anonymousGist": false,
	"editor.wordWrap": "on",
	"sync.host": "",
	"sync.pathPrefix": "",
	"editor.snippetSuggestions": "top",
	"standard.enable": false,
	"editor.formatOnPaste": false
}

It happens in all lines as long as the font is bold. It also happens after restarting. It doesn't fix itself on resizing.

@CherryDT CherryDT changed the title Start of selection in integrated terminal can confusingly overlap the previous character, and more characters at the end are copied than I selected When text is bold: Start of selection in integrated terminal can confusingly overlap the previous character, and more characters at the end are copied than I selected Jun 26, 2017
@CherryDT CherryDT changed the title When text is bold: Start of selection in integrated terminal can confusingly overlap the previous character, and more characters at the end are copied than I selected Integrated terminal: Selection not on the same position as displayed text when text is bold (causing copying more/different text than what appears selected) Jun 26, 2017
@Tyriar
Copy link
Member

Tyriar commented Jun 26, 2017

@CherryDT bold rendering is meant to be disabled if the widths are different https://github.com/sourcelair/xterm.js/blob/f95bc64971c577bba2cf375c2333e570ac6ef5d5/src/Renderer.ts#L211

We test "hello world" to check the width, maybe those characters are find but the numbers/punctuation chars differ in width?

@CherryDT
Copy link
Author

CherryDT commented Jun 26, 2017

Hm, no, even hello world appears different.
image

@Tyriar
Copy link
Member

Tyriar commented Jun 26, 2017

@CherryDT can you inspect the row via devtools (Help > Toggle Developer Tools) and verify that the xterm-bold class is added?

@Tyriar Tyriar modified the milestones: July 2017, June 2017 Jun 26, 2017
@CherryDT
Copy link
Author

CherryDT commented Jun 26, 2017

Yes it is.
image

I tried debugging the renderer, and I noticed that even though brokenBold is false, manually invoking checkBrokenBold gives me true. This makes me wonder why it yields false during initialization. How can I open the debugger early enough to break there? It seems to me like the interesting part is already over when I open the devtools after restarting VSCode.

(Maybe the custom font is applied only after this test has already been passed?)

EDIT: Got it, I can use Ctrl+R in devtools.

I think I found the reason for the issue. During the comparison, the size of both elements you test is zero:
image
The reason is that the terminal element you use for testing isn't in the DOM tree yet (terminal.parentNode.parentNode is null).
I tried attaching it manually to the element with the font styles on it, like this:
image
But then the sizes were still zero, because the element was hidden and therefore not rendered, i.e. without size calculation taking place either. Only after also removing the display: none;, I finally got the expected result true from checkBoldBroken.

@Tyriar Tyriar added the upstream Issue identified as 'upstream' component related (exists outside of VS Code) label Jun 26, 2017
@Tyriar
Copy link
Member

Tyriar commented Jun 26, 2017

@CherryDT created xtermjs/xterm.js#730 to follow this up. As a workaround you should just use a different font that is the same width. When xtermjs/xterm.js#730 is handled you just won't be getting any bold at all.

@Tyriar
Copy link
Member

Tyriar commented Jun 26, 2017

@CherryDT you can also disable bold completely using "terminal.integrated.enableBold": false if you want to stick with that font.

@Tyriar Tyriar added the help wanted Issues identified as good community contribution opportunities label Jul 7, 2017
@Tyriar Tyriar modified the milestones: Backlog, July 2017 Jul 7, 2017
@Tyriar
Copy link
Member

Tyriar commented Oct 20, 2017

This should have been fixed with the terminal rendering improvements in v1.17. Assigning to October for verification.

@Tyriar Tyriar closed this as completed Oct 20, 2017
@Tyriar Tyriar modified the milestones: Backlog, October 2017 Oct 20, 2017
@aeschli aeschli added the verified Verification succeeded label Nov 3, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Dec 4, 2017
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 Integrated terminal issues upstream Issue identified as 'upstream' component related (exists outside of VS Code) verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants