Conversation
| raise "No writable cache dir found. " + | ||
| "Check your home directory's permissions, " + | ||
| "or set the MINI_EXIFTOOL_PSTOREDIR env var " + | ||
| "and try again." |
There was a problem hiding this comment.
I wanted to keep the line length to a reasonable minimum, but one downside of splitting long strings up like this is that it makes the codebase harder to grep. What are your thoughts?
| "or set the MINI_EXIFTOOL_PSTOREDIR env var " + | ||
| "and try again." | ||
| else | ||
| File.join(cache_home, 'mini_exiftool') |
There was a problem hiding this comment.
I wanted to keep the code simple, but there are a couple wrinkles with this:
-
We no longer switch on windows vs. unix. I figured that in most cases, we will be using the value of
Gem.cache_home—which typically resolves to$HOME/.cacheon UNIX orENV["HOMEDRIVE"] || ENV["SystemDrive"] + "/.cache"on Windows (src 1 2)—so there's no need to give the directory a special prefix. In the case of our Windows fallbacks, however, now we have a simplemini_exiftooldirectory instead of_mini_exiftool.I don't know anything about software conventions on Windows, so I defer to your expertise here.
-
This also means that if a user explicitly specifies the pstoredir via a
$MINI_EXIFTOOL_PSTOREDIRenv var, then the actual location of the pstoredir will be a subdirectory of that. Which is not a problem, really, just maybe a little cluttery and not what you might expect.
I don't know how much either of these problems bothers you. For the former, we could simply restore the original ternary expression (maybe improved as Gem.win_platform? ? _mini_exiftool : mini_exiftool); for the latter, we could rewrite this method to treat ENV['MINI_EXIFTOOL_PSTOREDIR'] separately from all the other options.
|
Thanks for your effort and support. I have in the same time implemented a slightly different solution. Let me think a while. I will release a new version of mini_exiftool in the next days. |
|
Oh for sure. Thanks for the free software! 🙇🏻 |
2 participants
Fixes #51.