feat(link): add --force flag

[claytonrcarter]

Hi there, now that I'm using Pulsar, I find myself with a fair number of old Atom packages that need tweaks or updates, and so I'm doing this dance a lot:

$ git clone $PACKAGE_THAT_NEEDS_A_FIX_OR_UPDATE
$ cd $PACKAGE_THAT_NEEDS_A_FIX_OR_UPDATE
$ npm i
$ ppm link
Linking $CURRENTLY_INSTALLED_PACKAGE to $PACKAGE_THAT_NEEDS_A_FIX_OR_UPDATE failed: EEXIST: file already exists, symlink '$PACKAGE_THAT_NEEDS_A_FIX_OR_UPDATE' -> '$CURRENTLY_INSTALLED_PACKAGE'

# dang it!

Obviously, the fix is simple and easy (rm -rf $CURRENTLY_INSTALLED_PACKAGE & try again), but it also felt like a --force flag would be handy to just do that for me while rerunning the command.

This was a pretty trivial change, and I'm happy to make changes as desired. I added 2 tests: one to confirm that it errors if the target exists, and another to confirm that it nukes it and then links when used with the --force flag.

Alternatives

I briefly considered calling it link --triforce. ⚔️

Thank you!

[confused-Techie]

Thanks a ton for contributing!

This is honestly a fantastic idea! Although my only concern is the --force flag possibly not being obvious enough in behavior. Since this flag does completely erase data on the client system. Makes me almost want to suggest we call it --destroyOriginal or something aggressive, but at the same time this should only be data from the package, and not be anything a user can't just get back via a redownload.

So I'm 100% on board with the concept, as well as love the implementation, very much appreciating the specs you've written as well. But curious if @DeeDeeG has any thoughts on the best flag name.

But overall looks awesome!

[claytonrcarter]

only concern is the --force flag possibly not being obvious enough

I chose --force because it reminded me of rm -f ..., git branch --force ... and some other commands that have similar flags, so it felt pretty "standard" for this sort of thing. That said, I'm totally happy to change it if another option sounds better to you and/or the team.

Thanks a ton for contributing!

You're welcome. I'm happy to help out!

[savetheclocktower]

I'm fuzzy on the use case here.

Assume language-foo is a Pulsar package I've got installed, and I notice something I'd like to fix. Here's what I usually do:

cd ~/Code/AtomPackages # or whatever
git clone git@github.com:foo/language-foo
cd language-foo
npm install
ppm link --dev .

Packages in ~/.pulsar/dev/packages shadow the ones in ~/.pulsar/packages — if a package exists at the former path in dev mode, it'll be loaded instead of the latter path. (This also requires that you run Pulsar in dev mode with pulsar --dev, of course.)

I think if a package is going to be removed from the directory, it should be uninstalled first, even if ppm is the one doing the uninstallation.

This isn't a veto; just want to understand it better.

[confused-Techie]

From my understanding the use case is essentially:

• A user installs a package from the Pulsar Package Registry, that was originally published to Atom.
• They download the git repo to modify the package locally, and fix any issues they discover
• To then install the package during this process they find them selves having to very consistently first uninstall, then install the package from a local repository.

So I can absolutely see the valid use case, and it does make complete sense, since very likely you'd want to be able to track your changes on your own fork, so will need to download it again elsewhere prior to publication. While yes it could be uninstalled first, I get that it can become repetitive.

So while I think it's a total valid use case, I have to admit it's probably only valid for a small portion of the userbase, which is why I feel like a non-standard flag may be more appropriate. While it makes things easier for the population that needs it, it doesn't use something too common that people might misunderstand the effect it has.

But if you feel that it's not a widespread use enough, I'm happy to hear it out and reconsider

[confused-Techie]

@claytonrcarter Thanks for the feedback. It feels good to me to change it, but I'm not the most attached to this repo, so I'd much rather get the opinion of someone like I've pinged if they feel it's at all necessary.

But I do totally get that it's a perfect standard flag, which is partially why it might be good to change it, so it's not easy to do without realizing the effect. Since I don't want someone to assume it forces the linking process, not realizing it forces the deletion of a file elsewhere on the system, since the force usually means to force the action taking place, rather than some side effect of the action.

An example that kinda works for me is a command that connects to a wifi network using --force I'd assume would force the network connection, rather than forget another one because of some internal logic. But don't worry too much for now!

[savetheclocktower]

It's only slightly fewer keystrokes than ppm uninstall language-foo && ppm link, but I'm OK with it as long as the user understands that it's irrevocable and ppm unlink won't restore the original state.

[claytonrcarter]

The compelling part of this, for me, is convenience. If I run ppm link, I'm doing so because I want it to create a link. If it doesn't because I have something in the way (for whatever reason), then I want to "force" it to do so. (Like, "yes, I know what I'm doing".) Pressing up and then adding -f feels much more ergonomic than having to run rm -rf ~/.pulsar/packages/$FOO or ppm uninstall $FOO and then rerunning the link. For those reasons, I don't think that it really matters what the flag is, so long as it includes a short option. I would be fine with something like --destroyOriginal so long we also include an short flag like -d of -D, for example.

Other options

• -f/--force: If needed, remove package before creating link (current description is "Remove the target path before linking")
• -D/--delete: If needed, remove package before creating link (or --destroy or --destroyOriginal)
• -U/--uninstall: If needed, uninstall package before creating link

Prior art

• rm has a -f flag to "Attempt to remove the files without prompting for confirmation"
• git branch has a -f/--force to "Reset to , even if exists already." or "allow deleting the branch irrespective of its merged status"
• git branch has -d/--delete to "Delete a branch." and -D as Shortcut for --delete --force.
• ppm unpublish has -f/--force to "Do not prompt for confirmation"
• ppm unlink and ppm uninstall both have -h/--hard to "Unlink (or uninstall) package from ~/.pulsar/packages and ~/.pulsar/dev/packages"

if a package is going to be removed from the directory, it should be uninstalled first

Do you mean that you'd prefer that the user have to run ppm uninstall, or just that we invoke the code w/i ppm uninstall for the user? FWIW, the code in ppm uninstall is just running fs.removeSync(...) (from package fs-plus, I assume because fs.rmSync(targetPath, { recursive: true, force: true }) was not supported on early Node versions at the time of writing) after checking for a package.json, so this implementation is not too far off already. I can look into reusing the code in uninstall, though I wonder if the benefit will be really worth it just to wrap a call to removeSync().

requires that you run Pulsar in dev mode

I grant that this is the "blessed" path for this sort of thing, but – personally – I long ago stopped using dev mode because I found it easier and more convenient to just link my customized packages directly into ~/.pulsar/packages. As I recall, this came about because I wasn't hacking on Atom at the time, only on a few packages. And then, once I got my changes to where I wanted them, I just wanted to use them w/o having to always drop into dev mode. And not only because I wanted to test drive in real usage, but also because it can take some time before PRs are merged, and I didn't want to make my change and then abandon it for weeks or months (or ever) while I waited for a merge and release.

probably only valid for a small portion of the userbase

This is true, and it is a pretty straightforward workaround to have to manually uninstall or remove the package/dir that's in way. Adding a quick and simple way to say "yeah yeah yeah, I know what I'm doing, just create the link already" feels like a valid enough use case that it's might be worth a small DX/QOL improvement.

[savetheclocktower]

I don't feel strongly enough about this to keep discussing it when there are syntax highlighting fixes to do, so I'll leave you folks to it. :-)

[DeeDeeG]

Sounds good to me! As a sometimes Linux user (and to the extent I'm in the CLI on macOS), this is indeed what I'd expect it to be named, personally.

Reasonable and neat little feature that I hope no-one has the misfortune of using accidentally or without understanding what it means, but they have to go out of their way to do so, so IMO it's on them to know what it does and it's kind of what I'd expect "forcing" the link to do, personally. Not that it's totally unambiguous to folks unused to doing rm -rf where it's not already ringing a bell what this might do, I guess.

I sympathize with wanting to preserve whatever modifications or content a user might have accumulated in a given package's dir. But as an optional flag I feel it's a fair trade-off as-is. I can support giving users this tool, and having them have to reach for it by typing it in, but I don't think it's worthwhile to make it rather un-ergonomic by being long, at the cost of being able to type it easily, for the benefit of preventing the potential mistakes.

Implementation in src/ looks fine, I didn't review the specs very closely, but they are passing, so that's good enough for me!

Thanks for the contribution!

P.S. apologies for the delay, I've been away for a bit and am catching up on things.

[confused-Techie]

Thanks for taking a look at this one @DeeDeeG and glad to see you merged it.

Also of course thanks a ton to @claytonrcarter for your wonderful contribution and patience!