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

Install hpm by default #282

Closed
wants to merge 1 commit into from
Closed

Install hpm by default #282

wants to merge 1 commit into from

Conversation

mxstbr
Copy link

@mxstbr mxstbr commented Jul 19, 2016

This is an easy solution to issue #277. Installs hyperterm package manager (hpm) by @matheuss after installing hyperterm.

Further discussion possibly needed, maybe this should be in core?

@amilajack
Copy link
Contributor

Nice parallel to atom

@mathieudutour
Copy link

Hum when you download the release directly, there is no npm install done so hpm won't be installed

@mxstbr
Copy link
Author

mxstbr commented Jul 22, 2016

True, that's kinda annoying. I wonder how Atom does this with apm?

@amilajack
Copy link
Contributor

amilajack commented Jul 22, 2016

apm isn't installed when running npm i -g hpm. This assumes that the user has npm installed globally, and that isn't a requirement to install atom. Run which apm or which apm-beta and you'll see the executable in the /usr/local/bin/apm-beta directory while modules installed with npm are (for nvm users at least) in the ~/.nvm/versions/node/${node_version}/bin/. The binary files seem to have symbolic links to their corresponding javascript located in ../lib/node_modules/${module_folder}/${file}. Ex:

npm -> ../lib/node_modules/npm/bin/npm-cli.js

@amilajack
Copy link
Contributor

amilajack commented Jul 22, 2016

Also note that apm isn't listed when running npm list -g --depth 0.

@matheuss
Copy link
Member

matheuss commented Jul 22, 2016

I think that the best thing we can do is something like:

command -v npm >/dev/null 2>&1 ||  echo >&2 "You must install npm etc etc";

If the user does not have node and/or npm, he will not be able to install any plugins anyway.

@matheuss
Copy link
Member

matheuss commented Jul 22, 2016

apm comes bundled with Atom:

❯ ~ which apm | xargs ls -l
/usr/local/bin/apm -> /Applications/Atom.app/Contents/Resources/app/apm/node_modules/.bin/apm

I think it might be a good approach for us. Then hpm can check for npm and provide instructions (install nvm etc) if it is not installed 😊

@rauchg
Copy link
Member

rauchg commented Jul 22, 2016

Yeah, I think it's looking pretty great. I need to do more testing, so we'll reserve this for the next release.

@rauchg
Copy link
Member

rauchg commented Jul 22, 2016

Things we discussed that are outstanding concerns for this:

  • How do we install it in a location that we know is in $PATH?
  • How do we keep it updated? Ideally we pin it, and with each new release of HyperTerm we can bump it, and it updates hpm automatically
  • If we can't find a location in $PATH we can write to, how do we tell the user? (ie: notification)
    • Where in the menu do we display "Install hpm` in that scenario?
  • How do we tell users when we install it that they can now run hpm if needed?
    • Maybe we have an onboarding screen with a little html file that we also use to explain the configuration system?

@sindresorhus
Copy link
Contributor

How do we install it in a location that we know is in $PATH?

$ which npm

@matheuss
Copy link
Member

matheuss commented Jul 22, 2016

Also: we need node. I'm not completely sure, but I think that HyperTerm can run without node, can't it?

@lordgiotto
Copy link
Contributor

We can use the node executable bundled in electron I guess.

@rauchg
Copy link
Member

rauchg commented Jul 22, 2016

@sindresorhus we anticipate scenarios where npm is not installed, so we also have to consider that

@sindresorhus
Copy link
Contributor

Good point. You already have the user's $PATH from the shell-env module though, so you can just use any path defined there.

@sheerun
Copy link
Contributor

sheerun commented Jul 24, 2016

Why not bundling npm along hyperterm? No need to use system one.

(also when bundling npm, you can fix the version, e.g. to 3.x)

@sheerun sheerun mentioned this pull request Jul 24, 2016
@sheerun
Copy link
Contributor

sheerun commented Jul 24, 2016

I've submitted #381 that embeds npm, but I believe hyperterm shoudn't install hpm externally, but rather embed it as dependency, and link it to system similarly to atom.

@sheerun
Copy link
Contributor

sheerun commented Jul 24, 2016

PR #383 on to of global hotkey functionality, includes ability to embed default plugins into hyperterm. I think this should be primary way of embedding hpm.

One thing remaining is to create system links for hpm binary upon installation, but unfortunately I won't have time to implement it.

@matheuss
Copy link
Member

+1 for shipping npm with the app

@leo
Copy link
Contributor

leo commented Jul 25, 2016

@timothyis timothyis added the 👩‍🔬 Status: In Progress Issue or PR is currently active and work is in progress label Jul 29, 2016
@albinekb albinekb mentioned this pull request Sep 26, 2017
@MindTooth
Copy link

Guess this can be closed now with 2.0.0-canary9?

5700690

@leo leo closed this Jan 9, 2018
@leo
Copy link
Contributor

leo commented Jan 9, 2018

🎉 Big applause to @chabou! 👏 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👩‍🔬 Status: In Progress Issue or PR is currently active and work is in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet