Re: proposed patches to notmuch-emacs-mua

Subject: Re: proposed patches to notmuch-emacs-mua

Date: Sun, 20 Aug 2017 15:17:35 +0300

To: Joseph Mingrone, notmuch@freelists.org

Cc:

From: Jani Nikula

On Sat, 22 Jul 2017, Joseph Mingrone <jrm@ftfl.ca> wrote:
> From b552bdaad1686256ca1da388be0c714d4f0974f0 Mon Sep 17 00:00:00 2001
> From: Joseph Mingrone <jrm@ftfl.ca>
> Date: Thu, 20 Jul 2017 20:35:24 -0300
> Subject: [PATCH 3/3] Trivial changes to silence a few shellcheck.net warnings

We try to add a commit message even for the short ones. This one could
use a reference to the actual shellcheck warnings being fixed.

Based on prior experience, it's not really benefitial in the long term
to fix static checker warnings *unless* you also add some build rules to
actually run the checker. I'm not sure if it should be part of the
normal build, release checks, or test run, but there should be some
place where shellcheck gets run (semi-)regularly. Otherwise it'll just
bitrot back to giving warnings.

Other than that, the changes here look fine.

BR,
Jani.

>
> ---
>  emacs/notmuch-emacs-mua | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/emacs/notmuch-emacs-mua b/emacs/notmuch-emacs-mua
> index 521dd342..c66a0c00 100755
> --- a/emacs/notmuch-emacs-mua
> +++ b/emacs/notmuch-emacs-mua
> @@ -150,14 +150,14 @@ if [ -n "${MAILTO}" ]; then
>  	exit 1
>      fi
>      ELISP="(browse-url-mail \"${MAILTO}\")"
> -elif [ -z "${ELISP}" -a -n "${HELLO}" ]; then
> +elif [ -z "${ELISP}" ] && [ -n "${HELLO}" ]; then
>      ELISP="(notmuch)"
>  else
>      ELISP="(notmuch-mua-new-mail) ${ELISP}"
>  fi
>  
>  # Kill the terminal/frame if we're creating one.
> -if [ -z "$USE_EMACSCLIENT" -o -n "$CREATE_FRAME" -o -n "$NO_WINDOW" ]; then
> +if [ -z "$USE_EMACSCLIENT" ] || [ -n "$CREATE_FRAME" ] || [ -n "$NO_WINDOW" ]; then
>      ELISP="${ELISP} (message-add-action #'save-buffers-kill-terminal 'exit)"
>  fi
>  
> @@ -167,13 +167,13 @@ escape -v pwd "$PWD"
>  ELISP="(prog1 'done (require 'notmuch) (cd \"$pwd\") ${ELISP})"
>  
>  if [ -n "$PRINT_ONLY" ]; then
> -    echo ${ELISP}
> +    echo "${ELISP}"
>      exit 0
>  fi
>  
>  if [ -n "$USE_EMACSCLIENT" ]; then
>      # Evaluate the progn.
> -    exec ${EMACSCLIENT} ${NO_WINDOW} ${CREATE_FRAME} ${AUTO_DAEMON} --eval "${ELISP}"
> +    exec "${EMACSCLIENT}" ${NO_WINDOW} ${CREATE_FRAME} ${AUTO_DAEMON} --eval "${ELISP}"
>  else
> -    exec ${EMACS} ${NO_WINDOW} --eval "${ELISP}"
> +    exec "${EMACS}" ${NO_WINDOW} --eval "${ELISP}"
>  fi
> -- 
> 2.13.3
>

-- 
perskule

Thread: