Skip to content

Conversation

blink1073
Copy link
Contributor

Xterm is actively maintained and is being used in Jupyterlab.

cf #104 (comment)

@minrk minrk added this to the 5.0 milestone Jul 5, 2016
"preact": "^4.5.1",
"preact-compat": "^1.7.0"
"preact-compat": "^1.7.0",
"xterm": "^0.33.0"
Copy link
Member

Choose a reason for hiding this comment

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

Just for my own understanding, why are there some dependencies in package.json and some in bower.json? Are these in package.json installed by npm? Are we trying to move away from bower?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a general shift in the community from bower to npm where possible. The one glaring place where this is not yet feasible is MathJax.

@minrk
Copy link
Member

minrk commented Jul 5, 2016

Woo!

We should look through the various open Issues pertaining to terminal and see which ones are closed by this. I know the terminal paste issue has been open for a long time.

And we should also make sure this works without webpack if we really are going to rollback the webpack change for the legacy pages (I looked into this for a while last week, and it looked like a lot of work).

@takluyver
Copy link
Member

Thanks @blink1073 :-)

@blink1073
Copy link
Contributor Author

If we lose WebPack we also lose npm modules. I am actively working on a way to get everything to work in harmony: jupyterlab/jupyterlab#225

@blink1073
Copy link
Contributor Author

It could be that the classic notebook will need to similarly expose a public API for v5+.

@takluyver
Copy link
Member

webpack is currently failing with ModuleNotFoundError: Module not found: Error: Cannot resolve module 'style-loader' in /tmp/pip-build-uaps7hmg/notebook/notebook/static/terminal/js

@blink1073
Copy link
Contributor Author

@takluyver, awesome! That means adding bail: true did what it intended. ;)

@minrk
Copy link
Member

minrk commented Jul 5, 2016

@blink1073 have you figured out how to get runtime require to return the actual exact module instance (forgive my Python terminology) when it was bundled up with webpack? This is the backward-compatibility bit we need to figure out if we are going to keep webpack for the legacy endpoints.

@blink1073
Copy link
Contributor Author

@minrk, each WebPack bundle will be an AMD module in the notebook, exporting the desired API.

@blink1073
Copy link
Contributor Author

I think I'm close to cracking this nut ;).

@minrk
Copy link
Member

minrk commented Jul 5, 2016

That's very exciting. Looking forward to the PR. Thanks for working on it.

@takluyver
Copy link
Member

It's now failing in some Python code that checks that the expected JS files are in place - this probably just needs to be updated.

      File "/tmp/pip-build-ow9jpojc/notebook/setupbase.py", line 220, in check_package_data
        assert os.path.exists(path), "Missing package data: %s" % path
    AssertionError: Missing package data: notebook/static/components/term.js/src/term.js

pjoin(components, "underscore", "underscore-min.js"),
pjoin(components, "moment", "moment.js"),
pjoin(components, "moment", "min", "moment.min.js"),
pjoin(components, "term.js", "src", "term.js"),
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to replace it with a check for xterm.js? Or does the bundling mean there isn't a separate file for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are explicit checks for bower modules, there is no need IMO to check for node_modules.

@takluyver
Copy link
Member

Thanks, let's see how we get on with this.

@takluyver takluyver merged commit 797227b into jupyter:master Jul 5, 2016
@yuvipanda
Copy link
Contributor

\o/ Thank you all! The bot users of Wikimedia love you :)

On Tue, Jul 5, 2016 at 11:07 PM, Thomas Kluyver notifications@github.com
wrote:

Merged #1591 #1591.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1591 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAB23iVHeBoMGnuBNyChw3ttyKyQ-lciks5qSsd2gaJpZM4JFKEt
.

Yuvi Panda T
http://yuvi.in/blog

@mogthesprog
Copy link

Thanks for this guys! Seriously.... so many analysts will stop hating me because of this fix.

@yuvipanda
Copy link
Contributor

I've kinda of ported this to run on top of 4.2.1 with #1613. Note that I haven't touched js in a long time - this works for me, but might not work for everyone!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants