From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: "Basil L. Contovounesios" Newsgroups: gmane.emacs.bugs Subject: bug#30280: async-shell-command-display-buffer doesn't work anymore Date: Sun, 06 May 2018 17:18:32 +0100 Message-ID: <874ljkn4fb.fsf@tcd.ie> References: <87a7wxd1g9.fsf@mail.linkov.net> <83607kinmx.fsf@gnu.org> <877erz41re.fsf@tcd.ie> <87zi4tu2ic.fsf@mail.linkov.net> <83lggbd661.fsf@gnu.org> <87tvuyqhz8.fsf@tcd.ie> <87tvuxojag.fsf@mail.linkov.net> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1525623427 14685 195.159.176.226 (6 May 2018 16:17:07 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 6 May 2018 16:17:07 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: rrt@sc3d.org, 30280@debbugs.gnu.org, tino calancha To: Juri Linkov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun May 06 18:17:03 2018 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fFMLO-0003iF-Rr for geb-bug-gnu-emacs@m.gmane.org; Sun, 06 May 2018 18:17:03 +0200 Original-Received: from localhost ([::1]:42675 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fFMNV-0008Ri-S8 for geb-bug-gnu-emacs@m.gmane.org; Sun, 06 May 2018 12:19:13 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:37090) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fFMNO-0008RY-9O for bug-gnu-emacs@gnu.org; Sun, 06 May 2018 12:19:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fFMNK-0004of-SU for bug-gnu-emacs@gnu.org; Sun, 06 May 2018 12:19:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:43673) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fFMNK-0004oZ-Kq for bug-gnu-emacs@gnu.org; Sun, 06 May 2018 12:19:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fFMNK-0006Fr-Es for bug-gnu-emacs@gnu.org; Sun, 06 May 2018 12:19:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: "Basil L. Contovounesios" Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 06 May 2018 16:19:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 30280 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 30280-submit@debbugs.gnu.org id=B30280.152562352424016 (code B ref 30280); Sun, 06 May 2018 16:19:02 +0000 Original-Received: (at 30280) by debbugs.gnu.org; 6 May 2018 16:18:44 +0000 Original-Received: from localhost ([127.0.0.1]:51570 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fFMN0-0006FH-9H for submit@debbugs.gnu.org; Sun, 06 May 2018 12:18:44 -0400 Original-Received: from mail-wm0-f43.google.com ([74.125.82.43]:39400) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fFMMy-0006F3-Hg for 30280@debbugs.gnu.org; Sun, 06 May 2018 12:18:41 -0400 Original-Received: by mail-wm0-f43.google.com with SMTP id f8-v6so12188084wmc.4 for <30280@debbugs.gnu.org>; Sun, 06 May 2018 09:18:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tcd-ie.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:references:mail-followup-to:date:in-reply-to :message-id:user-agent:mime-version; bh=wUP+KrBp892PP7Uu+T7Nt1l7TOhLxPjoIuZtDdt3Kws=; b=Ma7jXP0QgLuPus+hlYYwnlidxf7urIhMoinUPpi+yd+XMVXhd8IMtryqqeyCeC0npg CsGlbtuJ3/kk5lT4dJvNOGFU/fxDGEdBhpZV4XkY2AJX0ngT/6ZHtSBi8w+7QNDzbUPD Q+59lz3oHp5Tpt63HIzexwbxqnxlb9R6t52WgK1k3Qti+2zjgXwuJaaUgvb5bfQLZEtL mQdQgRkRtP+5KfGeJU+YBZwf79vMNqjyFskEb3ydN07TeB1Fdup0bD3eDcAFuYRif25a RAdz2Aij1gDyU5tYkoAneP2R5vQ/SE1OIaG/jWrFrYHEdX9WAsVuhDkxBuzRdD098ZbS M+4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:mail-followup-to :date:in-reply-to:message-id:user-agent:mime-version; bh=wUP+KrBp892PP7Uu+T7Nt1l7TOhLxPjoIuZtDdt3Kws=; b=AGNAtsCYsPRqk54IJxnmv7usHL4UznIvhxFWIazTjyYaWR8Kw6Wug/qxmhGfJpWjDn knUdIEqjPCXLXHsmfbrfaF9ZcQtHv8YH4DS80r3H24LAz7dHKWvsBS9HMe66uIyLokC/ 7QXxhmDwQJ0HAKwRCJZKOOQgeofdmK7fyIGaGHT/y1JI0nlcFUX2b+zGEGOxo45DaksC ewwApGfa1WVq49I+TRLhx7AnozeMXGoSJbRJx78igHUQNwLzDWBsbnI/7m0VcfTwOFo6 7/Xwwud6Dig0ah12mDoQm3puXgpEobyjkxs7RM9sxRXOLvs28pvyV06DpOSK97AauFDB 1Svw== X-Gm-Message-State: ALQs6tA9Ig8lG4xhIlRv4hv5YfsPuv/AHSF0eHWyyKZWKacXSXM6gdqe 7583FfvVPg5GoP84AsYwnStM2Q== X-Google-Smtp-Source: AB8JxZpuy5YFLuKqX1aybUnTjB882tG++WGTbjXhXBVT5k4z7dI2OgSxjKsVBhR4Nhvs0nR+o4Su+Q== X-Received: by 2002:a50:8c05:: with SMTP id p5-v6mr45576889edp.282.1525623514909; Sun, 06 May 2018 09:18:34 -0700 (PDT) Original-Received: from localhost ([213.233.148.17]) by smtp.gmail.com with ESMTPSA id h51-v6sm9593021eda.88.2018.05.06.09.18.33 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 06 May 2018 09:18:34 -0700 (PDT) Mail-Followup-To: Juri Linkov , Eli Zaretskii , , <30280@debbugs.gnu.org>, tino calancha In-Reply-To: <87tvuxojag.fsf@mail.linkov.net> (Juri Linkov's message of "Sat, 3 Feb 2018 23:27:51 +0200") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:146046 Archived-At: --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Fix-failing-test-for-bug-30280.patch >From 871cc1bd0b4ef13b759814e4f32a644463e887d7 Mon Sep 17 00:00:00 2001 From: "Basil L. Contovounesios" Date: Fri, 20 Apr 2018 15:45:06 +0100 Subject: [PATCH 1/2] Fix failing test for bug#30280 * test/lisp/simple-tests.el (simple-tests-async-shell-command-30280): Fix failing test. --- test/lisp/simple-tests.el | 59 ++++++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/test/lisp/simple-tests.el b/test/lisp/simple-tests.el index 7a10df2058..678d9b9385 100644 --- a/test/lisp/simple-tests.el +++ b/test/lisp/simple-tests.el @@ -521,30 +521,51 @@ simple-test-undo-with-switched-buffer (do-auto-fill) (should (string-equal (buffer-string) "foo bar")))) + +;;; Shell command. + (ert-deftest simple-tests-async-shell-command-30280 () "Test for https://debbugs.gnu.org/30280 ." - :expected-result :failed (let* ((async-shell-command-buffer 'new-buffer) (async-shell-command-display-buffer nil) - (str "*Async Shell Command*") - (buffers-name - (cl-loop repeat 2 - collect (buffer-name - (generate-new-buffer str)))) + (base "name") + (first (buffer-name (generate-new-buffer base))) + (second (generate-new-buffer-name base)) + ;; `save-window-excursion' doesn't restore frame configurations. + (pop-up-frames nil) (inhibit-message t)) - (mapc #'kill-buffer buffers-name) - (async-shell-command - (format "%s -Q -batch -eval '(progn (sleep-for 3600) (message \"foo\"))'" - invocation-name)) - (async-shell-command - (format "%s -Q -batch -eval '(progn (sleep-for 1) (message \"bar\"))'" - invocation-name)) - (let ((buffers (mapcar #'get-buffer buffers-name)) - (processes (mapcar #'get-buffer-process buffers-name))) - (unwind-protect - (should (memq (cadr buffers) (mapcar #'window-buffer (window-list)))) - (mapc #'delete-process processes) - (mapc #'kill-buffer buffers))))) + ;; Let `shell-command' create the buffer as needed. + (kill-buffer first) + (unwind-protect + (save-window-excursion + ;; One command has no output, the other does. + ;; Removing the -eval argument also yields no output, but + ;; then both commands exit simultaneously when + ;; `accept-process-output' is called on the second command. + (dolist (form '("(sleep-for 8)" "(message \"\")")) + (async-shell-command (format "%s -Q -batch -eval '%s'" + invocation-name form) + first)) + ;; First command should neither have nor display output. + (let* ((buffer (get-buffer first)) + (process (get-buffer-process buffer))) + (should (buffer-live-p buffer)) + (should process) + (should (zerop (buffer-size buffer))) + (should (not (get-buffer-window buffer)))) + ;; Second command should both have and display output. + (let* ((buffer (get-buffer second)) + (process (get-buffer-process buffer))) + (should (buffer-live-p buffer)) + (should process) + (should (accept-process-output process 4 nil t)) + (should (> (buffer-size buffer) 0)) + (should (get-buffer-window buffer)))) + (dolist (name (list first second)) + (let* ((buffer (get-buffer name)) + (process (and buffer (get-buffer-process buffer)))) + (when process (delete-process process)) + (when buffer (kill-buffer buffer))))))) (provide 'simple-test) ;;; simple-test.el ends here -- 2.17.0 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0002-Minor-shell-command-simplifications.patch >From 8743148a16480f12923dbaecdefbc641b64d7f0a Mon Sep 17 00:00:00 2001 From: "Basil L. Contovounesios" Date: Sun, 6 May 2018 16:41:01 +0100 Subject: [PATCH 2/2] Minor shell-command simplifications * lisp/simple.el (shell-command): Use call-process-shell-command, start-process-shell-command, and file-attribute-size. Keep track of output-buffer only by its object, not by its name. (bug#30280) --- lisp/simple.el | 72 +++++++++++++++++++++++--------------------------- 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/lisp/simple.el b/lisp/simple.el index a0a6898e17..7958a3b134 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -3400,6 +3400,8 @@ async-shell-command (setq command (concat command " &"))) (shell-command command output-buffer error-buffer)) +(declare-function comint-output-filter "comint" (process string)) + (defun shell-command (command &optional output-buffer error-buffer) "Execute string COMMAND in inferior shell; display output, if any. With prefix argument, insert the COMMAND's output at point. @@ -3477,12 +3479,11 @@ shell-command (not (or (bufferp output-buffer) (stringp output-buffer)))) ;; Output goes in current buffer. (let ((error-file - (if error-buffer - (make-temp-file - (expand-file-name "scor" - (or small-temporary-file-directory - temporary-file-directory))) - nil))) + (and error-buffer + (make-temp-file + (expand-file-name "scor" + (or small-temporary-file-directory + temporary-file-directory)))))) (barf-if-buffer-read-only) (push-mark nil t) ;; We do not use -f for csh; we will not support broken use of @@ -3490,24 +3491,22 @@ shell-command ;; "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 - (if error-file - (list t error-file) - t) - nil shell-command-switch command) + (call-process-shell-command command nil (if error-file + (list t error-file) + t)) (when (and error-file (file-exists-p error-file)) - (if (< 0 (nth 7 (file-attributes error-file))) - (with-current-buffer (get-buffer-create error-buffer) - (let ((pos-from-end (- (point-max) (point)))) - (or (bobp) - (insert "\f\n")) - ;; Do no formatting while reading error file, - ;; because that can run a shell command, and we - ;; don't want that to cause an infinite recursion. - (format-insert-file error-file nil) - ;; Put point after the inserted errors. - (goto-char (- (point-max) pos-from-end))) - (display-buffer (current-buffer)))) + (when (< 0 (file-attribute-size (file-attributes error-file))) + (with-current-buffer (get-buffer-create error-buffer) + (let ((pos-from-end (- (point-max) (point)))) + (or (bobp) + (insert "\f\n")) + ;; Do no formatting while reading error file, + ;; because that can run a shell command, and we + ;; don't want that to cause an infinite recursion. + (format-insert-file error-file nil) + ;; Put point after the inserted errors. + (goto-char (- (point-max) pos-from-end))) + (display-buffer (current-buffer)))) (delete-file error-file)) ;; This is like exchange-point-and-mark, but doesn't ;; activate the mark. It is cleaner to avoid activation, @@ -3525,13 +3524,11 @@ shell-command ;; Command ending with ampersand means asynchronous. (let* ((buffer (get-buffer-create (or output-buffer "*Async Shell Command*"))) - (bname (buffer-name buffer)) - (directory default-directory) - proc) + (proc (get-buffer-process buffer)) + (directory default-directory)) ;; Remove the ampersand. (setq command (substring command 0 (match-beginning 0))) ;; Ask the user what to do with already running process. - (setq proc (get-buffer-process buffer)) (when proc (cond ((eq async-shell-command-buffer 'confirm-kill-process) @@ -3542,35 +3539,32 @@ shell-command ((eq async-shell-command-buffer 'confirm-new-buffer) ;; If will create a new buffer, query first. (if (yes-or-no-p "A command is running in the default buffer. Use a new buffer? ") - (setq buffer (generate-new-buffer bname)) + (setq buffer (generate-new-buffer (buffer-name buffer))) (error "Shell command in progress"))) ((eq async-shell-command-buffer 'new-buffer) ;; It will create a new buffer. - (setq buffer (generate-new-buffer bname))) + (setq buffer (generate-new-buffer (buffer-name buffer)))) ((eq async-shell-command-buffer 'confirm-rename-buffer) ;; If will rename the buffer, query first. (if (yes-or-no-p "A command is running in the default buffer. Rename it? ") - (progn - (with-current-buffer buffer - (rename-uniquely)) - (setq buffer (get-buffer-create bname))) + (with-current-buffer buffer + (rename-uniquely)) (error "Shell command in progress"))) ((eq async-shell-command-buffer 'rename-buffer) ;; It will rename the buffer. (with-current-buffer buffer - (rename-uniquely)) - (setq buffer (get-buffer-create bname))))) + (rename-uniquely))))) (with-current-buffer buffer (shell-command--save-pos-or-erase) (setq default-directory directory) - (setq proc (start-process "Shell" buffer shell-file-name - shell-command-switch command)) + (setq proc + (start-process-shell-command "Shell" buffer command)) (setq mode-line-process '(":%s")) (require 'shell) (shell-mode) - (set-process-sentinel proc 'shell-command-sentinel) + (set-process-sentinel proc #'shell-command-sentinel) ;; Use the comint filter for proper handling of ;; carriage motion (see comint-inhibit-carriage-motion). - (set-process-filter proc 'comint-output-filter) + (set-process-filter proc #'comint-output-filter) (if async-shell-command-display-buffer ;; Display buffer immediately. (display-buffer buffer '(nil (allow-no-window . t))) -- 2.17.0 --=-=-= Content-Type: text/plain I recently noticed that a test for an expected failure was added around the time of this bug report[1: ea8c0e1b9e]. [1: ea8c0e1b9e]: 2018-01-29 22:31:50 +0900 * test/lisp/simple-tests.el (simple-tests-async-shell-command-30280): Add test https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=ea8c0e1b9eaa6651919fb4e039e3fcb5a1fa73db I attach two patches. The first tries to make this test succeed in accordance with the resulting bugfix. The second suggests some simplifications to the logic in shell-command. WDYT? Thanks, -- Basil --=-=-=--