From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Phil Sainty Newsgroups: gmane.emacs.bugs Subject: bug#21609: bug#24837: 26.0.50; term.el: In char mode, displayed and executed commands can differ Date: Mon, 25 Sep 2017 13:48:57 +1300 Message-ID: <1f3ce5dd-e8b0-6aee-1614-bbe0200baa95@orcon.net.nz> References: <08c3b161-174d-1fb7-5df4-bbf7f7d47ee9@orcon.net.nz> <83val1xk58.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------90934D436BADF0CC42BF6F8B" X-Trace: blaine.gmane.org 1506300617 16995 195.159.176.226 (25 Sep 2017 00:50:17 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 25 Sep 2017 00:50:17 +0000 (UTC) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 Cc: 24837@debbugs.gnu.org, 21609@debbugs.gnu.org To: Philipp Stephani , Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Sep 25 02:50:12 2017 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 1dwHb5-0003n7-Ui for geb-bug-gnu-emacs@m.gmane.org; Mon, 25 Sep 2017 02:50:08 +0200 Original-Received: from localhost ([::1]:40143 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dwHbD-0003hx-0F for geb-bug-gnu-emacs@m.gmane.org; Sun, 24 Sep 2017 20:50:15 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:49747) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dwHb5-0003gS-6s for bug-gnu-emacs@gnu.org; Sun, 24 Sep 2017 20:50:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dwHb0-0001fS-AP for bug-gnu-emacs@gnu.org; Sun, 24 Sep 2017 20:50:07 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:48191) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dwHb0-0001fO-5c for bug-gnu-emacs@gnu.org; Sun, 24 Sep 2017 20:50:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1dwHaz-0002Zm-Qx for bug-gnu-emacs@gnu.org; Sun, 24 Sep 2017 20:50:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Phil Sainty Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 25 Sep 2017 00:50:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 21609 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 21609-submit@debbugs.gnu.org id=B21609.15063005519268 (code B ref 21609); Mon, 25 Sep 2017 00:50:01 +0000 Original-Received: (at 21609) by debbugs.gnu.org; 25 Sep 2017 00:49:11 +0000 Original-Received: from localhost ([127.0.0.1]:56867 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dwHaA-0002PO-Ot for submit@debbugs.gnu.org; Sun, 24 Sep 2017 20:49:11 -0400 Original-Received: from smtp-3.orcon.net.nz ([60.234.4.44]:36020) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dwHa8-0002P4-Lx; Sun, 24 Sep 2017 20:49:09 -0400 Original-Received: from [150.107.172.67] (port=32313 helo=[192.168.20.102]) by smtp-3.orcon.net.nz with esmtpa (Exim 4.86_2) (envelope-from ) id 1dwHZx-0007JP-PM; Mon, 25 Sep 2017 13:49:03 +1300 In-Reply-To: Content-Language: en-GB X-GeoIP: NZ X-Spam_score: -2.9 X-Spam_score_int: -28 X-Spam_bar: -- 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:137383 Archived-At: This is a multi-part message in MIME format. --------------90934D436BADF0CC42BF6F8B Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 24/09/17 23:59, Philipp Stephani wrote: > I haven't checked the code in detail, but it seems to be reasonable. > Since nobody complained in weeks, maybe just push it to the release > branch and see whether it breaks anything. Before we do that, here's a revised patch based on my own follow-ups to my initial patch. Changes to that version are as follows: I've switched the `buffer-read-only' let-binding to `inhibit-read-only', as you had originally suggested. It looks like programatically calling `read-only-mode' was incorrect on my part, so I'm now setting `buffer-read-only' instead. I've added a `read-only-mode-hook' function to track when the user toggles the read-only state, however, so that if the user happens to enable `read-only-mode' in line mode, toggling to and from char mode will not revert their selected read-only state. Lastly I'm trying to avoid messing with mouse selection in char mode, by *not* moving point back to the process mark if the last command input event was a mouse event. So keyboard events cannot leave point in the wrong position; and while mouse events can do so, as soon as the keyboard is involved again we jump to where we're supposed to be. This still happens in post-command-hook only. Question: Is there a reason to additionally do this in pre-command-hook? I initially thought I'd need to do so, due to it now being possible for a (mouse) command to leave point in the wrong position; however I think it's probably still fine to limit this to post-command-hook? IIRC the main danger of leaving point in the wrong position is mitigated by the buffer being read-only, which ensures that the user cannot make inconsistent changes to the buffer state in char mode; and I'm assuming that inferior process output is guaranteed to happen at the process mark. (The user could invoke a command which uses inhibit-read-only to update the buffer, but that's going to introduce inconsistencies no matter where point is at the time, so I think that's out of scope.) The user might also wish to invoke a keyboard command that acts upon the mouse-selected region, so limiting ourselves to post-command-hook means we won't interfere with such a command. New patch is attached. -Phil --------------90934D436BADF0CC42BF6F8B Content-Type: text/x-patch; name="0001-Avoid-creating-inconsistent-buffer-states-in-term-ch.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0001-Avoid-creating-inconsistent-buffer-states-in-term-ch.pa"; filename*1="tch" >From 2476e0a6aed6ba9760651df707b814046ffe45ca Mon Sep 17 00:00:00 2001 From: Phil Sainty Date: Sun, 3 Sep 2017 14:30:18 +1200 Subject: [PATCH] Avoid creating inconsistent buffer states in term-char-mode (bug#24837) * lisp/term.el (term-mode, term-char-mode, term-line-mode) (term-emulate-terminal): Make buffer read-only in `term-char-mode', except for the process filter's output. Use `read-only-mode-hook' to track and restore the user-set state of `buffer-read-only' for `term-line-mode'. term-line-mode-buffer-read-only: New variable. (term-line-mode-buffer-read-only-update): New function. (term-char-mode, term-line-mode): Use `term-goto-process-mark-maybe' in post-command-hook to counter-act unexpected changes to point. (term-goto-process-mark-maybe): New function. (term-goto-process-mark): New function. --- lisp/term.el | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/lisp/term.el b/lisp/term.el index c748c45..31c78af 100644 --- a/lisp/term.el +++ b/lisp/term.el @@ -427,6 +427,8 @@ term-old-mode-map (defvar term-old-mode-line-format) ; Saves old mode-line-format while paging. (defvar term-pager-old-local-map nil "Saves old keymap while paging.") (defvar term-pager-old-filter) ; Saved process-filter while paging. +(defvar-local term-line-mode-buffer-read-only nil + "The `buffer-read-only' state to set in `term-line-mode'.") (defcustom explicit-shell-file-name nil "If non-nil, is file name to use for explicitly requested inferior shell." @@ -1105,6 +1107,8 @@ term-mode (term-reset-size (cdr size) (car size))) size)) + (add-hook 'read-only-mode-hook #'term-line-mode-buffer-read-only-update nil t) + (easy-menu-add term-terminal-menu) (easy-menu-add term-signals-menu) (or term-input-ring @@ -1246,6 +1250,11 @@ term-char-mode (easy-menu-add term-terminal-menu) (easy-menu-add term-signals-menu) + ;; Don't allow changes to the buffer or to point which are not + ;; caused by the process filter. + (setq buffer-read-only 1) + (add-hook 'post-command-hook #'term-goto-process-mark-maybe nil t) + ;; Send existing partial line to inferior (without newline). (let ((pmark (process-mark (get-buffer-process (current-buffer)))) (save-input-sender term-input-sender)) @@ -1265,9 +1274,16 @@ term-line-mode you type \\[term-send-input] which sends the current line to the inferior." (interactive) (when (term-in-char-mode) + (setq buffer-read-only term-line-mode-buffer-read-only) + (remove-hook 'post-command-hook #'term-goto-process-mark-maybe t) (use-local-map term-old-mode-map) (term-update-mode-line))) +(defun term-line-mode-buffer-read-only-update () + "Update the user-set state of `buffer-read-only' in `term-line-mode'." + (when (term-in-line-mode) + (setq term-line-mode-buffer-read-only buffer-read-only))) + (defun term-update-mode-line () (let ((term-mode (if (term-in-char-mode) @@ -2711,6 +2727,7 @@ term-emulate-terminal count-bytes ; number of bytes decoded-substring save-point save-marker old-point temp win + (inhibit-read-only t) (buffer-undo-list t) (selected (selected-window)) last-win @@ -3109,6 +3126,21 @@ term-emulate-terminal (when (get-buffer-window (current-buffer)) (redisplay)))) +(defun term-goto-process-mark-maybe () + "Move point to the term buffer's process-mark upon keyboard input. + +Used as a buffer-local `post-command-hook' function in +`term-char-mode' to prevent commands from putting the buffer into +an inconsistent state by unexpectedly moving point. + +Mouse events are ignored so that mouse selection is unimpeded." + (unless (mouse-event-p last-command-event) + (term-goto-process-mark))) + +(defun term-goto-process-mark () + "Move point to the current process-mark for the term buffer process." + (goto-char (process-mark (get-buffer-process (current-buffer))))) + (defun term-handle-deferred-scroll () (let ((count (- (term-current-row) term-height))) (when (>= count 0) -- 2.8.3 --------------90934D436BADF0CC42BF6F8B--