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

fix handling of length in BufferLine.resize #1740

Merged
merged 3 commits into from
Oct 20, 2018

Conversation

jerch
Copy link
Member

@jerch jerch commented Oct 9, 2018

Currently we do not trim buffer lines upon resizing. BufferLine.resize wrongly adjusted .length to the new shorter cols value, which makes the invisible content beyond terminal.cols non accessible for consumers like Buffer.translateBufferLineToString. This PR restores the behavior of v3.7.

@jerch jerch added the type/bug Something is misbehaving label Oct 10, 2018
@Tyriar
Copy link
Member

Tyriar commented Oct 19, 2018

@jerch I can see lines being chopped off in VS Code, but when I try shrink and restore on xterm.js master demo it seems to keep the content? What's the test you were using the test this?

@jerch
Copy link
Member Author

jerch commented Oct 19, 2018

@Tyriar You can test it in the demo with the linkifier:

Put some longish URL in the terminal, maybe with some parameters, shrink/enlarge/shrink it so the URL matcher would still see it as a valid URL -- boom, the parameters are gone (matches wrongly the shorter one). Im not even sure what the right behavior here, tbh I like the WYSIWYG idea with terminals more than the hidden stuff idea, but you told me that vscode does some selection based things for tasks relying on that behavior, therefore this PR restores that behavior.

@Tyriar Tyriar added this to the 3.9.0 milestone Oct 20, 2018
@Tyriar Tyriar merged commit 00c1211 into xtermjs:master Oct 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is misbehaving
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants