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

From: Jakub Narebski <[email-masked]>
Date: Thu, 16 Feb 2012 01:15:27 -0800 (PST)

Matthieu Moy <[email-masked]> writes:

> +verbose "Auto-detecting PDF viewer"
> +for command in xdg-open evince okular xpdf acroread; do
> +    if [ "$PDFVIEWER" = "" ]; then
> +	if command -v "$command" >/dev/null 2>&1; then
> +	    PDFVIEWER="$command"
> +	else
> +	    verbose_progress
> +	fi
> +    fi
> +done
> +verbose_done "$PDFVIEWER"

Why we autodetect PDF viewer unconditionally?  Why we do not stop on
first detected viewer rather than last?  Why not

  +if [ "$PDFVIEWER" = "" ]; then
  +	verbose "Auto-detecting PDF viewer"
  +	for command in xdg-open evince okular xpdf acroread; do
  +		if command -v "$command" >/dev/null 2>&1; then
  +			PDFVIEWER=$command
  +			break
  +		else
  +			verbose_progress
  +		fi
  +	done
  +	verbose_done "$PDFVIEWER"
  +fi

Nb. Documentation/CodingGuidelines says:

  For shell scripts specifically (not exhaustive):
  
  [...]
  
   - We prefer "test" over "[ ... ]".

I know that 'contrib/' is more relaxed...
-- 
Jakub Narebski