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

CHE-3210: Integration xterm.js instead of xterm. #3943

Merged
merged 1 commit into from
Feb 15, 2017
Merged

CHE-3210: Integration xterm.js instead of xterm. #3943

merged 1 commit into from
Feb 15, 2017

Conversation

AndrienkoAleksandr
Copy link
Contributor

@AndrienkoAleksandr AndrienkoAleksandr commented Jan 29, 2017 •
edited by JamesDrummond
Loading

Signed-off-by: Aleksandr Andrienko [email protected]

What does this PR do?

Integrate xterm.js (version 2.2.3) widget instead of xterm. Source code of xtermjs: https://github.com/sourcelair/xterm.js. Site: http://xtermjs.org/

Changelog and Release Note Information**

Changelog

Integrate xterm.js (version 2.2.3) widget instead of xterm.

Release Notes

We have replaced our terminal implementation with xterm.js, a Web terminal that is well-supported by Microsoft and Sourcelair. This update resolves a number of outstanding bugs and brings forward new terminal :

  1. Text-based application support: Use terminal to work with applications like bash and git
  2. Curses-based application support: Use terminal to work with applications like vim and tmux
  3. Mouse events support, which lets extension developers captural click and scroll events passing them to back-end controlling processes
  4. Chinese, Japanese, and Korean character support
  5. Insert international characters using IME input with your keyboard
  6. Hotkeys for copy text: "Ctrl/Insert" and "Shift/Insert"
  7. Horizontal scroll bar

The xTerm project and its various collaborators keep a rich set of terminal documentation. We are really pleased with the richness of the features, stability of the terminal, and the ecosystem commitment. We have and will continue to give contributions back as we make additional improvements.

What issues does this PR fix or reference?

#3529

@azatsarynnyy @vparfonov @olexii4 @akurinnoy @skabashnyuk review please.

@TylerJewell
Copy link

Missing Changelog, Release Notes, and Docs PR. We can see examples of what need to be done with this PR: #3940

Also - we should identify the PM that will review the release notes and docs PRs to verify completeness. I recommend @JamesDrummond for this.

@JamesDrummond - as part of this review, we need to go through any old videos on Che site and any docs where we discuss terminal and make sure we are providing up to date information.

@AndrienkoAleksandr - this is very impressive work.

@codenvy-ci
Copy link

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1803/

@skabashnyuk
Copy link
Contributor

I suggest instead of copy entire xterm.js source code in our codebase inherit sources as maven dependency

<dependency>
    <groupId>org.webjars.npm</groupId>
    <artifactId>jackfan.us.kg-sourcelair-xterm-js</artifactId>
    <version>2.2.3</version>
</dependency>

and apply patches if needed. It will:

  • simplify version upgrades
  • reduce our codebase
  • easy to see how we differ from upstream project

@AndrienkoAleksandr
Copy link
Contributor Author

qa found bug #3947 . I need fix it. I analyzed this issue: this is bug xterm.js parser. Midnight Commander send restore cursor sequence "8l", but xterm.js didn't save valid cursor position.

@AndrienkoAleksandr
Copy link
Contributor Author

AndrienkoAleksandr commented Jan 31, 2017 •
edited
Loading

And we need fix (reproduces seldom) second bug: #3958

@JamesDrummond
Copy link
Contributor

JamesDrummond commented Jan 31, 2017 •
edited
Loading

@AndrienkoAleksandr Will this terminal have a "timeout" period? Currently the terminal after a period of inactivity will become unresponsive. I would like there to be no "timeout" period.

@codenvy-ci
Copy link

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1861/

@AndrienkoAleksandr
Copy link
Contributor Author

ci-build

@codenvy-ci
Copy link

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1879/

@codenvy-ci
Copy link

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1907/

@codenvy-ci
Copy link

Build # 1916 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/1916/ to view the results.

@codenvy-ci
Copy link

Build # 1918 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/1918/ to view the results.

@AndrienkoAleksandr AndrienkoAleksandr force-pushed the CHE-3210 branch 2 times, most recently from ddd6f31 to 149075a Compare February 9, 2017 12:02
@AndrienkoAleksandr
Copy link
Contributor Author

ci-build

@codenvy-ci
Copy link

Build # 1921 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/1921/ to view the results.

@codenvy-ci
Copy link

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1934/

@TylerJewell TylerJewell mentioned this pull request Feb 10, 2017
Closed
@TylerJewell TylerJewell added the kind/enhancement A feature request - must adhere to the feature request template. label Feb 10, 2017
@TylerJewell
Copy link

Updated release notes. I think we should merge this in time for 5.3. The outstanding bugs seem like P2 bugs, and acceptable issues. We can include advisory in the release notes for any known issues. But @AndrienkoAleksandr @vkuznyetsov - let's plan to merge on Monday.

@codenvy-ci
Copy link

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1953/

@AndrienkoAleksandr AndrienkoAleksandr force-pushed the CHE-3210 branch 2 times, most recently from de66ce2 to e3eb567 Compare February 13, 2017 19:57
@codenvy-ci
Copy link

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1961/

@codenvy-ci
Copy link

Build # 1963 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/1963/ to view the results.

@bmicklea
Copy link

@AndrienkoAleksandr and @vkuznyetsov - this needs to be merged as part of 5.3. I've added milestone and blocker labels.

@bmicklea bmicklea added this to the 5.3.0 milestone Feb 14, 2017
@bmicklea bmicklea added the severity/blocker Causes system to crash and be non-recoverable or prevents Che developers from working on Che code. label Feb 14, 2017
@AndrienkoAleksandr AndrienkoAleksandr changed the title [WIP] CHE-3210: Integration xterm.js instead of xterm. CHE-3210: Integration xterm.js instead of xterm. Feb 15, 2017
skabashnyuk
skabashnyuk reviewed Feb 15, 2017
View reviewed changes
Copy link
Contributor

@skabashnyuk skabashnyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure the you adding third-party files that was part of CQ.

@AndrienkoAleksandr
Integrate xterm.js (version 2.2.3) widget instead of xterm.
4b27212 
Changelog:
Delete old xterm view.
Move requirejs from plugin Orion to the plugin RequireJs.
Create new plugin with xterm.js.
Mavenize xterm.js build, use for build gulp tasks instead of bash scripts.
Rework GWT code for integration xterm.js.
Fix css styles for integration with GWT css resources and using Eclipse CHE white and dark themes.
Rework terminal resize mechanizm.

Signed-off-by: Aleksandr Andrienko <[email protected]>
@AndrienkoAleksandr
Copy link
Contributor Author

ci-build

@JamesDrummond
Copy link
Contributor

Please provide status label.

@AndrienkoAleksandr AndrienkoAleksandr merged commit 830cc34 into master Feb 15, 2017
@AndrienkoAleksandr AndrienkoAleksandr deleted the CHE-3210 branch February 15, 2017 16:46
@codenvy-ci
Copy link

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/1981/

@JamesDrummond JamesDrummond mentioned this pull request Feb 17, 2017
Closed
9 tasks
JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
@AndrienkoAleksandr
Integrate xterm.js (version 2.2.3) widget instead of xterm. (eclipse-…
3a492a4 
…che#3943)

Changelog:
Delete old xterm view.
Move requirejs from plugin Orion to the plugin RequireJs.
Create new plugin with xterm.js.
Mavenize xterm.js build, use for build gulp tasks instead of bash scripts.
Rework GWT code for integration xterm.js.
Fix css styles for integration with GWT css resources and using Eclipse CHE white and dark themes.
Rework terminal resize mechanizm.

Signed-off-by: Aleksandr Andrienko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skabashnyuk skabashnyuk skabashnyuk left review comments

@vparfonov vparfonov vparfonov approved these changes

Assignees
No one assigned
Labels
kind/enhancement A feature request - must adhere to the feature request template. severity/blocker Causes system to crash and be non-recoverable or prevents Che developers from working on Che code.
Projects
None yet
Milestone
5.3.0
Development

Successfully merging this pull request may close these issues.
7 participants
@AndrienkoAleksandr @TylerJewell @codenvy-ci @skabashnyuk @JamesDrummond @bmicklea @vparfonov