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: Tue, 30 Jan 2018 18:53:09 +0000 Message-ID: <877erz41re.fsf@tcd.ie> References: <87a7wxd1g9.fsf@mail.linkov.net> <83607kinmx.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1517338480 7631 195.159.176.226 (30 Jan 2018 18:54:40 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 30 Jan 2018 18:54:40 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: Reuben Thomas , 30280@debbugs.gnu.org, Juri Linkov To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Jan 30 19:54:35 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 1egb2l-00006m-LA for geb-bug-gnu-emacs@m.gmane.org; Tue, 30 Jan 2018 19:54:07 +0100 Original-Received: from localhost ([::1]:55637 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1egb4m-0001U1-H7 for geb-bug-gnu-emacs@m.gmane.org; Tue, 30 Jan 2018 13:56:12 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:48716) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1egb3l-0001Am-FB for bug-gnu-emacs@gnu.org; Tue, 30 Jan 2018 13:56:06 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1egb2h-0006Vo-H0 for bug-gnu-emacs@gnu.org; Tue, 30 Jan 2018 13:55:09 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:41919) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1egb2h-0006UA-3N for bug-gnu-emacs@gnu.org; Tue, 30 Jan 2018 13:54:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1egb2g-0002lI-Kq for bug-gnu-emacs@gnu.org; Tue, 30 Jan 2018 13:54:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: "Basil L. Contovounesios" Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 30 Jan 2018 18:54: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.151733840210567 (code B ref 30280); Tue, 30 Jan 2018 18:54:02 +0000 Original-Received: (at 30280) by debbugs.gnu.org; 30 Jan 2018 18:53:22 +0000 Original-Received: from localhost ([127.0.0.1]:49816 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1egb21-0002kN-QR for submit@debbugs.gnu.org; Tue, 30 Jan 2018 13:53:22 -0500 Original-Received: from mail-wm0-f46.google.com ([74.125.82.46]:36902) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1egb1x-0002k6-Vb for 30280@debbugs.gnu.org; Tue, 30 Jan 2018 13:53:20 -0500 Original-Received: by mail-wm0-f46.google.com with SMTP id v71so3225034wmv.2 for <30280@debbugs.gnu.org>; Tue, 30 Jan 2018 10:53:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tcd-ie.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:in-reply-to:references:user-agent:date :message-id:mime-version; bh=Hv6KvYsbW/jk324Y9N8rjPXjIh72lljd60mc0CIra78=; b=gd3N5XxHLGCmBtgUUmReZo/HebCp3MkGYXksICw7wy3zMrah2DQ+JNI2E22wdseOxh ATu/d4BIlNPV6rMaf3ib7hy82cmVk75GYtvKuU5Jn/6y2aNg91ukZ/FQ5oJOKEO8iP+S +wmJopwmkHGZrNHP3j9Wd9AQ3Tc52IS8oIVrE4tuKx9fk4H+EU+7/STZwpGuyky44t5D hnnoXCiU9NI6mvK8OMkMyiArYyKv8hBOdME6J3beo+U5E0q7IYe+0PRcHaQabQQl9Fo9 bSrRACDty30K1u8xWjh5nhXbvh1rFtKV1GGMQmfXJHnMTOmaC894re1RKccs8cc0czIl Mh1w== 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:in-reply-to:references :user-agent:date:message-id:mime-version; bh=Hv6KvYsbW/jk324Y9N8rjPXjIh72lljd60mc0CIra78=; b=ErV5k/W50+1ENClJB8bbKD8Bdvb3FshjeiAXZtrCQl6dwAlAFymVj7px+26wbqeZmT buAGySwE7xTJrZH+sRLl6Q7EsndYhRvh464QRO5KPNi0yVDsxPrEgXvVrE/Bmx4COwKE y+fhFrQG43YXWlWR3OydozrL3WL37wkQ5HmLlQjGQ2Kn3dQbWlQ41JtZMNZizr/7/mlA 0jQbAta/8Zkj6cNemJf79USZtK4l1DEnRAVT+8kMTbl/ch/CYui2I5A49caf7Ag58+eR 144d8clpHM7ZEDiyHKOd/A95ONktgLqnzsS25nq4meiXeEwI1KEFoX/lqpuca0TSiZRp OAQw== X-Gm-Message-State: AKwxytehPxPmRa464Mth2FPxlvaYz1mZxLVeSU0gf4voGQLHrpmD0AmU nos0a1kJ1mvs6bdXsWz71Op25g== X-Google-Smtp-Source: AH8x226XytdilWFs0aWp6/dnRsvxy6gaqs/S6YDX+mo12oXEGKmlA4/aNYxxny5xb7PNYE3sNhUijA== X-Received: by 10.80.230.12 with SMTP id y12mr53382516edm.203.1517338392142; Tue, 30 Jan 2018 10:53:12 -0800 (PST) Original-Received: from localhost ([2a02:8084:4f41:8c80:9c34:da08:a010:edfc]) by smtp.gmail.com with ESMTPSA id w26sm8646899edw.7.2018.01.30.10.53.09 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 30 Jan 2018 10:53:10 -0800 (PST) In-Reply-To: <83607kinmx.fsf@gnu.org> (Eli Zaretskii's message of "Mon, 29 Jan 2018 19:24:38 +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:142709 Archived-At: --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Fix-deferred-display-of-async-shell-command-buffers.patch >From e55856682b1ff85e8743da04965e12a82e763689 Mon Sep 17 00:00:00 2001 From: "Basil L. Contovounesios" Date: Tue, 30 Jan 2018 16:18:14 +0000 Subject: [PATCH] Fix deferred display of async shell-command buffers * lisp/simple.el (shell-command): Display async shell buffer on process output for every, not just first, command invocation. Check buffer liveness, not name, before displaying. (bug#30213, bug#30280) --- lisp/simple.el | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/lisp/simple.el b/lisp/simple.el index 3ac6b86381..e2deceb4c2 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -3547,14 +3547,20 @@ shell-command ;; carriage motion (see comint-inhibit-carriage-motion). (set-process-filter proc 'comint-output-filter) (if async-shell-command-display-buffer + ;; Display buffer immediately. (display-buffer buffer '(nil (allow-no-window . t))) - (add-function :before (process-filter proc) - (lambda (process _string) - (let ((buf (process-buffer process))) - (when (and (zerop (buffer-size buf)) - (string= (buffer-name buf) - bname)) - (display-buffer buf)))))))) + ;; Defer displaying buffer until first process output. + ;; Use disposable named advice so that the buffer is + ;; displayed at most once per process lifetime. + (let ((nonce (make-symbol "nonce"))) + (add-function :before (process-filter proc) + (lambda (proc _string) + (let ((buf (process-buffer proc))) + (when (buffer-live-p buf) + (remove-function (process-filter proc) + nonce) + (display-buffer buf)))) + `((name . ,nonce))))))) ;; Otherwise, command is executed synchronously. (shell-command-on-region (point) (point) command output-buffer nil error-buffer))))))) -- 2.15.1 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Eli Zaretskii writes: >> From: Juri Linkov >> Date: Mon, 29 Jan 2018 00:20:30 +0200 >>=20 >> 0. emacs -Q >> 1. Eval: (setq async-shell-command-buffer 'new-buffer >> async-shell-command-display-buffer nil) >> 2. M-& sleep 3600 >> 3. M-& sleep 3; echo "xyzzy" >>=20 >> At the step 2 =E2=80=9C*Async Shell Command*=E2=80=9D is not shown becau= se of no output, >> this is ok. >>=20 >> But at the step 3 =E2=80=9C*Async Shell Command*<2>=E2=80=9D is not show= n after 3 seconds >> when the output arrives, this is a regression. >>=20 >> It seems it is caused by commit#9f4f130 from bug#28997 Commit 9f4f130 indeed touches the relevant code, but the bug was introduced along with the async-shell-command-display-buffer feature. >> PS: Also it doesn't work with less official configuration: >> 0. emacs -Q >> 1. Eval: (setq async-shell-command-display-buffer nil) >> (add-hook 'shell-mode-hook 'rename-uniquely) >> 2. M-& sleep 3600 >> 3. M-& sleep 3; echo "xyzzy" > > Thanks for reporting this. Basil, can you take a look, please? I am taking the liberty of CCing Reuben because I believe this report can be merged with bug#30213. As justification, a recap and correction of my diagnosis and solution from that report follows. The async-shell-command-display-buffer toggle determines whether the output buffer is displayed immediately or in the process filter, as the latter should only be executed on process output. The guard in the process filter, however, has always required that the buffer (a) be empty; and (b) have the same name as the original output buffer. I assume the reasoning behind (a) is that we only want the output buffer to be displayed when the process first outputs something, and not every time the process filter is called. As mentioned in bug#30213, though, the empty buffer check is The Wrong Thing when shell-command-dont-erase-buffer is non-nil, i.e. when we want to reuse a non-empty output buffer left behind by an old shell-command invocation. AIUI, the process filter advice needs a way to distinguish between the first time it receives process output and all its subsequent triggers, irrespective of buffer contents. I'm not entirely sure what the case for (b) is. Usually process filters need only check that their process buffer is live before acting on it; surely that is also The Right Thing in this case? The main issue with (b) is that it does not take into account the various flavours of async-shell-command-buffer under which the output buffer is renamed, as demonstrated in this bug report. I think the simplest fix for (b) would be something like --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=foo.diff diff --git a/lisp/simple.el b/lisp/simple.el index 3ac6b86381..4f6708dbc1 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -3552,8 +3552,7 @@ shell-command (lambda (process _string) (let ((buf (process-buffer process))) (when (and (zerop (buffer-size buf)) - (string= (buffer-name buf) - bname)) + (eq buf buffer)) (display-buffer buf)))))))) ;; Otherwise, command is executed synchronously. (shell-command-on-region (point) (point) command --=-=-= Content-Type: text/plain but I still don't see why we are forgoing a liveness check in favour of this. I attach a patch which addresses both bugs. Its solution for (a) is to make the advice disposable, i.e. it removes itself from the process filter after it has fulfilled its purpose of displaying the output buffer. A syntactically simpler implementation of this could use a plain boolean switch instead of removing the advice, but IMO the latter is semantically more sound (and possibly more performant in subsequent invocations of the process filter, though this should be irrelevant). WDYT? P.S. Would patch(es) for aesthetic changes to the rest of shell-command (such as removing redundant variables, inverting the condition of the massive if-then-else to reduce indentation, etc.) be welcome? If so, where should I send them? -- Basil --=-=-=--