Re: shell-command - missing shell-quote-argument for program?

[Lennart Borgman]

Eli Zaretskii wrote:
> > Date: Thu, 05 Oct 2006 16:53:10 +0200
> > From: Lennart Borgman <address@hidden>
> >
> > Lennart Borgman wrote:
> >     
> > > In some places the program to run from `shell-command' is not quoted 
> > > by `shell-quote-argument'. Examples are the calls in emerge.el.
> > >
> > > Should not the program name be quoted by `shell-quote-argment' when it 
> > > is possible?
> > >       
>
> If we quote emerge-command and the various emerge-*-program, we in
> effect disallow them to be shell commands with switches.  Is that
> reasonable?  If it is, then we should quote the programs.
>
> Note that emerge-protect-metachars assumes a Posix shell, so it will
> break on Windows with cmdproxy if the file names include whitespace.
>   

Dave Love thought that he think emerge is obsoleted by ediff. Is that 
correct?
(http://lists.gnu.org/archive/html/emacs-devel/2006-10/msg00224.html)

>   
> > And I should have mentioned `shell-command-to-string' to of course. More 
> > examples of probably missing shell quotings are in
> >
> > - filesets.el
> > - defcustom explicit-bash-args
> >     
>
> I think this indeed needs quoting.
>   

Yes, you are right. prog in the call to shell-command-to-string must be 
just a file here. (Excuse my confusion here.)

>   
> > - python-after-info-look: python-command
> >     
>
> But python.el seems to limit this to no whitespace, no?  If so,
> there's no need to quote.
>   

Could it really be true that files that are arguments to python can not 
contain spaces? Why should it apply here anyway, this is the program 
path (and perhaps something added after that).

I thought however before that it was not needed according to our earlier 
discussion, see 
http://lists.gnu.org/archive/html/emacs-devel/2006-10/msg00231.html. 
python-command could include more than just the program name, perhaps.

But on the other hand -V is concatenated to the command here so I quess 
it is just the program path and that it should be quoted.


>   
> > - flymake-get-project-include-dirs-imp: basedir should perhaps be quoted?
> >     
>
> Probably.
>
>   
Index: flymake.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/progmodes/flymake.el,v
retrieving revision 1.41
diff -c -r1.41 flymake.el
*** flymake.el    16 Feb 2006 11:40:51 -0000    1.41
--- flymake.el    11 Oct 2006 22:57:10 -0000
***************
*** 1021,1027 ****
       (progn
     (flymake-get-project-include-dirs-from-cache basedir))
     ;;else
!     (let* ((command-line  (concat "make -C\"" basedir "\" 
DUMPVARS=INCLUDE_DIRS dumpvars"))
        (output        (shell-command-to-string command-line))
        (lines         (flymake-split-string output "\n"))
        (count         (length lines))
--- 1021,1029 ----
       (progn
     (flymake-get-project-include-dirs-from-cache basedir))
     ;;else
!     (let* ((command-line  (concat "make -C\""
!                                   (shell-quote-argument basedir)
!                                   "\" DUMPVARS=INCLUDE_DIRS dumpvars"))
        (output        (shell-command-to-string command-line))
        (lines         (flymake-split-string output "\n"))
        (count         (length lines))

> > - ada-find-in-src-path
> >     
>
> Yes, but this isn't trivial, since it concats the directory with a
> wildcard.
>
>   

I do not understand quoting sufficiently well so I am not sure how to do 
this. When should shell-quote-argument be used and when should 
shell-quote-wildcard-pattern be used? The name shell-quote-argument 
suggests that it could always be used to me.

Maybe it should be done like below, but please correct me here:

Index: ada-xref.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/progmodes/ada-xref.el,v
retrieving revision 1.26
diff -c -r1.26 ada-xref.el
*** ada-xref.el    10 Feb 2006 09:00:31 -0000    1.26
--- ada-xref.el    11 Oct 2006 22:55:48 -0000
***************
*** 1916,1923 ****
       (set-buffer (get-buffer-create "*grep*"))
       (while dirs
     (insert (shell-command-to-string
!          (concat "egrep -i -h '^X|" regexp "( |$)' "
!              (file-name-as-directory (car dirs)) "*.ali")))
     (set 'dirs (cdr dirs)))

       ;;  Now parse the output
--- 1916,1926 ----
       (set-buffer (get-buffer-create "*grep*"))
       (while dirs
     (insert (shell-command-to-string
!          (concat "egrep -i -h '^X|"
!                          (shell-quote-argument regexp)
!                          "( |$)' "
!              (shell-quote-argument (file-name-as-directory (car dirs)))
!                          "*.ali")))
     (set 'dirs (cdr dirs)))

       ;;  Now parse the output

> > - ido-wide-find-dirs-or-files: several examples of missing quoting
> >     
>
> Was fixed since you wrote this, right?
>   

Yes, Kim fixed this.

>   
> > - locate.el: locate-update-command shoue perhaps be quoted? (But 
> > probably not, since it may include more than the program name. Bad 
> > structure?)
> >     
>
> I don't think it should be quoted automatically, for the reasons you
> mentioned.
>
>   
> > - fortune.el
> >     
>
> Yes.
>
>   
> > - org.el
> >     
>
> Maybe, I'm not sure I understand the semantics there.
>   

On line 9777 (in my some days old version) there is a line

     (setq cmd (format cmd file))

Carsten said the file name will be surrounded by quotes by the format 
command. I think this is a misunderstanding of what quoting means and 
that file should be quoted here and the quotes removed from the format 
string (which is cmd here). I told him this but he has not responded yet.

It also looks to me that somewhere in the construction of cmd on line 9235

       (shell-command cmd))

quoting is needed - but I have no idea of where it should be.


>   
> > - reftex-create-tags-file in reftex-global.el
> >     
>
> This was already taken care of.
>   

Yes, seems ok now. Thanks Carsten.