From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Tino Calancha Newsgroups: gmane.emacs.devel Subject: Re: [Emacs-diffs] master c711991: Allow not erase output buffer in shell commands Date: Wed, 21 Sep 2016 00:10:58 +0900 (JST) Message-ID: NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; format=flowed; charset=US-ASCII X-Trace: blaine.gmane.org 1474385145 22705 195.159.176.226 (20 Sep 2016 15:25:45 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 20 Sep 2016 15:25:45 +0000 (UTC) User-Agent: Alpine 2.20 (DEB 67 2015-01-07) Cc: Emacs developers , tino.calancha@gmail.com To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Sep 20 17:25:36 2016 Return-path: Envelope-to: ged-emacs-devel@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 1bmMvF-0003mB-2t for ged-emacs-devel@m.gmane.org; Tue, 20 Sep 2016 17:25:25 +0200 Original-Received: from localhost ([::1]:36041 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmMvF-0003kn-Aa for ged-emacs-devel@m.gmane.org; Tue, 20 Sep 2016 11:25:25 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:37311) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmMhP-00019s-Hy for emacs-devel@gnu.org; Tue, 20 Sep 2016 11:11:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bmMhL-00079k-9H for emacs-devel@gnu.org; Tue, 20 Sep 2016 11:11:06 -0400 Original-Received: from mail-pf0-x241.google.com ([2607:f8b0:400e:c00::241]:36271) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmMhK-00078p-TK for emacs-devel@gnu.org; Tue, 20 Sep 2016 11:11:03 -0400 Original-Received: by mail-pf0-x241.google.com with SMTP id n24so1091388pfb.3 for ; Tue, 20 Sep 2016 08:11:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:date:to:cc:subject:message-id:user-agent:mime-version; bh=aBp4/bcxvzV5Zde7DsoVm+EHfoOxevTtobopXFAr/bw=; b=ZedRTvjODEeLNyzQh8SU3QQ/bK/JvvD75cw9pXJZDzgfxRpVranhAlqL4kyDq4jwz0 GOIakNsgYhRjbNZl6So82k2+DpoSI3FtfJNGxCcGWqTlkUIE3XPcV3OSivWh1SzBzCfR rnIJaP/tLZF2c/ikniitt4cRVftMpp8GXNLo4MCpc7ZWe4WAG7fpfn/mRqcG26jlN/Fm eNQT/dhxL957PKetay8GhXTgOtBfXPFPpm0ZH9ZJ/XDOxb47QtXeyAtnB1vFtJshnOiv eiD+R28TSmks1e08kK1J+NIvJVJo5fmzusnB+mU0taFTY721bNNssHgqAFp0JQjkOffx CsFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:date:to:cc:subject:message-id:user-agent :mime-version; bh=aBp4/bcxvzV5Zde7DsoVm+EHfoOxevTtobopXFAr/bw=; b=VStzGcg5AQxi/7QP1mYhD0Zs9Fyomt+BZwJih3UX3VJsioimCMdwmN/7yhN/4npWHI tk3/qs8F4TZm5qbbm5DaPhi18RpfNyWbmGotyS38CbjjVN5zVe5ny76lT/mO3B5Wi1Zd yWXFUQzeg17CevZ8dzmPwD9IkK4sB7dIZarED4wYL42EJJRgWuCQy7pwVV7HegWZePxW CgfkI3qpTMeA9b/5PLhPmDbEFed22C/QrICOZH7YT1CcaKbAOfXE1X46DzsxZmIN3n5+ BNdEhpBD1Tajb9G1tD+e0sUvzy4Cun3syIw2V4TpmhEvFF0aTHYEitqjN1++8u2jHWzV FsMA== X-Gm-Message-State: AE9vXwPL4LksYp9EFcDYDmVYs0177wJofDd/Mos/Xz+tcCJr9HNfcvFyttKlnM/J+uZTFQ== X-Received: by 10.98.20.137 with SMTP id 131mr37005581pfu.11.1474384261823; Tue, 20 Sep 2016 08:11:01 -0700 (PDT) Original-Received: from calancha-pc (57.92.100.220.dy.bbexcite.jp. [220.100.92.57]) by smtp.gmail.com with ESMTPSA id r66sm23273851pfb.54.2016.09.20.08.10.59 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 20 Sep 2016 08:11:01 -0700 (PDT) X-Google-Original-From: Tino Calancha X-X-Sender: calancha@calancha-pc X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2607:f8b0:400e:c00::241 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:207629 Archived-At: Hi Stefan, thank you very much for your comments. I have addressed all of them. See the patch below. On Wed, 22 Aug 2016, Stefan Monnier wrote: >misc.texi *is* the documentation, so it makes no sense for it to say "see the >documentation of the variable for details". >etc/NEWS on the other hand is *not* where we put documentation. So the >above shouldn't be a paragraph but a just a line announcing the new var. Fixed. Thanks. >> +(defcustom shell-command-not-erase-buffer nil >Usually we use "dont" or "inhibit" (please check to see which is more >standard/common within the core Elisp code, or better yet: which is >better) rather than just "not". "inhibit" appears factor ~2 times than "dont". I have chosen "dont" which is shorter and quit clear. >> + "If non-nil, output buffer is not erased between shell commands. >You might as well say "before" rather than "between", so it's more precise. Fixed. Thanks. >> +Also, a non-nil value set the point in the output buffer >> +once the command complete. >Why use a single var for those two meanings? >The `end-last-out' choice makes a lot of sense also when combined with >"do erase", yet your system prevents me from using both at the same time. Added a new option 'shell-command-set-buffer-point' which controls the point position. >> +The value `beg-last-out' set point at the beginning of the output, >Try to be more explicit about which output we're talking about. Eg. here I'd probably say "beginning of the new output". >> +`end-last-out' set point at the end of the buffer, >And here "end of the new output". Fixd. Thanks. >> `save-point' +restore the buffer position before the command." >Makes it sound like we restore the buffer position and then we run the >command, rather than after the command we restore the position it >had earlier. I reduced the sentence to just say: and `save-point' restore the point. >From the context should be clear that the point is set to the value before the command started. >> +(defvar shell-command-saved-pos nil >This is an internal variable. So its name should start with >"shell-command--" (notice the double dash). Fixed. Thanks. >> +It is an alist (BUFFER . POS), where BUFFER is the output >> +buffer, and POS is the point position in BUFFER once the >> command finish. >Why use an alist rather than set the variable buffer-locally? Not coffee enough when i wrote that part. Changed to a permanent buffer local variable. Thanks. >> +This variable is used when `shell-command-not-erase-buffer' is non-nil.") >Variable should generally say what they are expected to hold, rather >than who uses them. Dropped the excess of literature. Thank you. >> +(defun shell-command--save-pos-or-erase () ^^ > great! I have my moments... sometimes; don't get used. >> + (let ((sym shell-command-not-erase-buffer) >`sym' is an odd name since we don't care about the fact that it's >a symbol (we're not going to call things like symbol-name, ...). Not uing `sym' anymore. Thanks. >> + pos) >> + (setq buffer-read-only nil) >you can move this (setq buffer-read-only nil) before the previous `let', >so that you can then fold the `setq' of `pos' into the `let'. Good idea, fixed. Thank you. >> + ;; Setting buffer-read-only to nil doesn't suffice >> + ;; if some text has a non-nil read-only property, >> + ;; which comint sometimes adds for prompts. >> + (setq pos >> + (cond ((eq sym 'save-point) (point)) >> + ((eq sym 'beg-last-out) (point-max)) >> + ((not sym) >> + (let ((inhibit-read-only t)) >> + (erase-buffer) nil)))) >Here you can use > (pcase shell-command-not-erase-buffer > ('save-point ...) > ('beg-last-out ...) > ... ) >instead. Implemented. Thank you. >> + (when (buffer-live-p buf) >> + (let ((win (car (get-buffer-window-list buf))) >Why use get-buffer-window-list rather than get-buffer-window? >Why use a nil value for `all-frames'? I was doing something wrong. Fixed. Thank you. >> + (pmax (with-current-buffer buf (point-max)))) >> + (unless (and pos (memq sym '(save-point beg-last-out))) >> + (setq pos pmax)) >Why not compute pmax here on the fly, rather than pre-compute it before >we know if we'll need it? Right. Fixed. Thanks. >> + ;; Set point in the window displaying buf, if any; otherwise >> + ;; display buf temporary in selected frame and set the point. >> + (if win >> + (set-window-point win pos) >> + (save-window-excursion >> + (let ((win (display-buffer >> + buf >> + '(nil (inhibit-switch-frame . t))))) >> + (set-window-point win pos))))))))) >You *cannot* undo a `display-buffer' after the fact (the display-buffer >call can have very visible side-effects in itself. For example, in my >setup, it creates a new frame and waits for me to manually place this >frame on the screen. No amount of save-window-excursion will make me >forget that I had to place the frame manually, and worse yet: it might >even fail to get rid of that new frame). > >If the buffer is not displayed, then there's simply no set-window-point >to perform: use `goto-char' instead. Ok, fixed as you suggest. I guess, in cases where the buffer is not displayed, (goto-char pos) might not always work as `set-window-point': a posterior display of the buffer maybe doesn't show point at pos. ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; >From f87db51ef46d4ad03cdc612d4fded5ecef39df41 Mon Sep 17 00:00:00 2001 From: Tino Calancha Date: Tue, 20 Sep 2016 23:18:07 +0900 Subject: [PATCH] Split shell-command-dont-erase-buffer in 2 options One controls if the output buffer should be erased; the other controls the position of the point after the command completes. Suggested by Stefan Monnier in: https://lists.gnu.org/archive/html/emacs-devel/2016-08/msg00463.html * lisp/simple.el (shell-command-set-buffer-point): New option; control position of point in the output buffer after command completes. (shell-command-dont-erase-buffer): Fix doc string. (shell-command--saved-pos): Rename from shell-command-saved-pos. Make permanent buffer local. (shell-command--save-pos-or-erase): Use shell-command-set-buffer-point. (shell-command--set-point-after-cmd): Idem. Do not force a display of the output buffer. (shell-command-on-region): Use shell-command-set-buffer-point. * doc/emacs/misc.texi (shell-command-dont-erase-buffer) (shell-command-set-buffer-point): Update documentation. ;* etc/NEWS (Changes in Emacs 25.2): ;Add entry for shell-command-set-buffer-point. ;Update entry for shell-command-dont-erase-buffer. --- doc/emacs/misc.texi | 18 +++++++--- etc/NEWS | 12 +++---- lisp/simple.el | 98 +++++++++++++++++++++++++++-------------------------- 3 files changed, 69 insertions(+), 59 deletions(-) diff --git a/doc/emacs/misc.texi b/doc/emacs/misc.texi index 502ccad..54e2a3e 100644 --- a/doc/emacs/misc.texi +++ b/doc/emacs/misc.texi @@ -772,12 +772,22 @@ Single Shell inserted into a buffer of that name. @vindex shell-command-dont-erase-buffer - By default, the output buffer is erased between shell commands. + By default, the output buffer is erased before shell commands. If you change the value of the variable @code{shell-command-dont-erase-buffer} to a non-@code{nil} value, -the output buffer is not erased. This variable also controls where to -set the point in the output buffer after the command completes; see the -documentation of the variable for details. +the output buffer is not erased. + +@vindex shell-command-set-buffer-point + The variable @code{shell-command-set-buffer-point} controls where +to set the point in the output buffer after the command completes: +@code{beg-last-out} set point at the beginning of the new output, +@code{end-last-out} set point at the end of the new output, and +@code{save-point} restores the point to its position before the +command started. + + When @code{shell-command-not-erase-buffer} is @code{nil}, the default +value, the position of the point is unspecified, i.e., an implementation +detail. @node Interactive Shell @subsection Interactive Subshell diff --git a/etc/NEWS b/etc/NEWS index 9b992d0..6ba7581 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -68,13 +68,11 @@ inferior shell with the buffer region as input. +++ ** The new user option 'shell-command-dont-erase-buffer' controls -if the output buffer is erased between shell commands; if non-nil, -the output buffer is not erased; this variable also controls where -to set the point in the output buffer: beginning of the output, -end of the buffer or save the point. -When 'shell-command-dont-erase-buffer' is nil, the default value, -the behaviour of 'shell-command', 'shell-command-on-region' and -'async-shell-command' is as usual. +if the output buffer is erased before shell commands. + ++++ +** The new user option 'shell-command-set-buffer-point' controls +where to set point in the output buffer of a shell command. +++ ** The new user option 'mouse-select-region-move-to-beginning' diff --git a/lisp/simple.el b/lisp/simple.el index 7e68baa..7b5e556 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -38,25 +38,31 @@ compilation-current-error (defvar compilation-context-lines) (defcustom shell-command-dont-erase-buffer nil - "If non-nil, output buffer is not erased between shell commands. -Also, a non-nil value set the point in the output buffer -once the command complete. -The value `beg-last-out' set point at the beginning of the output, -`end-last-out' set point at the end of the buffer, `save-point' -restore the buffer position before the command." + "If non-nil, output buffer is not erased before shell commands." :type '(choice (const :tag "Erase buffer" nil) + (const :tag "Don't erase buffer" t)) + :group 'shell + :version "25.2") + +(defcustom shell-command-set-buffer-point nil + "If non-nil, set point in the output buffer once the command complete. +The value `beg-last-out' set point at the beginning of the new output, +`end-last-out' set point at the end of the new output, and `save-point' +restore the point. +Note that `save-point' only has sense if `shell-command-dont-erase-buffer' +is non-nil." + :type '(choice + (const :tag "None" nil) (const :tag "Set point to beginning of last output" beg-last-out) (const :tag "Set point to end of last output" end-last-out) - (const :tag "Save point" save-point)) + (const :tag "Restore point" save-point)) :group 'shell :version "25.2") -(defvar shell-command-saved-pos nil - "Point position in the output buffer after command complete. -It is an alist (BUFFER . POS), where BUFFER is the output -buffer, and POS is the point position in BUFFER once the command finish. -This variable is used when `shell-command-dont-erase-buffer' is non-nil.") +(defvar-local shell-command--saved-pos nil + "Point position in the output buffer after command complete.") +(put 'shell-command--saved-pos 'permanent-local t) (defcustom idle-update-delay 0.5 "Idle time delay before updating various things on the screen. @@ -3234,49 +3240,44 @@ async-shell-command-buffer (defun shell-command--save-pos-or-erase () "Store a buffer position or erase the buffer. See `shell-command-dont-erase-buffer'." - (let ((sym shell-command-dont-erase-buffer) - pos) - (setq buffer-read-only nil) - ;; Setting buffer-read-only to nil doesn't suffice - ;; if some text has a non-nil read-only property, - ;; which comint sometimes adds for prompts. - (setq pos - (cond ((eq sym 'save-point) (point)) - ((eq sym 'beg-last-out) (point-max)) - ((not sym) - (let ((inhibit-read-only t)) - (erase-buffer) nil)))) + (setq buffer-read-only nil) + ;; Setting buffer-read-only to nil doesn't suffice + ;; if some text has a non-nil read-only property, + ;; which comint sometimes adds for prompts. + (let ((pos (pcase shell-command-set-buffer-point + ('save-point (and shell-command-dont-erase-buffer (point))) + ('beg-last-out (if shell-command-dont-erase-buffer + (point-max) + (point-min))) + ((pred null) nil)))) + (unless shell-command-dont-erase-buffer + (let ((inhibit-read-only t)) + (erase-buffer))) + (goto-char (point-max)) (when pos - (goto-char (point-max)) - (push (cons (current-buffer) pos) - shell-command-saved-pos)))) + (setq shell-command--saved-pos pos)))) (defun shell-command--set-point-after-cmd (&optional buffer) "Set point in BUFFER after command complete. BUFFER is the output buffer of the command; if nil, then defaults to the current BUFFER. -Set point to the `cdr' of the element in `shell-command-saved-pos' -whose `car' is BUFFER." - (when shell-command-dont-erase-buffer - (let* ((sym shell-command-dont-erase-buffer) - (buf (or buffer (current-buffer))) - (pos (alist-get buf shell-command-saved-pos))) - (setq shell-command-saved-pos - (assq-delete-all buf shell-command-saved-pos)) +Set point to `shell-command--saved-pos'" + (when (and shell-command-set-buffer-point + ;; 'save-point is undefined when we erase the buffer. + (not (and (eq shell-command-set-buffer-point 'save-point) + (not shell-command-dont-erase-buffer)))) + (let ((buf (or buffer (current-buffer)))) (when (buffer-live-p buf) - (let ((win (car (get-buffer-window-list buf))) - (pmax (with-current-buffer buf (point-max)))) - (unless (and pos (memq sym '(save-point beg-last-out))) - (setq pos pmax)) - ;; Set point in the window displaying buf, if any; otherwise - ;; display buf temporary in selected frame and set the point. + (set-buffer buf) + (let ((win (get-buffer-window nil t)) + (pos (or shell-command--saved-pos (point-max)))) + ;; Set point in the window displaying buf, if any. + ;; If buf is not displayed, use `goto-char', although this + ;; might not set point permanently. (if win (set-window-point win pos) - (save-window-excursion - (let ((win (display-buffer - buf - '(nil (inhibit-switch-frame . t))))) - (set-window-point win pos))))))))) + (goto-char pos)) + (setq shell-command--saved-pos nil)))))) (defun async-shell-command (command &optional output-buffer error-buffer) "Execute string COMMAND asynchronously in background. @@ -3562,7 +3563,7 @@ display-message-or-buffer ;; We have a sentinel to prevent insertion of a termination message ;; in the buffer itself, and to set the point in the buffer when -;; `shell-command-dont-erase-buffer' is non-nil. +;; `shell-command-set-buffer-point' is non-nil. (defun shell-command-sentinel (process signal) (when (memq (process-status process) '(exit signal)) (shell-command--set-point-after-cmd (process-buffer process)) @@ -3683,7 +3684,8 @@ shell-command-on-region (or output-buffer "*Shell Command Output*")))) (unwind-protect (if (and (eq buffer (current-buffer)) - (or (not shell-command-dont-erase-buffer) + (or (and (not shell-command-dont-erase-buffer) + (not shell-command-set-buffer-point)) (and (not (eq buffer (get-buffer "*Shell Command Output*"))) (not (region-active-p))))) ;; If the input is the same buffer as the output, -- 2.9.3 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; In GNU Emacs 25.1.50.1 Repository revision: 83fbb3a6dd75e01a768cb6b3348b7c947711ee46