Thank you for your review
You are right, your code looks much cleaner and does not contain redundancies.
I also think that testing for exit status is better, since this also does not
spawn the test process. I created a git diff, which would be nice if it goes
upstream.
diff --git a/src/platform/darwin.sh b/src/platform/darwin.sh
index 342ecce..5d0e4de 100644
--- a/src/platform/darwin.sh
+++ b/src/platform/darwin.sh
@@ -43,6 +43,12 @@ qrcode() {
fi
}
-GETOPT="$(brew --prefix gnu-getopt 2>/dev/null || { which port &>/dev/null &&
echo /opt/local; } || echo /usr/local)/bin/getopt"
+if command -v brew >/dev/null; then
+ GETOPT_PREFIX="$(brew --prefix)/opt/gnu-getopt"
+elif command -v port >/dev/null; then
+ GETOPT_PREFIX="/opt/local"
+fi
+GETOPT="${GETOPT_PREFIX:-/usr/local}/bin/getopt"
+
SHRED="srm -f -z"
BASE64="openssl base64"
> Am 23.10.2018 um 05:54 schrieb Allan Odgaard <[email protected]>:
>
> On 22 Oct 2018, at 23:50, [email protected]
> <mailto:[email protected]> wrote:
>
> currently (and as I can tell also in the past, see [1],[2],[3],[4],[5]), brew
> —prefix <FORMULA> is really slow (see the example output below), which makes
> it somewhat annoying using pass on macOS. I hunted this down and found that
> the problem is the following: brew runs git rev-parse --short=4 --verify -q
> HEAD every time to find the currently installed version of the formula (which
> seems somewhat unreasonable). Depending on your git and network connectivity,
> this may take longer. Using brew —prefix without a formula is reasonably
> fast. As far as I can tell, there is currently no way to change the prefix
> path in brew, thus using $(brew —prefix)/opt/gnu-getopt should be a save way
> for determining the path to gnu-getopt.
>
> Great find! I’ve been somewhat annoyed myself by the ~1s delay when using
> pass, but always assumed it was just GPG being slow.
>
> I patched this and attached the patch to this mail. Since my implementation
> is somewhat longer, I think it should not be implemented in one line anymore,
> but split it up as I did (see the patch). If you come up with something
> better let me know (or, of course, patch it yourself). Additionally, since I
> am not using Macports, I can not test if this also works with ports. Before
> considering pushing my patch upstream, someone needs to test this! For brew,
> it works fine.
>
> The elif and else branches look wrong, as we are not in a command context, so
> shouldn’t prefix with echo, plus including the which line is redundant when
> the elif already test for the presence of port.
>
> I also find it cleaner to test the exit status of command instead of testing
> against non-zero output.
>
> Here’s my revised proposal for the relevant code to setup GETOPT:
>
> if command -v brew >/dev/null; then
> GETOPT_PREFIX="$(brew --prefix)/opt/gnu-getopt"
> elif command -v port >/dev/null; then
> GETOPT_PREFIX="/opt/local"
> fi
> GETOPT="${GETOPT_PREFIX:-/usr/local}/bin/getopt"
> If we find neither brew nor port then we use GETOPT_PREFIX before falling
> back to /usr/local.
>
> Don’t think we actually need this, but it makes the code look nicer :)
>
> _______________________________________________
> Password-Store mailing list
> [email protected]
> https://lists.zx2c4.com/mailman/listinfo/password-store
_______________________________________________
Password-Store mailing list
[email protected]
https://lists.zx2c4.com/mailman/listinfo/password-store
