ADDED $PATH insert/remove commands. Tasks can now use await.

[justjoeyuk]

I've added the ability to Add/Remove Oni to the $PATH on OSX. It would most likely work on Linux if someone went ahead and added tweaks. I only have access to OSX right now so it's the only fix I can present.

I wanted a way to change the command to Add/Remove depending on a boolean check. This would require me to know when a task has been executed. When a task is executed, an event is now fired. I check that event for either of the two $PATH modifications and reload the commands if it was either of them.

The nature of asking a users permission (sudo) is asynchronous and so kicking off the event would have happened before the user confirmed their password, so I went ahead and made task completions async. That way, each task can now perform awaits.

Let me know if there are any thoughts. Demo:

[justjoeyuk]

This pull request is for issue #346

[justjoeyuk]

@extr0py It seems that the require is causing the linter to fail. sudo-prompt only allows require it seems and I've not much experience with TypeScript, so not sure how to proceed. I think we should use sudo-prompt as it's cross platform and makes it easier for other platforms in the future.

[justjoeyuk]

Should all be fixed now 👍

[justjoeyuk]

Updated the pull request. Since we're trying to keep each platforms implementation separate, it checks exclusively for OSX when adding the commands at the moment, with a TODO: for when the linux/windows ones are complete.

[bryphe]

Wow, very cool! Thanks for implementing this @justjoeyuk ! I'll give it a go shortly 👍

[bryphe]

Thanks for going the extra mile here to wire up showing add / remove correctly, that's awesome!

[bryphe]

Can you please clamp the dependency at 7.1.1? (Remove the ^)

[bryphe]

Looks like this would address both #636 and #346 - quite a first PR 😄

[bryphe]

Can we derive this from process.execPath or process.helperExecPath? This logic will work fine in the install-from-dmg case, but I see a decent amount of .zip downloads too. It'd be great if it worked in that case as well.

[bryphe]

Just tried this out, worked great! OSX is Oni's most downloaded platform, and this addresses one of the first blockers in the experience - excellent contribution!

I think the only thing that would improve the experience is if we gave some sort of confirmation message when we successfully add (or remove) the path. Unfortunately, there isn't a message / notification UI yet. This can come in a separate PR, though - I added created issue #653 to track that.

So the only remaining items I see are:

• Is it possible to derive the application path from process.execPath or process.helperExecPath? If so, this would work in the zip case too
• Clamp the version of sudo-prompt

Once those are addressed, I'll go ahead and bring it in. This is really awesome, thanks for working on this @justjoeyuk !

[justjoeyuk]

@extr0py Added a way for tasks to show messages once completed. It's a start, but ultimately the completion handlers should return whether or not they were successful and we can then decide what to do by the returned value.

Clamped the version for sudo-prompt.

Will look into alternative ways to find where the executable is running from.

[justjoeyuk]

@extr0py I've not got the application path at run-time so it's flexible. I've tested it and all appears to be working. Let me know if you have any problems with it.

[bryphe]

Excellent, thank you for making the updates @justjoeyuk ! Trying it out now. The code looks good to me overall.

[bryphe]

The dialog looks great! I am hitting an issue, though.

I'm getting an error when it hits _runSudoCommand:
Error: ENOENT: no such file or directory, open '/Users/extr0py/oni/dist/Mac/Oni.app/Contents/Resources/app../../Resources/Oni.icns'

Looks like perhaps its maybe missing the / after app?

Digging into it, I think a robust fix is to use path.join to handle some of these corner cases:

        const appDirectory = path.join(path.dirname(process.mainModule.filename),"..", "..");
        const options = { name: "Oni", icns: path.join(appDirectory, "Resources/Oni.icns") };
        const linkPath = path.join(appDirectory, "MacOS/Oni");

Could you please update that logic to use path.join? Once I updated that, it started working and I was able to add to path again (with the icon to boot!)

[bryphe]

(I also saw the appveyor build failed, that looks like it was a problem installing electron, not related to the PR - I kicked it off again)

[justjoeyuk]

@extr0py All done 🎉

[bryphe]

Awesome, thanks @justjoeyuk ! The path handling looks more robust now, and the 'add to path' gesture works great on my OS X machine. Most excellent.

I'll go ahead and bring this in! Thanks again for putting this together.

I believe this has had a bounty on it for a while against #346 - so make sure to claim that!