GNU bug report logs - #57795
FIXME about save-match-data in shell-command

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 57795 in the body.
You can then email your comments to 57795 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: FIXME about save-match-data in shell-command
Date: Wed, 14 Sep 2022 00:36:30 -0700

Severity: wishlist

In `shell-command', we have the following comment:

	;; Preserve the match data in case called from a program.
        ;; FIXME: It'd be ridiculous for an Elisp function to call
        ;; shell-command and assume that it won't mess the match-data!

I think we should decide to either get rid of the FIXME, or to take the
plunge and remove `save-match-data'.  (Note that the latter has been
there since 1995.)

Message #8 received at 57795 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 57795 <at> debbugs.gnu.org
Subject: Re: bug#57795: FIXME about save-match-data in shell-command
Date: Wed, 14 Sep 2022 15:25:35 +0200

Stefan Kangas <stefankangas <at> gmail.com> writes:

> In `shell-command', we have the following comment:
>
> 	;; Preserve the match data in case called from a program.
>         ;; FIXME: It'd be ridiculous for an Elisp function to call
>         ;; shell-command and assume that it won't mess the match-data!
>
> I think we should decide to either get rid of the FIXME, or to take the
> plunge and remove `save-match-data'.  (Note that the latter has been
> there since 1995.)

Hm -- I originally thought that removing that save-match-data would lead
to subtle errors in callers that depend on this behaviour.

But...  there's also code like

  (let ((handler
	 (find-file-name-handler (directory-file-name default-directory)
				 'shell-command)))
    (if handler
	(funcall handler 'shell-command command output-buffer error-buffer)
      (if (and output-buffer
               (not (string-match "[ \t]*&[ \t]*\\'" command))

before that save-match-data, so the match data will be overwritten in
many cases anyway.

The code was (originally in 1991):

+(defun shell-command (command &optional flag)
+  "Execute string COMMAND in inferior shell; display output, if any.
+If COMMAND ends in ampersand, execute it asynchronously.
+ 
+Optional second arg non-nil (prefix arg, if interactive)
+means insert output in current buffer after point (leave mark after it).
+This cannot be done asynchronously."
+  (interactive (list (read-string "Shell command: " last-shell-command)
+		     current-prefix-arg))
+  (if flag
+      (progn (barf-if-buffer-read-only)
+	     (push-mark)
+	     ;; We do not use -f for csh; we will not support broken use of
+	     ;; .cshrcs.  Even the BSD csh manual says to use
+	     ;; "if ($?prompt) exit" before things which are not useful
+	     ;; non-interactively.  Besides, if someone wants their other
+	     ;; aliases for shell commands then they can still have them.
+	     (call-process shell-file-name nil t nil
+			   "-c" command)
+	     (exchange-point-and-mark))
+    ;; Preserve the match data in case called from a program.
+    (let ((data (match-data)))

And here we see that the command does really preserve the match data in
all circumstances -- but that has been lost over the years.

So I think removing the save-match-data should probably not lead to any
regressions, so I've now removed it.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.