[PATCH] emacs: notmuch-show: remove extraneous shell quoting
Tomi Ollila
tomi.ollila at iki.fi
Wed Sep 14 02:51:51 PDT 2016
On Wed, Sep 14 2016, Matt Armstrong <marmstrong at google.com> wrote:
> Remove shell quoting from notmuch-show--build-buffer. The args list
> is ultimately passed to call-process, which passes them verbatim to
> the subprocess (typically, notmuch). The quoting, intended for a
> shell, is unnecessary and confusing.
To motivate someone(tm) to test and review this patch, I'd like to know
which problem does this _fix_ (if any). If it does not then such a good
change as this looks like it is takes a bit longer to be taken to review
(and the reviewers need to be more careful for spotting potential
regressions...)
Tomi
>
> This code originally appeared in
> 9193455fa1476ea3957475e77379b75efa6dd90b, aiming to support remote
> notmuch execution over ssh. The commit message included this example
> shell script (simplified):
>
> #!/bin/sh
> ssh remotehost notmuch $@
>
> That script doesn't properly quote "$@". Even if it did, it wouldn't
> work because the shell ssh starts on the remote host does another
> round of shell word splitting. Fortunately, notmuch already provides
> an example of a correct shell script on
> https://notmuchmail.org/remoteusage/ (again simplified):
>
> #!/bin/bash
> printf -v ARGS "%q " "$@"
> ssh remotehost notmuch $ARGS
>
> With the above script I'm able to use notmuch in "remote" mode.
> Quoted search terms work as expected, etc.
>
> ---
> emacs/notmuch-show.el | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 5a585f3..da2d685 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1267,9 +1267,9 @@ first relevant message.
> If no messages match the query return NIL."
> (let* ((basic-args (list notmuch-show-thread-id))
> (args (if notmuch-show-query-context
> - (append (list "\'") basic-args
> - (list "and (" notmuch-show-query-context ")\'"))
> - (append (list "\'") basic-args (list "\'"))))
> + (append basic-args
> + (list "and (" notmuch-show-query-context ")"))
> + basic-args))
> (cli-args (cons "--exclude=false"
> (when notmuch-show-elide-non-matching-messages
> (list "--entire-thread=false"))))
> --
> tg: (89c8d27..) t/remote-quoting (depends on: master)
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list