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

Proper path integration for all OS binaries #187

Open
1 task done
Daeraxa opened this issue Dec 1, 2022 · 7 comments
Open
1 task done

Proper path integration for all OS binaries #187

Daeraxa opened this issue Dec 1, 2022 · 7 comments
Labels
enhancement New feature or request

Comments

@Daeraxa
Copy link
Member

Daeraxa commented Dec 1, 2022

Have you checked for existing feature requests?

  • Completed

Summary

Commands missing from path

See here for Discord conversation.

Basically we should provide a solution to the various packages to make sure that the Pulsar and PPM commands can always be found on the path.

For example if you currently download the Appimage the only way to get the pulsar command working is to include it on the path manually even if integrated with appimageinstaller. Same with the tar.gz.

There is already a caveat for macOS in Atom where a command is provided that isn't accessible to other platforms to install the application and apm/ppm commands. (see conversation and atom docs).

When you first open Atom, it will try to install the atom and apm commands for use in the terminal. In some cases, Atom might not be able to install these commands because it needs an administrator password.
...
To install the atom and apm commands, run "Window: Install Shell Commands" from the Command Palette, which will prompt you for an administrator password.

There also seems to be a current issue with the Windows binary not installing this to the path which was seemingly handled by Squirrel. Discord.

Now this does work in many places. The .deb file certainly adds pulsar to the path and I suspect the .rpm and macOS .dmg are similar as the pulsar.sh file seems to be the one responsible for this.

How to fix it?

I think it is important that this gets resolved as currently in the docs it assumes that pulsar -p will always work and it seems a bit silly to offer formats where this integration isn't possible without manual steps.
It might be useful simply to expose this command to all platforms and extend the logic to work for all of them. This would allow manual install of the command no matter what system it is run on (and in cases where it doesn't work it will be easier to expand this so that it does). This would also potentially solve the Windows problem and take the guesswork out of the existing macOS case.

There should be a mechanism for this to run automatically on first start as well (maybe check from the config.cson) but it maybe shouldn't run without prompting for those who want to use it entirely as a portable app without leaving any traces. Maybe a first start popup asking to integrate Pulsar on the path with a brief explanation.

Enter your response below:

Seamless integration of the pulsar and ppm commands without needing to manually add Pulsar to the path.

Any alternatives?

Updated installer? A new appimage command for the appimage such as ./Pulsar_1.63.0.appimage --install that would be the .desktop exec or an option? Use docs to cover this use case?

None really seem ideal with our supported distributions.

Other examples:

I think Zed has this option as it doesn't put itself on the path on first install.
Many other applications simply don't come as an appimage and just as OS specific packages which work natively.

@DeeDeeG
Copy link
Member

DeeDeeG commented Dec 28, 2022

🍎 The macOS issue is fixed. See #251 and the two linked PRs, which each fixed half of the issues affecting the pulsar.sh script on macOS. ✔️

@DeeDeeG
Copy link
Member

DeeDeeG commented Dec 28, 2022

🐧 Pretty simple fix for Linux: we can symlink to /usr/bin/ppm just like we currently symlink to /usr/bin/pulsar, at least for .deb and .rpm build targets of electron-builder.

In this file: https://github.com/pulsar-edit/pulsar/blob/5f96f2737d787040ff76999eef6de35d9ee8aeca/script/post-install.sh

Which is run after installing the .deb or .rpm packages, per the electron-builder config:

deb: { afterInstall: "script/post-install.sh" },
rpm: {
afterInstall: "script/post-install.sh",

UPDATE: This suggestion is a PR now: #273


We could also consider supporting the "Install Shell Commands" command in-app, just like we do on macOS, to workaround any issues with our packaging, or for folks who have gotten the app as an AppImage or .tar.gz targets, but those are pretty portable/arbitrary where you save them to, and that makes it a lot harder to predict where the real binary will be for us to symlink to...

The AppImage one is kinda unfortunate in that sense, since it's such a smooth user experience otherwise, but I think those users should use pulsar -p or roll their own symlink if they don't mind.

Likewise for the .tar.gz folks, I especially think they don't expect automatic system integration from just extracting a compressed archive of the files..

@DeeDeeG
Copy link
Member

DeeDeeG commented Dec 28, 2022

🪟 For Windows, I suppose we can try to invoke setx with Administrator permissions at some point in the install to add Pulsar's install dir to the PATH? Which would allow running Pulsar from the CLI.

https://stackoverflow.com/a/69383805

Perhaps during the installer:

And/or we could likewise add this to a Windows version of the "Install Shell Commands" command, which users can manually run in-app.

Portable edition should probably not be put on the PATH, nor used with extensive ppm commands, since it is a self-extracting 7-zip archive, and it takes a super long time to launch... Imagine waiting through that super long extraction process just to run a CLI program...

@confused-Techie
Copy link
Member

@DeeDeeG I'm personally on board with adding everything to be a variant of "Install Shell Commands" as long as that's clearly documented it feels the best, just to offload the choice and responsibility for users.

And if it's an option for every OS we can add it right into the "System Settings" page like other OS integration features, rather than have it hidden away behind a command. But of course for simplicity the button there can just invoke the command.

But I am really glad to see there is a way to fix, at least quickly for Linux, and is working on MacOS, then looks like it just needs another script for Windows.

An alternative approach for Windows, is from some quick research I was doing a few days ago about electron-builder it looks like packaging Windows as an AppX would alleviate the concern about PATH, and allow for some other features Windows Users might expect, like App Uninstallation directly from Settings rather than Control Panel like what currently has to occur

@DeeDeeG
Copy link
Member

DeeDeeG commented Dec 28, 2022

Interesting about AppX...

@confused-Techie
Copy link
Member

@Daeraxa Now with #604 merged, I can personally say that Pulsar has full proper PATH integration for Windows.

And you mentioned above that macOS should be fine as well, and some versions of the Linux installer.

So really to close this issue out, should we extend the behavior of macOS to Linux to be able to add Pulsar to the PATH manually there as well? I'd love to take a shot at this, but can you think of any examples or possibilities about the code that would have to be implemented itself?

And for posterity:

  • Windows
  • macOS
  • Linux: AppImage
  • Linux: tar.gz
  • Linux: rpm
  • Linux: deb

This is looking pretty good, and that we are close to having it fully solved, with obviously a few discrepancies still

@Daeraxa
Copy link
Member Author

Daeraxa commented Jul 21, 2023

Really anyone downloading the tar.gz should be (and probably will be) dealing with this themselves, at most we may need to provide instructions on where the binaries are in the file structure.

Appimage is a bit more difficult pulsar -p should work out the gate so long as you either integrated the appimage directly (with something like appimagelauncher) or did it yourself - see https://discord.com/channels/992103415163396136/1094684411397681244/1106722578892062811 and https://discord.com/channels/992103415163396136/1094684411397681244/1106728722142331081. Not sure how we could easily expose ppm though as you would need to unpack the appimage first. There probably is a way of doing it but very likely beyond me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants