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

Add cask for TotalFinder #395

Closed
wants to merge 1 commit into from
Closed

Conversation

athaeryn
Copy link
Contributor

Add a cask for the Finder replacement TotalFinder.

@vitorgalvao
Copy link
Member

When you use install, you don’t need the link line.

@athaeryn
Copy link
Contributor Author

@vitorgalvao Ah, good to know. I couldn't find any documentation on install (or uninstall, for that matter) at all, so maybe it would be good to update CONTRIBUTING.md with some information regarding a) the fact that it exists, and b) that you don't need the link line, as you mentioned.

@vitorgalvao
Copy link
Member

There’s still no documentation because the feature isn’t completely done yet (install works, but uninstall does not).

Related, could you please add a uninstall 'TotalFinder Uninstaller.app' line? It’s not something to hurry, thought, as the pull request shouldn’t be merged until that part is implemented (that’s why there are so many hanging).

@athaeryn
Copy link
Contributor Author

I see. I saw that issue but didn't read all the way through, I just saw the syntax examples and assumed that since some other casks I inspected used install I could throw it in. 😄

Also, I added the line. Should I squash these commits?

@vitorgalvao
Copy link
Member

Some casks have an installer but not an uninstaller, and others are there for testing this feature, hence why you saw some with install, but none with uninstall (having it there will give an error).

Regarding your question, do however you prefer. It’d make the history a bit cleaner, but it’s not a requirement, a lot of casks get merged with multiple commits.

@athaeryn
Copy link
Contributor Author

Okay! I squashed everything into one commit for when uninstall is working and things can be merged in.

@fanquake
Copy link
Contributor

@athaeryn The current release version is now @ 1.4.18, care to update this pull so we can get it merged?

@athaeryn
Copy link
Contributor Author

Yeah, I'll do that right now!

@athaeryn
Copy link
Contributor Author

I just squashed it into the same commit.

@nanoxd
Copy link
Contributor

nanoxd commented Sep 16, 2013

@darwin Besides running the .app uninstaller, will a pkgutil remove the application properly or are there other files that must be removed?

@darwin
Copy link

darwin commented Sep 16, 2013

@nanoxd I'm afraid there is more to it than pkgutil can do (never tried it, I use Packages.app to build my installer package without really digging deep into Apple's pkg stuff)

the uninstaller runs this applescript:
https://github.com/binaryage/totalfinder-installer/blob/master/Uninstall.applescript

@nanoxd
Copy link
Contributor

nanoxd commented Sep 16, 2013

@darwin thanks for the quick reply. We will see how we can integrate osascript into brew-cask to parse applescript

@darwin
Copy link

darwin commented Sep 16, 2013

@nanoxd why don't you want to launch that uninstaller app? (I'm not familiar with cask)

/Library/ScriptingAdditions/TotalFinder.osax/Contents/Resources/TotalFinder.bundle/Contents/Resources/TotalFinder\ Uninstaller.app

I could change uninstallation to something which fits your system.

@nanoxd
Copy link
Contributor

nanoxd commented Sep 16, 2013

@phinze would have a better response to this but we currently don't support .app uninstalls.

We support the following uninstall methods:

  • script specifies script to run (relative to cask dir) to uninstall
  • args specifies arguments for script
  • input specifies input for script
  • launchctl specifies list of launchctl services to remove
  • pkgutil specifies regexp of pkgutil to remove all files for and forget
  • files specifies list of files to explicitly remove (as a fallback)

@vitorgalvao
Copy link
Member

@darwin The uninstaller app launches a GUI (which we try to avoid). The first line of this comment sums it up.

@darwin
Copy link

darwin commented Sep 16, 2013

@vitorgalvao thanks, I get it now.

I can provide some headless script to do the uninstallation. Right now my uninstaller app uses Platypus (it was quick and ease solution). But I could wrap it info my own app and support headless execution.

@nanoxd
Copy link
Contributor

nanoxd commented Sep 19, 2013

@darwin that would be great. We also are having an issue with TotalTerminal launching a gui. Any way the script could run on both apps?

@darwin
Copy link

darwin commented Oct 1, 2013

I just released TotalFinder 1.4.29 on beta channel with new uninstaller.

sudo /Library/ScriptingAdditions/TotalFinder.osax/Contents/Resources/TotalFinder.bundle/Contents/Resources/TotalFinder\ Uninstaller.app/Contents/MacOS/TotalFinder\ Uninstaller --headless

uninstaller source code can be found here:
https://github.com/binaryage/uninstaller

TotalFinder download here:
http://totalfinder.binaryage.com/beta-changes

@nanoxd
Copy link
Contributor

nanoxd commented Oct 4, 2013

@darwin Thank you!

@athaeryn Can you update the Application to fit the new install script? If you have any issues, we are in the IRC channel.

@phinze phinze closed this in 0fb6213 Oct 21, 2013
@phinze
Copy link
Contributor

phinze commented Oct 21, 2013

merged with latest version of totalfinder and usage of new uninstallation script; thanks to all for your work on this! 🔨

kevinSuttle pushed a commit to kevinSuttle/homebrew-cask that referenced this pull request Dec 8, 2013
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants