From: Jakub Narebski <jnareb@gmail.com>
Date: Thu, 16 Feb 2012 13:40:26 GMT
Subject: Re: [PATCH v3] git-latexdiff: new command in contrib, to use latexdiff and Git
Message-ID: <m3d39esxrg.fsf@localhost.localdomain>
In-Reply-To: <1329395775-18294-1-git-send-email-Matthieu.Moy@imag.fr>

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Changes since v2:
> 
[...]
> - Shell style issues (thanks to Jakub)
[...]

> +verbose "Auto-detecting PDF viewer"
> +candidates="xdg-open evince okular xpdf acroread"
> +if [ "$(uname)" = Darwin ]; then
> +    # open exists on GNU/Linux, but does not open PDFs
> +    candidates="open $candidates"
> +fi
> +
> +for command in $candidates; do
> +    if [ "$PDFVIEWER" = "" ]; then
> +	if command -v "$command" >/dev/null 2>&1; then
> +	    PDFVIEWER="$command"
> +	else
> +	    verbose_progress
> +	fi
> +    fi
> +done
> +verbose_done "$PDFVIEWER"

Eh?  I don't see shell style issues fixed (loop inside conditional
instead of vice-versa, "test ..." instead of "[ ... ]").

Nb. I think it would be good to put detecting PDF viewer in its own
function, don't you?

-- 
Jakub Narebski