Subject: Re: bug#24837: bug#21609: bug#24837: 26.0.50; term.el: In char mode, displayed and executed commands can differ

From: Phil Sainty <[email-masked]>
Date: Thu, 12 Oct 2017 23:54:42 +1300

On 11/10/17 01:35, Eli Zaretskii wrote:
> In that case, the text should say something like
>   Customize these options to nil if you want the previous behavior.
> 
>> In any case I'll see if I can improve the wording...

I've revised the text for the NEWS entry and both user options.

>From c0c1cb126dfc68a44be2b8d4b4a9350823d3d47c Mon Sep 17 00:00:00 2001
From: Phil Sainty <[email-masked]>
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-char-mode-buffer-read-only: New user option.
term-line-mode-buffer-read-only: New buffer-local variable.
(term-line-mode-buffer-read-only-update): New function.

(term-char-mode, term-line-mode): Use `term-set-goto-process-mark'
in pre-command-hook, and `term-goto-process-mark-maybe' in
post-command-hook to counter-act unexpected changes to point when
using `term-char-mode'.

term-char-mode-point-at-process-mark: New user option.
term-goto-process-mark: New buffer-local variable.
(term-set-goto-process-mark): New function.
(term-goto-process-mark-maybe): New function.
(term-process-mark): New function.

* etc/NEWS: Mention the new behaviour and user options.
---
 etc/NEWS     | 17 +++++++++++
 lisp/term.el | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index 87a82de..6b26a69 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1164,6 +1164,23 @@ provided.
 The old Flymake behaviour is preserved in the so-called "legacy
 backend", which has been updated to benefit from the new UI features.
 
+** Term
+
+---
+*** `term-char-mode' now avoids buffer states which are inconsistent
+with the state of the terminal understood by the inferior process.
+
+The buffer is made read-only to prevent changes from being made by
+anything other than the process filter; and movements of point away
+from the process mark are counter-acted so that the cursor is in the
+correct position after each command.
+
+New user options `term-char-mode-buffer-read-only' and
+`term-char-mode-point-at-process-mark' control these behaviours, and
+are enabled by default.  Customize these options to nil if you want
+the previous behaviours.
+
+
 
 * New Modes and Packages in Emacs 26.1
 
diff --git a/lisp/term.el b/lisp/term.el
index c748c45..2046578 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."
@@ -487,6 +489,41 @@ term-input-ring-file-name
   :type 'boolean
   :group 'term)
 
+(defcustom term-char-mode-buffer-read-only t
+  "If non-nil, only the process filter may modify the buffer in char mode.
+
+A non-nil value makes the buffer read-only in `term-char-mode',
+which prevents editing commands from making the buffer state
+inconsistent with the state of the terminal understood by the
+inferior process.  Only the process filter is allowed to make
+changes to the buffer.
+
+Customize this option to nil if you want the previous behaviour."
+  :version "26.1"
+  :type 'boolean
+  :group 'term)
+
+(defcustom term-char-mode-point-at-process-mark t
+  "If non-nil, keep point at the process mark in char mode.
+
+A non-nil value causes point to be moved to the current process
+mark after each command in `term-char-mode' (provided that the
+pre-command point position was also at the process mark).  This
+prevents commands that move point from making the buffer state
+inconsistent with the state of the terminal understood by the
+inferior process.
+
+Mouse events are not affected, so moving point and selecting text
+is still possible in char mode via the mouse, after which other
+commands can be invoked on the mouse-selected point or region,
+until the process filter (or user) moves point to the process
+mark once again.
+
+Customize this option to nil if you want the previous behaviour."
+  :version "26.1"
+  :type 'boolean
+  :group 'term)
+
 (defcustom term-scroll-to-bottom-on-output nil
   "Controls whether interpreter output causes window to scroll.
 If nil, then do not scroll.  If t or `all', scroll all windows showing buffer.
@@ -1105,6 +1142,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 +1285,13 @@ 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.
+    (when term-char-mode-buffer-read-only
+      (setq buffer-read-only t))
+    (add-hook 'pre-command-hook #'term-set-goto-process-mark nil t)
+    (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 +1311,20 @@ term-line-mode
 you type \\[term-send-input] which sends the current line to the inferior."
   (interactive)
   (when (term-in-char-mode)
+    (when term-char-mode-buffer-read-only
+      (setq buffer-read-only term-line-mode-buffer-read-only))
+    (remove-hook 'pre-command-hook #'term-set-goto-process-mark t)
+    (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'.
+
+Called as a buffer-local `read-only-mode-hook' function."
+  (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 +2768,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 +3167,46 @@ term-emulate-terminal
     (when (get-buffer-window (current-buffer))
       (redisplay))))
 
+(defvar-local term-goto-process-mark t
+  "Whether to reset point to the current process mark after this command.
+
+Set in `pre-command-hook' in char mode by `term-set-goto-process-mark'.")
+
+(defun term-set-goto-process-mark ()
+  "Sets `term-goto-process-mark'.
+
+Always set to nil if `term-char-mode-point-at-process-mark' is nil.
+
+Called as a buffer-local `pre-command-hook' function in
+`term-char-mode' so that when point is equal to the process mark
+at the pre-command stage, we know to restore point to the process
+mark at the post-command stage.
+
+See also `term-goto-process-mark-maybe'."
+  (setq term-goto-process-mark
+        (and term-char-mode-point-at-process-mark
+             (eq (point) (marker-position (term-process-mark))))))
+
+(defun term-goto-process-mark-maybe ()
+  "Move point to the term buffer's process mark upon keyboard input.
+
+Called 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.
+
+Only acts when the pre-command position of point was equal to the
+process mark, and the `term-char-mode-point-at-process-mark'
+option is enabled.  See `term-set-goto-process-mark'."
+  (when term-goto-process-mark
+    (unless (mouse-event-p last-command-event)
+      (goto-char (term-process-mark)))))
+
+(defun term-process-mark ()
+  "The current `process-mark' for the term buffer process."
+  (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