-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Convert xterm.js to TypeScript #839
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Multiple declarations per line makes it more difficult to read what exactly is declared and generally makes code more verbose and less likely to fill in types
This brings in proper support to resize both buffers (xtermjs#510) and fixes an exception that was caused by wrongfully not clearing the normal buffer when resizing while the alt buffer is active. There was an obscure bug in this that could have caused some great confusion later on; When switching to the alt buffer, a hard terminal reset was performed which tried to retain the buffers. However, because buffers was initialized in the Terminal constructor to a new BufferSet, the Terminal.buffer convenience pointer was pointing at a stale alt buffer which was the one actually being used, not Terminal.buffers.alt. Fixes xtermjs#842 Fixes xtermjs#510
The alt buffer is now cleared immediated after activating the normal buffer and is filled when switching to it. The tests were failing because the alt buffer wasn't being cleared properly with the previous solution.
setOption('scrollback', 0) no longer returns false, it just warns.
I pulled #843 into this for the v3 branch while it's still fresh in my head. |
parisk
approved these changes
Aug 6, 2017
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome.
Clean and tidy, despite its size 😄!
The code looks great and the demo works great as well.
LGTM 🚀.
😍 💟 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Finally it's happening 😄. There was quite a bit of code/variables removed while doing this.
Fixes #335
Major changes:
one-variable-per-declaration
tslint ruleInputHandler._terminal
as anIInputHandlingTerminal
which keeps all ofInputHandler
's need in that interface, without messing withITerminal
.Cool little feature I added with the types: strongly typing the strings for
getOption
/setOption
: