Re: [PATCH v3] git-latexdiff: new command in contrib, to use latexdiff and Git

From: Jakub Narebski <jnareb@gmail.com>
Date: Feb 16, 2012, 13:40 UTC
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)

[...]

Show 17 quoted lines

> +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