Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Switch from pty.js to node.pty with node-pty-prebuilt #551

Closed
wants to merge 1 commit into from

Conversation

ashthespy
Copy link

Follow up to #547

Tested working on Win10 and Atom 1.27.2 (chrome:58.0.3029.110, electron:1.7.15)

@ivankravets
Copy link
Member

It does not work for me on Mac:

The module 'platformio-atom-ide-terminal/origin/node_modules/node-pty-prebuilt/build/Release/pty.node'
was compiled against a different Node.js version using
NODE_MODULE_VERSION 57. This version of Node.js requires
NODE_MODULE_VERSION 54. Please try re-compiling or re-installing
the module (for instance, using `npm rebuild` or`npm install`). Error: The module 'platformio-atom-ide-terminal/origin/node_modules/node-pty-prebuilt/build/Release/pty.node'
was compiled against a different Node.js version using
NODE_MODULE_VERSION 57. This version of Node.js requires
NODE_MODULE_VERSION 54. Please try re-compiling or re-installing
the module (for instance, using `npm rebuild` or`npm install`).

Are you sure that it works for you WITHOUT COMPILER?

/cc @daviwil

@ashthespy
Copy link
Author

@ivankravets positive I don't compile anything - just installed it on a vanilla Win10 machine.
What does apm install --verbose have to say?

@ivankravets
Copy link
Member

What does apm install --verbose have to say?

It tries to build node-pty from source files. I don't have Xcode on my machine.

@ashthespy
Copy link
Author

Strange - given node-pty-prebuilt-v0.7.3-electron-v54-darwin-x64 exists.

@ivankravets
Copy link
Member

Let's wait when someone will test too.

@the-j0k3r
Copy link
Collaborator

the-j0k3r commented Jun 22, 2018

So were waiting for someone to test this on macOS? Currently pio is dead with atom 1.28.0 with electron 2.0.3 on Windows. @ashthespy does this work there for you?

@darron1217
Copy link

This fixed my problem :)

On Atom 1.28.0 Window 10

image

@the-j0k3r
Copy link
Collaborator

the-j0k3r commented Jun 22, 2018

nice... if it fixes yours fixes mine also its the same bug :) #555

@darron1217
Copy link

darron1217 commented Jun 22, 2018

I checked it working on Mac too
on node 8.11

nateonpasteboard_2018-06-22-14-33-11

@daviwil
Copy link

daviwil commented Jun 22, 2018

Hey folks, just wanted to let you know I'm working on getting node-pty-prebuilt updated to 0.7.5 and to fill in the gaps with some of the newer Node versions since the last release of prebuilds for this module in January. Will keep you posted!

@rustnnes
Copy link

@ashthespy, after change the two files described on this PR, what should I do to manage this to work?

@the-j0k3r
Copy link
Collaborator

@ivankravets
Copy link
Member

@the-j0k3r without Windows binaries?

@the-j0k3r
Copy link
Collaborator

Ah dang spoke too soon :( @ivankravets theres no pty.js binaries for m57 anyway so we wait for @daviwil

@daviwil
Copy link

daviwil commented Jun 22, 2018

My Windows build failed, looking into it now

@ashthespy
Copy link
Author

@rustnnes cd to the package dir and apm rebuild --verbose

@rustnnes
Copy link

Works on Windows 10 x64, atom 1.28 x64, platformio-ide-terminal 2.8.2. Thanks!

By the way…
@ashthespy, with files properly edited, should apm rebuild --verbose auto-install node-pty-prebuilt or did you have to manually install it? It worked, but only after that, using npm i node-pty-prebuilt at package dir.

@the-j0k3r
Copy link
Collaborator

the-j0k3r commented Jun 26, 2018

@rustnnes I can confirm that you need node-pty-prebuilt in directory else it wont work just like that, I think this is the price of manually re-building the modules with something that wasn't installed via apm, if this PR was merged and released you would have a seamless experience and it would work out of the box.

I think its better to wait for node-pty-prebuilt 0.7.5 from @daviwil to become available in all platforms, then @ashthespy bump the package to ^0.7.5 and then with some luck @ivankravets will merge this and watch a ton of bugs disappear.

@the-j0k3r
Copy link
Collaborator

@ashthespy GOOD NEWS node-pty-prebuild 0.7.6 is now available with builds for all OS's and supported electron versions. https://github.com/daviwil/node-pty-prebuilt/releases/tag/v0.7.6

If you bump and rebase this PR on master now is the chance to potentially get this merged.

thanks @daviwil for latest release fresh out the press.;)

@the-j0k3r
Copy link
Collaborator

To save time I've rebased this branch on master and bumped node-pty-prebuilt #585

@ivankravets
Copy link
Member

Thanks! Resolved in https://github.com/platformio/platformio-atom-ide-terminal/releases/tag/v2.9.0

Please re-test.

@the-j0k3r the-j0k3r changed the title Switch from pty.js to node.pty with node-pty-prebuil Switch from pty.js to node.pty with node-pty-prebuilt Jul 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants