GNU bug report logs - #14637
recent viper-mode regression

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 14637 in the body.
You can then email your comments to 14637 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: Jim Meyering <jim <at> meyering.net>
To: bug-emacs <at> gnu.org
Subject: recent viper-mode regression
Date: Mon, 17 Jun 2013 00:59:55 +0200

Hello,
I periodically use built-from-git emacs and noticed that in the
last couple of weeks a common command in viper-mode, "!Ggrep ..."
had begun to malfunction.  Normally, !G<cmd> runs CMD on the contents
of the buffer (from the line with point to EOF) and replaces those
lines with the output of the command.

Now, however, it merely appends the result, instead of replacing the
source lines.

E.g., if you run this,

  seq 10 > k; emacs -q -nw -f viper-mode k

then type "!Ggrep 3", you'll end up with this in your buffer:
  1
  2
  3
  4
  5
  6
  7
  8
  9
  10
  3

Instead, the buffer should (and used to) contain this single line:

  3

Message #10 received at 14637-done <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Jim Meyering <jim <at> meyering.net>
Cc: 14637-done <at> debbugs.gnu.org
Subject: Re: bug#14637: recent viper-mode regression
Date: Tue, 18 Jun 2013 23:25:20 +0300

> Now, however, it merely appends the result, instead of replacing the
> source lines.

I know nothing about viper-mode, but apparently this regression is
caused by revno:112695 that now requires for the callers of
`shell-command-on-region' to explicitly specify its argument REPLACE
as documented in its docstring for a long time.  However, this fix
breaks such callers that don't care about providing the correct
non-nil REPLACE argument to behave according to the documentation.
Grepping for `shell-command-on-region' revealed the exhaustive list
of the callers that are fixed now with this patch to set both args
OUTPUT-BUFFER and REPLACE to the same value in unison:

=== modified file 'lisp/emulation/vi.el'
--- lisp/emulation/vi.el	2012-09-17 05:41:04 +0000
+++ lisp/emulation/vi.el	2013-06-18 20:17:20 +0000
@@ -1148,7 +1148,8 @@ (defun vi-shell-op (motion-command arg &
       (cond ((null shell-command)
 	     (setq shell-command (read-string "!" nil))
 	     (setq vi-last-shell-command shell-command)))
-      (shell-command-on-region begin end shell-command (not (vi-prefix-char-value arg)))
+      (shell-command-on-region begin end shell-command (not (vi-prefix-char-value arg))
+			                               (not (vi-prefix-char-value arg)))
       t)))
 
 (defun vi-shift-op (motion-command arg amount)

=== modified file 'lisp/emulation/vip.el'
--- lisp/emulation/vip.el	2013-03-12 02:08:21 +0000
+++ lisp/emulation/vip.el	2013-06-18 20:17:24 +0000
@@ -775,7 +775,7 @@ (defun vip-execute-com (m-com val com)
 		  (if (= com ?!)
 		      (setq vip-last-shell-com (vip-read-string "!"))
 		    vip-last-shell-com)
-		  t)))
+		  t t)))
 	      ((= com ?=)
 	       (save-excursion
 		 (set-mark vip-com-point)
@@ -3042,7 +3042,7 @@ (defun ex-command ()
 	  (goto-char beg)
 	  (set-mark end)
 	  (vip-enlarge-region (point) (mark))
-	  (shell-command-on-region (point) (mark) command t))
+	  (shell-command-on-region (point) (mark) command t t))
 	(goto-char beg)))))
 
 (defun ex-line-no ()

=== modified file 'lisp/emulation/viper-cmd.el'
--- lisp/emulation/viper-cmd.el	2013-05-22 03:21:30 +0000
+++ lisp/emulation/viper-cmd.el	2013-06-18 20:17:31 +0000
@@ -1548,7 +1548,7 @@ (defun viper-exec-bang (m-com com)
 		(car viper-shell-history)
 		))
        viper-last-shell-com)
-     t)))
+     t t)))
 
 (defun viper-exec-equals (m-com com)
   (save-excursion

=== modified file 'lisp/emulation/viper-ex.el'
--- lisp/emulation/viper-ex.el	2013-05-22 03:21:30 +0000
+++ lisp/emulation/viper-ex.el	2013-06-18 20:17:34 +0000
@@ -2176,7 +2176,7 @@ (defun ex-command ()
 	  (goto-char beg)
 	  (set-mark end)
 	  (viper-enlarge-region (point) (mark t))
-	  (shell-command-on-region (point) (mark t) command t))
+	  (shell-command-on-region (point) (mark t) command t t))
 	(goto-char beg)))))
 
 (defun ex-compile ()

=== modified file 'lisp/mh-e/mh-alias.el'
--- lisp/mh-e/mh-alias.el	2013-01-01 09:11:05 +0000
+++ lisp/mh-e/mh-alias.el	2013-06-18 20:17:37 +0000
@@ -141,7 +141,7 @@ (defun mh-alias-local-users ()
             (insert-file-contents "/etc/passwd")))
        ((stringp mh-alias-local-users)
         (insert mh-alias-local-users "\n")
-        (shell-command-on-region (point-min) (point-max) mh-alias-local-users t)
+        (shell-command-on-region (point-min) (point-max) mh-alias-local-users t t)
         (goto-char (point-min))))
       (while  (< (point) (point-max))
         (cond

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: 14637 <at> debbugs.gnu.org
Cc: juri <at> jurta.org, jim <at> meyering.net
Subject: Re: bug#14637: recent viper-mode regression
Date: Tue, 18 Jun 2013 20:28:18 -0400

> Grepping for `shell-command-on-region' revealed the exhaustive list
> of the callers that are fixed now with this patch to set both args
> OUTPUT-BUFFER and REPLACE to the same value in unison:

Do all the callers need to be fixed?  Most of them?  A minority only?
If it's not just a minority, then maybe the better fix is to revert
revno:112695 (or at least change it) so that existing code doesn't need
to be changed.


        Stefan

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

From: Jim Meyering <jim <at> meyering.net>
To: help-debbugs <at> gnu.org (GNU bug Tracking System)
Cc: 14637 <at> debbugs.gnu.org
Subject: Re: bug#14637: closed (Re: bug#14637: recent viper-mode regression)
Date: Wed, 19 Jun 2013 10:56:51 +0200

GNU bug Tracking System wrote:
> Your bug report
>
> #14637: recent viper-mode regression
>
> which was filed against the emacs package, has been closed.
>
> The explanation is attached below, along with your original report.
> If you require more details, please reply to 14637 <at> debbugs.gnu.org.

Confirmed.
Thank you for tracking that down and fixing it!

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

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 14637 <at> debbugs.gnu.org
Subject: Re: bug#14637: recent viper-mode regression
Date: Wed, 19 Jun 2013 23:53:07 +0300

>> Grepping for `shell-command-on-region' revealed the exhaustive list
>> of the callers that are fixed now with this patch to set both args
>> OUTPUT-BUFFER and REPLACE to the same value in unison:
>
> Do all the callers need to be fixed?  Most of them?  A minority only?
> If it's not just a minority, then maybe the better fix is to revert
> revno:112695 (or at least change it) so that existing code doesn't need
> to be changed.

I fixed all the callers that missed the required REPLACE arg:
5 were in vi mode, and 1 in mh-e.  They are just a minority
out of total 25 occurrences of `shell-command-on-region'.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: 14637 <at> debbugs.gnu.org
Subject: Re: bug#14637: recent viper-mode regression
Date: Wed, 19 Jun 2013 22:02:38 -0400

> I fixed all the callers that missed the required REPLACE arg:
> 5 were in vi mode, and 1 in mh-e.  They are just a minority
> out of total 25 occurrences of `shell-command-on-region'.

OK, then, go ahead, thank you,


        Stefan

Previous Next

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