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

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

[Lennart Borgman]

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?

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

[Lennart Borgman]

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?

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
- python-after-info-look: python-command
- flymake-get-project-include-dirs-imp: basedir should perhaps be quoted?
- ada-find-in-src-path
- ido-wide-find-dirs-or-files: several examples of missing quoting
- locate.el: locate-update-command shoue perhaps be quoted? (But 
probably not, since it may include more than the program name. Bad 
structure?)
- fortune.el
- org.el
- reftex-create-tags-file in reftex-global.el

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

[Lennart Borgman]

Lennart Borgman wrote:
> More examples of probably missing shell quotings are in
>
> - shell.el: defcustom explicit-bash-args

I can not find any uses of explicit-bash-args in the lisp sources. I 
guess it should be removed.

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

[Andreas Schwab]

Lennart Borgman <lennart.borgman.073@student.lu.se> writes:

> I can not find any uses of explicit-bash-args in the lisp sources.

C-h f shell RET

"[...]
The shell file name (sans directories) is used to make a symbol name
such as `explicit-csh-args'.  If that symbol is a variable,
its value is used as a list of arguments when invoking the shell.
Otherwise, one argument `-i' is passed to the shell."

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Kim F. Storm]

Andreas Schwab <schwab@suse.de> writes:

> Lennart Borgman <lennart.borgman.073@student.lu.se> writes:
>
>> I can not find any uses of explicit-bash-args in the lisp sources.
>
> C-h f shell RET
>
> "[...]
> The shell file name (sans directories) is used to make a symbol name
> such as `explicit-csh-args'.  

I don't know, but I would assume there are more bash than csh users
these days?  So maybe that should be changed to explicit-bash-args ?


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

[David Kastrup]

storm@cua.dk (Kim F. Storm) writes:

> Andreas Schwab <schwab@suse.de> writes:
>
>> Lennart Borgman <lennart.borgman.073@student.lu.se> writes:
>>
>>> I can not find any uses of explicit-bash-args in the lisp sources.
>>
>> C-h f shell RET
>>
>> "[...]
>> The shell file name (sans directories) is used to make a symbol name
>> such as `explicit-csh-args'.  
>
> I don't know, but I would assume there are more bash than csh users
> these days?  So maybe that should be changed to explicit-bash-args ?

I think it should be changed to the shell name used most often on
systems that are predominantly GNU.

It may be that "explicit-sh-args" would actually be more often what is
used rather than "explicit-bash-args" since I would suspect that using
bash under the "sh" name is pretty common.

On the other hand, customizing bash-specific options into
explicit-sh-args by default does not seem like a good idea, either.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

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

[Kim F. Storm]

David Kastrup <dak@gnu.org> writes:

>> I don't know, but I would assume there are more bash than csh users
>> these days?  So maybe that should be changed to explicit-bash-args ?
>
> I think it should be changed to the shell name used most often on
> systems that are predominantly GNU.

Agree.

> It may be that "explicit-sh-args" would actually be more often what is
> used rather than "explicit-bash-args" since I would suspect that using
> bash under the "sh" name is pretty common.

I hope not!
That would defeat the troubles we go through to setup explicit-bash-args.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

[Richard Stallman]

    C-h f shell RET

    "[...]
    The shell file name (sans directories) is used to make a symbol name
    such as `explicit-csh-args'.  If that symbol is a variable,
    its value is used as a list of arguments when invoking the shell.
    Otherwise, one argument `-i' is passed to the shell."

We should have a note about this to the doc string of explicit-bash-args,
so that people looking at it understand how it is used.
Would you please add one?

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

[Andreas Schwab]

Richard Stallman <rms@gnu.org> writes:

>     C-h f shell RET
>
>     "[...]
>     The shell file name (sans directories) is used to make a symbol name
>     such as `explicit-csh-args'.  If that symbol is a variable,
>     its value is used as a list of arguments when invoking the shell.
>     Otherwise, one argument `-i' is passed to the shell."
>
> We should have a note about this to the doc string of explicit-bash-args,
> so that people looking at it understand how it is used.
> Would you please add one?

The doc string already says:

  Args passed to inferior shell by M-x shell, if the shell is bash.

IMHO this should be clear enough (there is even a link to the shell
function).  That the variable is not referenced explicitly is an
implementation detail that doesn't belong in the doc string of
explicit-bash-args.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Richard Stallman]

    IMHO this should be clear enough (there is even a link to the shell
    function).  That the variable is not referenced explicitly is an
    implementation detail that doesn't belong in the doc string of
    explicit-bash-args.

Could you put it in a comment instead, please?

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

[Andreas Schwab]

Richard Stallman <rms@gnu.org> writes:

> Could you put it in a comment instead, please?

Not sure what to put there, IMHO the doc string has already enough
information about its usage.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Richard Stallman]

    > Could you put it in a comment instead, please?

    Not sure what to put there, IMHO the doc string has already enough
    information about its usage.

The comment should explain HOW the code refers to this variable,
so that someone else won't grep for it, not find it, and delete it!

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

[Andreas Schwab]

Richard Stallman <rms@gnu.org> writes:

> The comment should explain HOW the code refers to this variable,

That's already explained in the doc string for shell.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Richard Stallman]

    > The comment should explain HOW the code refers to this variable,

    That's already explained in the doc string for shell.

That explanation isn't in the right place, and doesn't mention the
name explicit-bash-args.  People looking at the definition of
explicit-bash-args have no way of finding it.

I went to add a comment, but saw Kim Storm had already added what we
need (thanks, Kim).  Please take a look, and then you'll understand.

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

[Kim F. Storm]

Richard Stallman <rms@gnu.org> writes:

>     > Could you put it in a comment instead, please?
>
>     Not sure what to put there, IMHO the doc string has already enough
>     information about its usage.
>
> The comment should explain HOW the code refers to this variable,
> so that someone else won't grep for it, not find it, and delete it!

Done.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

[Kim F. Storm]

Lennart Borgman <lennart.borgman.073@student.lu.se> writes:

> And I should have mentioned `shell-command-to-string' to of
> course. More examples of probably missing shell quotings are in
>
> - ido-wide-find-dirs-or-files: several examples of missing quoting

Thank you for the heads up.  I've fixed ido.el

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

[Lennart Borgman]

Lennart Borgman wrote:
> 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?
>
> And I should have mentioned `shell-command-to-string' to of course. 
> More examples of probably missing shell quotings are in
>
> - filesets.el

The author of filesets Thomas Link has replied to me that he currently 
does not use Emacs and cannot maintain filesets.el until that situation 
changes.

I myself therefore suggest the following patch to filesets.el:

Index: filesets.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/filesets.el,v
retrieving revision 1.29
diff -c -r1.29 filesets.el
*** filesets.el    13 Aug 2006 17:05:12 -0000    1.29
--- filesets.el    6 Oct 2006 13:15:12 -0000
***************
*** 1701,1707 ****
            ok)
            t)))
      (when ok
!       (let ((cmd (format txt (buffer-file-name))))
      (message "Filesets: %s" cmd)
      (filesets-cmd-show-result cmd
                    (shell-command-to-string cmd))))))
--- 1701,1707 ----
            ok)
            t)))
      (when ok
!       (let ((cmd (format txt (shell-quote-argument (buffer-file-name)))))
      (message "Filesets: %s" cmd)
      (filesets-cmd-show-result cmd
                    (shell-command-to-string cmd))))))

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

[Kim F. Storm]

Lennart Borgman <lennart.borgman.073@student.lu.se> writes:

> The author of filesets Thomas Link has replied to me that he currently 
> does not use Emacs and cannot maintain filesets.el until that situation 
> changes.
>
> I myself therefore suggest the following patch to filesets.el:

Isolated, it looks ok, but if you look at filesets-quote, maybe
it should use shell-quote-argment too, and we should
use filesets-quote on buffer-file-name below.

But it's too complex for me to follow the logic in filesets.

>
> Index: filesets.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/filesets.el,v
> retrieving revision 1.29
> diff -c -r1.29 filesets.el
> *** filesets.el    13 Aug 2006 17:05:12 -0000    1.29
> --- filesets.el    6 Oct 2006 13:15:12 -0000
> ***************
> *** 1701,1707 ****
>             ok)
>             t)))
>       (when ok
> !       (let ((cmd (format txt (buffer-file-name))))
>       (message "Filesets: %s" cmd)
>       (filesets-cmd-show-result cmd
>                     (shell-command-to-string cmd))))))
> --- 1701,1707 ----
>             ok)
>             t)))
>       (when ok
> !       (let ((cmd (format txt (shell-quote-argument (buffer-file-name)))))
>       (message "Filesets: %s" cmd)
>       (filesets-cmd-show-result cmd
>                     (shell-command-to-string cmd))))))

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

[Richard Stallman]

    Isolated, it looks ok, but if you look at filesets-quote, maybe
    it should use shell-quote-argment too, and we should
    use filesets-quote on buffer-file-name below.

    But it's too complex for me to follow the logic in filesets.


Is there anyone out there who CAN figure out what is right here?

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

[Lennart Borgman]

Kim F. Storm wrote:
> Lennart Borgman <lennart.borgman.073@student.lu.se> writes:
>
>   
>> The author of filesets Thomas Link has replied to me that he currently 
>> does not use Emacs and cannot maintain filesets.el until that situation 
>> changes.
>>
>> I myself therefore suggest the following patch to filesets.el:
>>     
>
> Isolated, it looks ok, but if you look at filesets-quote, maybe
> it should use shell-quote-argment too, and we should
> use filesets-quote on buffer-file-name below.
>
> But it's too complex for me to follow the logic in filesets.
>   
I just printed filesets.el - 40 pages ;-)

Yes, I missed filesets-quote. It is used in 
filesets-get-quoted-selection. This is in turned used only in 
filesets-command:

    ("Grep <<selection>>"
     "grep"
     ("-n " filesets-get-quoted-selection " " "<<file-name>>"))

This is a shell command so in filesets-get-quoted-selection I think that 
shell-quote-argument should be used.

filesets-quote is also used in filesets-run-cmd--repl-fn:

              ((equal arg "<<file-name>>")
               (filesets-quote (buffer-file-name)))

filesets-rum-cmd--repl-fn is only used in filesets-run-cmd where it is 
used two times. The first use of this function looks like this

            (cond
             ((stringp fn)
              (let* ((args
                  (let ((txt ""))
                    (dolist (this args txt)
                      (setq txt
                        (concat txt
                            (filesets-run-cmd--repl-fn
                             this
                             (lambda (this)
                               (if (equal txt "") "" " ")
                               (format "%s" this))))))))
                 (cmd (concat fn " " args)))
                (filesets-cmd-show-result
                 cmd (shell-command-to-string cmd))))

What is intended is probably something like this:

                        (cond
                         ((stringp fn)
                          (let* ((args
                                  (let ((txt ""))
                                    (dolist (this args txt)
                                      (setq txt
                                            (concat txt
                                                    (if (equal txt "") 
"" " ")
                                                    
(shell-quote-argument this))))))
                                 (setq fn (shell-quote-argument fn))
                                 (cmd (concat fn " " args)))
                            (filesets-cmd-show-result
                             cmd (shell-command-to-string cmd))))


Coming this far I looked a bit more carefully at filesets-run-cmd. And I 
do not understand what it is doing. The code below is from this 
function. I have however replaced the name of the loop variable from 
"this" to "file". Further down in the function neither "file" or 
"buffer" is used. Seems strange to me. Can someone explain what the code 
does or what it was supposed to do? :

            (dolist (file files nil)
              (save-excursion
                (save-restriction
                  (let ((buffer (filesets-find-file file)))
                    (when buffer
                      (goto-char (point-min))




BTW the optional argument in (defun filesets-run-cmd--repl-fn (arg 
&optional format-fn) should not be optional I think. Leaving it out will 
result in (funcall nil ...).



>   
>> Index: filesets.el
>> ===================================================================
>> RCS file: /cvsroot/emacs/emacs/lisp/filesets.el,v
>> retrieving revision 1.29
>> diff -c -r1.29 filesets.el
>> *** filesets.el    13 Aug 2006 17:05:12 -0000    1.29
>> --- filesets.el    6 Oct 2006 13:15:12 -0000
>> ***************
>> *** 1701,1707 ****
>>             ok)
>>             t)))
>>       (when ok
>> !       (let ((cmd (format txt (buffer-file-name))))
>>       (message "Filesets: %s" cmd)
>>       (filesets-cmd-show-result cmd
>>                     (shell-command-to-string cmd))))))
>> --- 1701,1707 ----
>>             ok)
>>             t)))
>>       (when ok
>> !       (let ((cmd (format txt (shell-quote-argument (buffer-file-name)))))
>>       (message "Filesets: %s" cmd)
>>       (filesets-cmd-show-result cmd
>>                     (shell-command-to-string cmd))))))
>>     
>
>   


-- 
lennart.borgman.073@student.lu.se
http://OurComments.org/blog/

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

[Lennart Borgman]

Lennart Borgman wrote:
>
> Coming this far I looked a bit more carefully at filesets-run-cmd. And 
> I do not understand what it is doing. The code below is from this 
> function. I have however replaced the name of the loop variable from 
> "this" to "file". Further down in the function neither "file" or 
> "buffer" is used. Seems strange to me. Can someone explain what the 
> code does or what it was supposed to do? :
>
>            (dolist (file files nil)
>              (save-excursion
>                (save-restriction
>                  (let ((buffer (filesets-find-file file)))
>                    (when buffer
>                      (goto-char (point-min))
>
Oh, please forget this question. I did not see the find-file. That 
changes the current buffer and that is what is used later.

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

[Thomas Link]

> Yes, I missed filesets-quote. It is used in 
> filesets-get-quoted-selection. This is in turned used only in 
> filesets-command:
>
>    ("Grep <<selection>>"
>     "grep"
>     ("-n " filesets-get-quoted-selection " " "<<file-name>>"))
>
> This is a shell command so in filesets-get-quoted-selection I think 
> that shell-quote-argument should be used.
You mustn't forget that this variable may also contain lisp function, so 
using shell-quote-argument all the time probably isn't ok.
> filesets-rum-cmd--repl-fn
The idea of this function is not only to fill in file names but to 
provide some kind of template language. Eg in the last version of 
filesets (somewhere on the web but which relies on some personal 
xemacs-compatibility-library & makes use of cl which is why RMS rejected 
it), this function knows two additional patterns: <file-list> and 
<<file-list>>. I'd think that with time somebody might want to have 
other patterns which is why I personally wouldn't move quoting to 
filesets-rum-cmd.

> BTW the optional argument in (defun filesets-run-cmd--repl-fn (arg 
> &optional format-fn) should not be optional I think. Leaving it out 
> will result in (funcall nil ...).
What was I thinking.


	

	
		
___________________________________________________________ 
Der frühe Vogel fängt den Wurm. Hier gelangen Sie zum neuen Yahoo! Mail: http://mail.yahoo.de

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

[Thomas Link]

> !       (let ((cmd (format txt (shell-quote-argument 
> (buffer-file-name)))))
Looks good to me. The function filesets-cmd-shell-command is called when 
selecting "Run Command" from the menu and when then selecting "Run Shell 
Command". The user will be prompted for a command string that is then 
passed through format replacing %s with the current buffer file name. 
Without the shell-quote-argument this most likely resulted in an file 
not found error when the file name contained blanks or other special 
characters.

I really don't have the time to dig deeper into this but I think that 
filesets-quote in the function filesets-run-cmd--repl-fn should be 
replaced with shell-quote-argument too.

Regards,
Thomas.


		
___________________________________________________________ 
Telefonate ohne weitere Kosten vom PC zum PC: http://messenger.yahoo.de

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

[Lennart Borgman]

Thomas Link wrote:
>
>> !       (let ((cmd (format txt (shell-quote-argument 
>> (buffer-file-name)))))
> Looks good to me. The function filesets-cmd-shell-command is called 
> when selecting "Run Command" from the menu and when then selecting 
> "Run Shell Command". The user will be prompted for a command string 
> that is then passed through format replacing %s with the current 
> buffer file name. Without the shell-quote-argument this most likely 
> resulted in an file not found error when the file name contained 
> blanks or other special characters.
>
> I really don't have the time to dig deeper into this but I think that 
> filesets-quote in the function filesets-run-cmd--repl-fn should be 
> replaced with shell-quote-argument too.
>
> Regards,
Thomas, could you give an example of what txt could be here?

Re: [Bulk] Re: shell-command - missing shell-quote-argument for program?

[Thomas Link]

> Thomas, could you give an example of what txt could be here?

All of this can be customized via the filesets-commands variable. This 
really is just a proposal for a hopefully useful command. It's not 
essential.

The call originates in filesets-run-cmd that runs a command as defined 
in filesets-commands on the files belonging to a fileset. 
filesets-run-cmd can be called via the menu or interactively from the 
command line. A command can be either a function or a string (which is 
interpreted as name of a shell command). By default, the following 
commands are defined:

     - Query Replace
     - Query Replace (regexp)
     - Grep <<selection>>
     - Run Shell Command

The command in question is the last one on the list, which could be used 
for running rarely used shell commands that aren't worth customizing 
filesets-commands.

If you look at filesets-commands, you'll see that "Run Shell Command" is 
defined as (filesets-cmd-shell-command 
(filesets-cmd-shell-command-getargs)), where 
filesets-cmd-shell-command-getargs does nothing but query the user for a 
shell command. So, txt is the user input, i.e. some text/string like 
"grep foo %s".

BTW in filesets-commands, filesets uses <<file-name>> to insert the 
buffer's filename, so this is a minor inconsistency but %s is shorter & 
more convenient in this situation.

HTH.


		
___________________________________________________________ 
Telefonate ohne weitere Kosten vom PC zum PC: http://messenger.yahoo.de

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

[Richard Stallman]

Since Thomas thinks your change is correct, would you please install it?

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

[Eli Zaretskii]

> Date: Fri, 06 Oct 2006 15:20:51 +0200
> From: Lennart Borgman <lennart.borgman.073@student.lu.se>
> Cc: Thomas Link <t.link@gmx.at>
> 
> The author of filesets Thomas Link has replied to me that he currently 
> does not use Emacs and cannot maintain filesets.el until that situation 
> changes.
> 
> I myself therefore suggest the following patch to filesets.el:

Thanks, I installed it.

In the future, please make sure that your mailer does not convert TABs
into spaces, since that causes Patch to fail.  Also, please include a
ChangeLog entry with the patch.

Btw, would you like to submit patches for the other places where such
quoting is required?

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

[Lennart Borgman]

Dave Love wrote:
> Lennart Borgman <lennart.borgman.073@student.lu.se> writes:
>
>   
>> Hi all Authors,
>>
>> I mail you all directly about the possible bugs below in case you do
>> not read the Emacs Devel list. It would be very nice if you could fix
>> these bugs. (There was another such bug in ediff which I mailed
>> Michael Kifer seperately earlier.)
>>     
>
> I'm sorry, but I don't know why you're sending this to me -- I can't
> get things fixed in Emacs.  Anyhow, I don't understand what you're
> talking about, so I suspect others won't.  You need to be explicit in
> a bug report and send it to the maintainer(s) who can deal with it.
>
> By the way, as far as I know, emerge is obsoleted by ediff.  It should
> be in lisp/obsolete so as not have time wasted on it.  rms demurred on
> that, but for no good reason as far as I remember.
>   
Hi Dave,

Thanks for responding. I am sorry if I were not clear enough about the 
problem. The problem is that when you call shell-command (or 
shell-command-to-string) the arguments may need to be quoted so that the 
shell does not mistreat them. A common example is file names with spaces 
in them. If a file name with spaces are used as argument to 
shell-command it must be quoted. Otherwise the shell will treat it as 
several arguments.

I found examples of what I suspected was missing quoting in the files I 
mentioned and I therefore mailed you as authors (since there is no 
maintainer mentioned in the files).

So far Kim Storm has responded and fixed the problem in ido.el. I would 
be very glad if you (I mean the authors here) tried to point out where 
the call to shell-quote-argument should be inserted to avoid the 
problem. It seems much simpler if you do it. However if you do not do 
that I will try to do that myself in a couple of days.

Kind regards,
L


Here are the files again:

- emerge.el
According to Dave it is probably obsolete and replaced by ediff. Then I 
will just leave emerge.el as it is.

- filesets.el

- shell.el: defcustom explicit-bash-args <- never used?
This was a misunderstanding on my side. Resolved on the devel list.

- progmodes/python.el: python-after-info-look: python-command

- progmodes/flymake.el: flymake-get-project-include-dirs-imp: basedir 
should perhaps be quoted?

- progmodes/ada-xref.el: ada-find-in-src-path

- ido.el: ido-wide-find-dirs-or-files: several examples of missing quoting
Kim has fixed ido.el.

- locate.el: locate-update-command shoue perhaps be quoted? (But 
probably not, since it may include more than the program name. Bad 
structure?)

- fortune.el

- org.el

- reftex-global.el: reftex-create-tags-file

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

[Slawomir Nowaczyk]

On Tue, 10 Oct 2006 00:14:46 +0200
Lennart Borgman <lennart.borgman.073@student.lu.se> wrote:

<snip>
#> I found examples of what I suspected was missing quoting in the files
#> I mentioned and I therefore mailed you as authors (since there is no
#> maintainer mentioned in the files).

<snip>

#> Here are the files again:
#> 
#> - progmodes/python.el: python-after-info-look: python-command

I am not entirely sure it is needed, but the following shouldn't hurt in
any case.

**********************************************************************

--- m:/EmacsCVS/EmacsCVS/lisp/progmodes/python.el       2006-09-26 20:28:22.912208000 +0200
+++ c:/Emacs/lisp/progmodes/python.el   2006-10-10 08:39:42.179456000 +0200
@@ -1630,7 +1630,7 @@
 (defun python-after-info-look ()
   "Set up info-look for Python.
 Used with `eval-after-load'."
-  (let* ((version (let ((s (shell-command-to-string (concat python-command
+  (let* ((version (let ((s (shell-command-to-string (concat (shell-quote-argument python-command)
                                                            " -V"))))
                    (string-match "^Python \\([0-9]+\\.[0-9]+\\>\\)" s)
                    (match-string 1 s)))

**********************************************************************

-- 
 Best wishes,
   Slawomir Nowaczyk
     ( slawomir.nowaczyk.847@student.lu.se )

Never be afraid to try something new. Remember, amateurs built
the ark. Professionals built the Titanic.

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

[Lennart Borgman]

Slawomir Nowaczyk wrote:
> On Tue, 10 Oct 2006 00:14:46 +0200
> Lennart Borgman <lennart.borgman.073@student.lu.se> wrote:
>
> <snip>
> #> I found examples of what I suspected was missing quoting in the files
> #> I mentioned and I therefore mailed you as authors (since there is no
> #> maintainer mentioned in the files).
>
> <snip>
>
> #> Here are the files again:
> #> 
> #> - progmodes/python.el: python-after-info-look: python-command
>
> I am not entirely sure it is needed, but the following shouldn't hurt in
> any case.
>
> **********************************************************************
>
> --- m:/EmacsCVS/EmacsCVS/lisp/progmodes/python.el       2006-09-26 20:28:22.912208000 +0200
> +++ c:/Emacs/lisp/progmodes/python.el   2006-10-10 08:39:42.179456000 +0200
> @@ -1630,7 +1630,7 @@
>  (defun python-after-info-look ()
>    "Set up info-look for Python.
>  Used with `eval-after-load'."
> -  (let* ((version (let ((s (shell-command-to-string (concat python-command
> +  (let* ((version (let ((s (shell-command-to-string (concat (shell-quote-argument python-command)
>                                                             " -V"))))
>                     (string-match "^Python \\([0-9]+\\.[0-9]+\\>\\)" s)
>                     (match-string 1 s)))
>
> **********************************************************************
>   

Thanks, but it looks to me that this should not be quoted. 
`python-command' is not just a file.

My mistake. Sorry.

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

[Slawomir Nowaczyk]

On Tue, 10 Oct 2006 09:02:24 +0200
Lennart Borgman <lennart.borgman.073@student.lu.se> wrote:

#> > I am not entirely sure it is needed, but the following shouldn't
#> > hurt in any case.
#> > <snip>
#> 
#> Thanks, but it looks to me that this should not be quoted.
#> `python-command' is not just a file.
#> 
#> My mistake. Sorry.

Right, it looks like python-command may include arguments that should be
passed to the interpreter. My patch is obviously wrong.

Seems like it is up to a user customising this variable to make sure any
meta characters are escaped properly... Does anyone think it would make
sense to add some information about it to the doc string? Or are
problems with that too unlikely to worry about them?

-- 
 Best wishes,
   Slawomir Nowaczyk
     ( slawomir.nowaczyk.847@student.lu.se )

A man needs a wife because, sooner or later, something is bound to
happen that he can't blame on the Government.

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

[Kim F. Storm]

Slawomir Nowaczyk <slawomir.nowaczyk.847@student.lu.se> writes:

> On Tue, 10 Oct 2006 09:02:24 +0200
> Lennart Borgman <lennart.borgman.073@student.lu.se> wrote:
>
> #> > I am not entirely sure it is needed, but the following shouldn't
> #> > hurt in any case.
> #> > <snip>
> #> 
> #> Thanks, but it looks to me that this should not be quoted.
> #> `python-command' is not just a file.
> #> 
> #> My mistake. Sorry.
>
> Right, it looks like python-command may include arguments that should be
> passed to the interpreter. My patch is obviously wrong.
>
> Seems like it is up to a user customising this variable to make sure any
> meta characters are escaped properly... Does anyone think it would make
> sense to add some information about it to the doc string? Or are
> problems with that too unlikely to worry about them?

If the user messes up the variable, he will notice immediately the next
time he tries the related command.

We only need to focus on getting things right for arguments which
are problematic, e.g. when user enters a file name with spaces or
a file name read from a directory contains spaces...

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

[Dave Love]

Lennart Borgman <lennart.borgman.073@student.lu.se> writes:

> Thanks for responding. I am sorry if I were not clear enough about the
> problem. The problem is that when you call shell-command (or
> shell-command-to-string) the arguments may need to be quoted so that
> the shell does not mistreat them.

??  Shell-quoting the args would break them unless they comprise a
single shell word, in which case there's no point.

> A common example is file names with
> spaces in them. If a file name with spaces are used as argument to
> shell-command it must be quoted. Otherwise the shell will treat it as
> several arguments.

Well, that's different; maybe that's what you meant by the first bit.
If you're composing a shell command you have to DTRT, but
`shell-command-to-string' is probably the least of your problems with
file names that shells split.  You shouldn't normally use
`shell-command' non-interactively.

Why don't you submit patches?  It's still not clear to me what you're
complaining about.  There are surely more serious Emacs issues to work
on, though.

> I found examples of what I suspected was missing quoting in the files
> I mentioned and I therefore mailed you as authors (since there is no
> maintainer mentioned in the files).

If you sent to me because of python.el, the maintainer is supposed to
be FSF, unfortunately.  (I assume you mean the version in the Emacs
sources rather than the fixed and enhanced one I maintain.)

> - emerge.el
> According to Dave it is probably obsolete and replaced by ediff. Then
> I will just leave emerge.el as it is.

If it's not in lisp/obsolete it still needs to be maintained.  It's
also documented in the manual, rather than Ediff, unfortunately.

> - progmodes/python.el: python-after-info-look: python-command

I don't have the Emacs CVS version to hand, but if that has a problem,
please chase whoever changed it.  In my version the only instance of
`shell-command' is:
   (shell-command-to-string (concat python-command " -V"))
and I don't understand what you mean by referring to python-command.
Have you read its doc (or has that been changed too)?

The thing that is/was wrong in principle with command arg processing
in python.el is `python-args-to-list' -- see its fixme.  Similarly for
anything else that's copied the mechanism from cmuscheme.el.  (The
inferior interpreter mechanism is one of the things which should be
abstracted, though, rather than fixing individual versions IMNSHO.)

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

[Lennart Borgman]

Dave Love wrote:
> Lennart Borgman <lennart.borgman.073@student.lu.se> writes:
>
>   
>> Thanks for responding. I am sorry if I were not clear enough about the
>> problem. The problem is that when you call shell-command (or
>> shell-command-to-string) the arguments may need to be quoted so that
>> the shell does not mistreat them.
>>     
>
> ??  Shell-quoting the args would break them unless they comprise a
> single shell word, in which case there's no point.
>   
There must be some misunderstanding here, but please let us not discuss 
this now. I think we have resolved most of the issues I have been 
complaining about here.

>   
>> A common example is file names with
>> spaces in them. If a file name with spaces are used as argument to
>> shell-command it must be quoted. Otherwise the shell will treat it as
>> several arguments.
>>     
>
> Well, that's different; maybe that's what you meant by the first bit.
> If you're composing a shell command you have to DTRT, but
> `shell-command-to-string' is probably the least of your problems with
> file names that shells split.  You shouldn't normally use
> `shell-command' non-interactively.
>
> Why don't you submit patches?  It's still not clear to me what you're
> complaining about.  There are surely more serious Emacs issues to work
> on, though.
>   
These are possible bugs and I consider them serious. The most common 
problem is that with file names containing spaces.

>   
>> I found examples of what I suspected was missing quoting in the files
>> I mentioned and I therefore mailed you as authors (since there is no
>> maintainer mentioned in the files).
>>     
>
> If you sent to me because of python.el, the maintainer is supposed to
> be FSF, unfortunately.  (I assume you mean the version in the Emacs
> sources rather than the fixed and enhanced one I maintain.)
>   
Yes, but I wanted your opinion only.

>   
>> - emerge.el
>> According to Dave it is probably obsolete and replaced by ediff. Then
>> I will just leave emerge.el as it is.
>>     
>
> If it's not in lisp/obsolete it still needs to be maintained.  It's
> also documented in the manual, rather than Ediff, unfortunately.
>   
Looks like you are right.

>   
>> - progmodes/python.el: python-after-info-look: python-command
>>     
>
> I don't have the Emacs CVS version to hand, but if that has a problem,
> please chase whoever changed it.  In my version the only instance of
> `shell-command' is:
>    (shell-command-to-string (concat python-command " -V"))
> and I don't understand what you mean by referring to python-command.
> Have you read its doc (or has that been changed too)?
>   
My impression right now is that this special case could not be quoted 
since python-command could be a file name with arguments.

> The thing that is/was wrong in principle with command arg processing
> in python.el is `python-args-to-list' -- see its fixme.  Similarly for
> anything else that's copied the mechanism from cmuscheme.el.  (The
> inferior interpreter mechanism is one of the things which should be
> abstracted, though, rather than fixing individual versions IMNSHO.)
>   
Thanks for the pointer. Has this been fixed in the version of python.el 
that you maintain yourself?

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

[Dave Love]

Lennart Borgman <lennart.borgman.073@student.lu.se> writes:

>> I don't have the Emacs CVS version to hand, but if that has a problem,
>> please chase whoever changed it.  In my version the only instance of
>> `shell-command' is:
>>    (shell-command-to-string (concat python-command " -V"))
>> and I don't understand what you mean by referring to python-command.
>> Have you read its doc (or has that been changed too)?
>>
> My impression right now is that this special case could not be quoted
> since python-command could be a file name with arguments.

If you think there's any doubt about that expression, I'm afraid
you're quite confused, and I suspect you're confused about other
things.

>> The thing that is/was wrong in principle with command arg processing
>> in python.el is `python-args-to-list' -- see its fixme.  Similarly for
>> anything else that's copied the mechanism from cmuscheme.el.  (The
>> inferior interpreter mechanism is one of the things which should be
>> abstracted, though, rather than fixing individual versions IMNSHO.)
>>
> Thanks for the pointer. Has this been fixed in the version of
> python.el that you maintain yourself?

No, that would be a waste of time; forget I mentioned it.

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

[Lennart Borgman]

Dave Love wrote:
> Lennart Borgman <lennart.borgman.073@student.lu.se> writes:
>
>   
>>> I don't have the Emacs CVS version to hand, but if that has a problem,
>>> please chase whoever changed it.  In my version the only instance of
>>> `shell-command' is:
>>>    (shell-command-to-string (concat python-command " -V"))
>>> and I don't understand what you mean by referring to python-command.
>>> Have you read its doc (or has that been changed too)?
>>>
>>>       
>> My impression right now is that this special case could not be quoted
>> since python-command could be a file name with arguments.
>>     
>
> If you think there's any doubt about that expression, I'm afraid
> you're quite confused, and I suspect you're confused about other
> things.
>   

I am sorry, you are wasting your time. I am not able to be impressed.

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

[Eli Zaretskii]

> Date: Thu, 05 Oct 2006 16:53:10 +0200
> From: Lennart Borgman <lennart.borgman.073@student.lu.se>
> 
> 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.

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

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

> - flymake-get-project-include-dirs-imp: basedir should perhaps be quoted?

Probably.

> - ada-find-in-src-path

Yes, but this isn't trivial, since it concats the directory with a
wildcard.

> - ido-wide-find-dirs-or-files: several examples of missing quoting

Was fixed since you wrote this, right?

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

> - reftex-create-tags-file in reftex-global.el

This was already taken care of.

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 <lennart.borgman.073@student.lu.se>
>>
>> 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.

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

[Andreas Schwab]

Lennart Borgman <lennart.borgman.073@student.lu.se> writes:

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

This is wrong, the argument is already enclosed in single quotes.  There
is no need for further quoting, since the regexp cannot contain single
quotes (it is constructed from an Ada identifier).

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Eli Zaretskii]

> From: Andreas Schwab <schwab@suse.de>
> Cc: Eli Zaretskii <eliz@gnu.org>,
> 	Carsten Dominik <dominik@science.uva.nl>, emacs-devel@gnu.org
> Date: Sat, 14 Oct 2006 19:50:01 +0200
> 
> > 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)
> 
> This is wrong, the argument is already enclosed in single quotes.

Actually, I think that using '..' quoting is wrong, because it assumes
a Posix shell.  I think this should be rewritten by removing the
single quotes and instead quoting (via shell-quote-argument) the
entire regexp that is the result of `(concat "^X|" regexp "( |$)")'.
Do you agree that this is more portable?

Also, I thought that the issue here was with quoting the elements of
the `dirs' list, since they are file names that could include embedded
whitespace and other characters special to the shell.

Btw, while we are at that, `egrep' might not work with the latest
versions of GNU Grep, where `egrep' is a shell script that requires
/bin/sh.  "grep -E" is better, I think.

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

[Lennart Borgman]

[-- Attachment #1: Type: text/plain, Size: 1781 bytes --]

Eli Zaretskii wrote:
>> From: Andreas Schwab <schwab@suse.de>
>> Cc: Eli Zaretskii <eliz@gnu.org>,
>> 	Carsten Dominik <dominik@science.uva.nl>, emacs-devel@gnu.org
>> Date: Sat, 14 Oct 2006 19:50:01 +0200
>>
>>     
>>> 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)
>>>       
>> This is wrong, the argument is already enclosed in single quotes.
>>     
>
> Actually, I think that using '..' quoting is wrong, because it assumes
> a Posix shell.  I think this should be rewritten by removing the
> single quotes and instead quoting (via shell-quote-argument) the
> entire regexp that is the result of `(concat "^X|" regexp "( |$)")'.
> Do you agree that this is more portable?
>
> Also, I thought that the issue here was with quoting the elements of
> the `dirs' list, since they are file names that could include embedded
> whitespace and other characters special to the shell.
>
> Btw, while we are at that, `egrep' might not work with the latest
> versions of GNU Grep, where `egrep' is a shell script that requires
> /bin/sh.  "grep -E" is better, I think.
>   
Attached a new patch.

[-- Attachment #2: ada-xref.el --]
[-- Type: text/plain, Size: 1086 bytes --]

Index: lisp/progmodes/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
*** lisp/progmodes/ada-xref.el	10 Feb 2006 09:00:31 -0000	1.26
--- lisp/progmodes/ada-xref.el	15 Oct 2006 09:22:19 -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,1927 ----
        (set-buffer (get-buffer-create "*grep*"))
        (while dirs
  	(insert (shell-command-to-string
! 		 (concat
!                   "grep -E -i -h "
!                   (shell-quote-argument (concat "^X|" regexp "( |$)"))
!                   " "
!                   (shell-quote-argument (file-name-as-directory (car dirs)))
!                   "*.ali")))
  	(set 'dirs (cdr dirs)))
  
        ;;  Now parse the output

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

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

[Kim F. Storm]

Lennart Borgman <lennart.borgman.073@student.lu.se> writes:

>> Btw, while we are at that, `egrep' might not work with the latest
>> versions of GNU Grep, where `egrep' is a shell script that requires
>> /bin/sh.  "grep -E" is better, I think.
>>   
> Attached a new patch.
> Index: lisp/progmodes/ada-xref.el

Thanks.  Installed.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

[Eli Zaretskii]

> Date: Sat, 14 Oct 2006 17:33:01 +0200
> From: Lennart Borgman <lennart.borgman.073@student.lu.se>
> CC:  emacs-devel@gnu.org, Carsten Dominik <dominik@science.uva.nl>
> 
> 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)

It may be obsolete, but as long as we don't remove it from Emacs, it
better be right.

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

I was talking about the program name, not the file arguments.

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

[Kim F. Storm]

Lennart Borgman <lennart.borgman.073@student.lu.se> writes:

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


Shouldn't you remove the old quotes here?

    (let* ((command-line  (concat "make -C "
                                  (shell-quote-argument basedir)
                                  " DUMPVARS=INCLUDE_DIRS dumpvars"))

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


This looks more correct to me:

          (concat "egrep -i -h "
                  (shell-quote-argument (concat "^X|" regexp "( |$)")) " "
                  (shell-quote-argument (file-name-as-directory (car dirs))) "*.ali")))


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

[Lennart Borgman]

Kim F. Storm wrote:
> Lennart Borgman <lennart.borgman.073@student.lu.se> writes:
>
>   
>> 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))
>>     
>
>
> Shouldn't you remove the old quotes here?
>
>     (let* ((command-line  (concat "make -C "
>                                   (shell-quote-argument basedir)
>                                   " DUMPVARS=INCLUDE_DIRS dumpvars"))
>   
Yes, of course. Thanks.

>   
>> 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
>>     
>
>
> This looks more correct to me:
>
>           (concat "egrep -i -h "
>                   (shell-quote-argument (concat "^X|" regexp "( |$)")) " "
>                   (shell-quote-argument (file-name-as-directory (car dirs))) "*.ali")))
>
>   
Yes, I misread it totally.

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

[Lennart Borgman]

[-- Attachment #1: Type: text/plain, Size: 1859 bytes --]

Lennart Borgman wrote:
> Kim F. Storm wrote:
>> Lennart Borgman <lennart.borgman.073@student.lu.se> writes:
>>
>>  
>>> 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))
>>>     
>>
>>
>> Shouldn't you remove the old quotes here?
>>
>>     (let* ((command-line  (concat "make -C "
>>                                   (shell-quote-argument basedir)
>>                                   " DUMPVARS=INCLUDE_DIRS dumpvars"))
>>   
> Yes, of course. Thanks.
It would have been better if I had attached the new patch too ;-)
Here it is.

(And excuse the name of the file name of the last patch I sent. It 
should of course have been something like ada-xref-el.patch)

[-- Attachment #2: flymake-el.patch --]
[-- Type: text/plain, Size: 1146 bytes --]

Index: lisp/progmodes/flymake.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/progmodes/flymake.el,v
retrieving revision 1.41
diff -c -r1.41 flymake.el
*** lisp/progmodes/flymake.el	16 Feb 2006 11:40:51 -0000	1.41
--- lisp/progmodes/flymake.el	15 Oct 2006 09:17:25 -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))

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

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

[Kim F. Storm]

Lennart Borgman <lennart.borgman.073@student.lu.se> writes:

> It would have been better if I had attached the new patch too ;-)
> Here it is.

> Index: lisp/progmodes/flymake.el
> ===================================================================

Thanks.  Installed.

--
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

[Richard Stallman]

    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)

He often judges such questions based on premises that are rather
different from the ones I would use.  So I can't assume I would
agree with him if I had studied the same facts.  I'd like to see
the evaluation of someone whose judgment I can rely on,
before making it obsolete.

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

[Slawomir Nowaczyk]

On Sat, 14 Oct 2006 17:33:01 +0200
Lennart Borgman <lennart.borgman.073@student.lu.se> wrote:

#> >> - 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.

Doc string of python-command contains this: "Note that IPython may not
work properly; it must at least be used with the `-cl' flag, i.e. use
`ipython -cl'." I do not know if python-mode really works with
IronPython, but if we are advising people to include flags in
python-command, we probably should not be quoting it.

-- 
 Best wishes,
   Slawomir Nowaczyk
     ( slawomir.nowaczyk.847@student.lu.se )

Smoking helps you lose weight -- one lung at a time!

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

[Richard Stallman]

    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.

If that's what these variables do, their values should not be quoted,
so that they can include some arguments.

    > - defcustom explicit-bash-args

Since that exists only to be sent as arguments,
it should not be quoted.  The user will put in whatever
quoting is called for.

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

[Lennart Borgman]

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

Richard Stallman wrote:
>     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.
>
> If that's what these variables do, their values should not be quoted,
> so that they can include some arguments.
>   

Ok.

>     > - defcustom explicit-bash-args
>
> Since that exists only to be sent as arguments,
> it should not be quoted.  The user will put in whatever
> quoting is called for.
>
>   

That is a misunderstanding. It is the variable prog inside that 
defcustom that should be quoted with shell-quote-argument. prog must 
point to a file so there is no problem here.

I have attached a patch.

[-- Attachment #2: shell-el.diff --]
[-- Type: text/plain, Size: 935 bytes --]

Index: lisp/shell.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/shell.el,v
retrieving revision 1.140
diff -c -r1.140 shell.el
*** lisp/shell.el	2 Sep 2006 23:30:21 -0000	1.140
--- lisp/shell.el	15 Oct 2006 19:11:30 -0000
***************
*** 293,299 ****
  	     (equal name "bash")
  	     (file-executable-p prog)
  	     (string-match "bad option"
! 			   (shell-command-to-string (concat prog " --noediting"))))
  	'("-i")
        '("--noediting" "-i")))
    "Args passed to inferior shell by \\[shell], if the shell is bash.
--- 293,300 ----
  	     (equal name "bash")
  	     (file-executable-p prog)
  	     (string-match "bad option"
! 			   (shell-command-to-string
!                             (concat (shell-quote-argument prog) " --noediting"))))
  	'("-i")
        '("--noediting" "-i")))
    "Args passed to inferior shell by \\[shell], if the shell is bash.

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

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

[Kim F. Storm]

Lennart Borgman <lennart.borgman.073@student.lu.se> writes:

> That is a misunderstanding. It is the variable prog inside that
> defcustom that should be quoted with shell-quote-argument. prog must
> point to a file so there is no problem here.
>
> I have attached a patch.
> Index: lisp/shell.el
> ===================================================================

Thanks.  Installed.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

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

[Stefan Monnier]

> Should not the program name be quoted by `shell-quote-argment' when it
> is possible?

I haven't looked at those cases, but I know that in some cases this is done
on purpose so that you can use a program name such as
"/some/command -arg1 -arg2".


        Stefan

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

[Eli Zaretskii]

> Date: Thu, 05 Oct 2006 16:34:04 +0200
> From: Lennart Borgman <lennart.borgman.073@student.lu.se>
> 
> Should not the program name be quoted by `shell-quote-argment' when it 
> is possible?

Only when Emacs generates the program name itself.  When the program
comes from the user typing its name (typically, into the minibuffer),
it's the user's responsibility to quote whatever arguments need that,
precisely as she would when typing the command at the shell's prompt.
(Stefan pointed out one situation where the latter behavior is quite
necessary, but there are other similar cases as well.)

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

[Lennart Borgman]

Eli Zaretskii wrote:
>> Date: Thu, 05 Oct 2006 16:34:04 +0200
>> From: Lennart Borgman <lennart.borgman.073@student.lu.se>
>>
>> Should not the program name be quoted by `shell-quote-argment' when it 
>> is possible?
>>     
>
> Only when Emacs generates the program name itself.  When the program
> comes from the user typing its name (typically, into the minibuffer),
> it's the user's responsibility to quote whatever arguments need that,
> precisely as she would when typing the command at the shell's prompt.
> (Stefan pointed out one situation where the latter behavior is quite
> necessary, but there are other similar cases as well.)
>   
Thanks, I see. Is that pointed out somewhere in the manual already?

But this does not change the need for quoting the file argument in those 
cases I pointed out. I am however not quite sure about what to do so I 
would like some suggestions.