pulsar-edit / pulsar

A Community-led Hyper-Hackable Text Editor
https://pulsar-edit.dev
Other
3.24k stars 137 forks source link

Experiment: Redirect `-p`/`--package` to `ppm` via `pulsar.sh`… #1066

Closed savetheclocktower closed 1 week ago

savetheclocktower commented 2 months ago

This one is in draft mode until I can get some Linux testers.

Identify the Bug

There are a handful of issues with pulsar.sh on macOS and Linux; they’re largely covered in #1053:

Description of the Change

Alternate Designs

We already do the main alternative to handling --package in pulsar.sh — and we should keep it as a fallback.

Possible Drawbacks

Bash scripts are a bit harder to write and don’t have a great testing situation, so I’d love to get field reports on how well this works on Linux. I was able to test the macOS parts on my local machine, but on Linux I’m mainly flying blind.

Verification Process

Extra testing for Linux:

Release Notes

savetheclocktower commented 2 months ago

OK, I grabbed a lightweight Ubuntu-flavored distro and did some testing on VirtualBox. The script needed some tweaks because of typos and my Bash naïvete, but now it works solidly on tar.gz distributions of Pulsar for Linux.

The same logic that works properly for Pulsar at an arbitrary location on disk will also work for Pulsar installed at the standard /opt/Pulsar path of the RPM and DEB packages — but if, somehow, it doesn't, then we'll still check a hard-coded /opt/Pulsar path to be safe.

I think the code path for CI is silly, but I want to understand it better before I take it out.

AppImage is another ball of wax and will have to be tackled in a separate PR.

savetheclocktower commented 2 months ago

Failing editor test is the one that's known to be flaky. Not going to bother with re-running it for now.

DeeDeeG commented 1 month ago

Re-running CI so I have some binaries to test. (Old ones are expired and deleted by GitHub Actions due to the passage of time.)

CI is passing as of before my doing so, for the record.

DeeDeeG commented 2 weeks ago

I'm not sure if pulsar.sh was ever expected to work for .tar.gz bundles, as it's just as easy to call the pulsar binary directly by absolute (full) path as the launcher, and it basically works fine.

(Come to think of it: Why do we really need the .sh launcher??? I have always found directly launching the actual Electron binary just as satisfactory to my needs as the launcher. I don't have a lot of love for mysterious "don't think about it too hard" kinds of complexity...)

So, if that works, I consider it above and beyond.

DeeDeeG commented 2 weeks ago

Sorry for the delay in review.

Your work on testing this has been thorough enough I am just about temped to round it up to being all the review this PR needs. Will try to make time to properly review it to whatever extent it still needs it beyond what you've already done to establish its bona fides.

savetheclocktower commented 2 weeks ago

I'm not sure if pulsar.sh was ever expected to work for .tar.gz bundles, as it's just as easy to call the pulsar binary directly by absolute (full) path as the launcher, and it basically works fine.

(Come to think of it: Why do we really need the .sh launcher??? I have always found directly launching the actual Electron binary just as satisfactory to my needs as the launcher. I don't have a lot of love for mysterious "don't think about it too hard" kinds of complexity...)

So, if that works, I consider it above and beyond.

I wouldn't quite say it works fine. It logs some main process output to the console. Some features don't work, like --wait. (Those last two, taken together, make it impossible to use Pulsar as a Git editor for commit messages, interactive rebases, etc.) Having the wrapper allowed us to fix a major encoding headache on Windows and it allows us to redirect --package to PPM on all platforms without making people wait for Electron to start up.

The goal is to be able to give people commands to run (in documentation, in Discord, etc.) and know they'll basically work the same everywhere, instead of “how did you install this? because if it was X, you'll run this, but if it's Y, you'll run this…”

This is also a prerequisite for AppImage users being able to invoke Pulsar properly (#1069). Right now it doesn't work at all because it tries to look for an executable in a fixed location on disk. Scratch that; right now the AppImage runs its own script that invokes the executable directly and behaves exactly like invoking the executable.