search

lisamelton / video-transcoding-scripts

Utilities to transcode, inspect and convert videos.
MIT License
603 stars 76 forks source link

detect HandBrakeCLI path via Spotlight #14

Closed vszakats closed 9 years ago

vszakats commented 9 years ago

Hello Don, I've made a small patch to lookup the location of HandBrakeCLI by using Spotlight. This makes it unnecessary to add it to PATH.

lisamelton commented 9 years ago

Viktor,

Thanks for the patch. Props to you for trying to solve the tool dependency problem that plagues every developer.

However, I'm declining your patch. I'll explain why.

First, Spotlight is not an appropriate mechanism to implement this.

Using Spotlight and mdfind this way is an OS X-only solution. My scripts are designed to work on Windows, Linux and other platforms. I would be rejecting your patch for that reason alone.

And just as important, Spotlight is just not capable of finding the correct binary on a user's file system. For example, if I execute this command:

mdfind 'kMDItemContentType == public.unix-executable && kMDItemFSName == HandBrakeCLI'

...Spotlight finds 36 different versions of HandBrakeCLI on my current machine. And your patch, using head -1 to trivially select the first item in that list picks the wrong one.

I suspect many other users who don't empty out their ~/Downloads folder would have similar problems.

Also, your patch is only trying to find HandBrakeCLI. My scripts have many other tool dependencies that are just as important.

I'm closing this now, but feel free to continue commenting here.

vszakats commented 9 years ago

Oops, huge sorry for totally disregarding all other platforms (saying this from a completely multi-platform background – I can only blame it on the late night work). Agreed on your decision and many thanks for detailing your reasons.