Feedback on getting rid of `term-suppress-hard-newline'

[John Shahid]

Hi all,

I should add a little bit of context and explain why I am trying to get
rid of `term-suppress-hard-newline'.  I use ansi-term on a daily basis
and I stay in "char mode" all the time unless I'm copying something from
the terminal output.  ansi-term adds extra newlines at the to break long
lines into multiple lines of text that fit the terminal width.  There is
an option to disable this behavior called `term-suppress-hard-newline'.
This option is useful when:

1. The window dimensions changes to accommodate the long line(s).  It is
nice to see the line unwrap and adjust to the larger window width.

2. The text is copied from the terminal buffer to another buffer.  It is
nice not to have extra newlines that weren't part of the original
output, specially when copying large amounts of text from the terminal
output.

But, `term-suppress-hard-newline' feels like a hack.  It has few edge
cases that I have been running into.  For example, `term-unwrap-line'
can break long lines unexpectedly.  This causes edits to the beginning
of the command line (e.g. inserting or removing a character) to mess up
the terminal screen.  Furthermore `term-down' doesn't adjust the
`term-current-column' and `term-start-line-column' properly.  It just
assumes that the line starts at column 0, which isn't the case when a
long line is wrapped around.

I think those issues I mentioned above are fix-able.  But, I think that
`term-suppress-hard-newline' breaks an assumption that is made in the
rest of the code.  Instead, I experimented with a different approach
that i would like to get some feedback on.

1. Add a text property to mark extra newlines when they are inserted
(e.g. `term-newline' is set to `t')

2. On resize reflow the text.  That is, remove the extra newlines and
add new ones to make sure that lines fit in the terminal width.

3. Set a `filter-buffer-substring-function' to remove those extra
newlines.

I attached a patch that I have been using locally.  Let me know what you
think.

>From 2b6332a66b56fea987fe70d336c1742ae6352ffa Mon Sep 17 00:00:00 2001
From: John Shahid <address@hidden>
Date: Sat, 8 Dec 2018 10:32:36 -0500
Subject: [PATCH] wip: add some reflow and copy logic

TODO: need to be tested
---
 lisp/term.el | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index 9f8f1f703a..024adb7f70 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -1106,6 +1106,7 @@ term-mode
   (make-local-variable 'term-scroll-show-maximum-output)
   (make-local-variable 'term-ptyp)
   (make-local-variable 'term-exec-hook)
+  (setq-local filter-buffer-substring-function 'term-filter-buffer-substring)
   (set (make-local-variable 'term-vertical-motion) 'vertical-motion)
   (set (make-local-variable 'term-pending-delete-marker) (make-marker))
   (make-local-variable 'term-current-face)
@@ -1132,9 +1133,33 @@ term-mode
       (setq term-input-ring (make-ring term-input-ring-size)))
   (term-update-mode-line))
 
+(defun term-insert-fake-newline (&optional count)
+  (let ((old-point (point)))
+    (term-insert-char ?\n count)
+    (put-text-property old-point (point) 'term-newline t)))
+
+(defun term-remove-fake-newlines ()
+  (goto-char (point-min))
+  (while (setq fake-newline (next-single-property-change (point)
+                                                         'term-newline))
+    (goto-char fake-newline)
+    (let (buffer-read-only)
+      (delete-char 1))))
+
+(defun term-filter-buffer-substring (beg end &optional del)
+  (let ((content (buffer-substring--filter beg end del)))
+    (with-temp-buffer
+      (insert content)
+      (term-remove-fake-newlines)
+      (buffer-string))))
+
 (defun term-reset-size (height width)
   (when (or (/= height term-height)
             (/= width term-width))
+    ;; delete all fake newlines
+    (when (/= width term-width)
+      (save-excursion
+        (term-remove-fake-newlines)))
     (let ((point (point)))
       (setq term-height height)
       (setq term-width width)
@@ -1147,7 +1172,21 @@ term-reset-size
       (setq term-start-line-column nil)
       (setq term-current-row nil)
       (setq term-current-column nil)
-      (goto-char point))))
+      (goto-char point))
+    (save-excursion
+      ;; add fake newlines for the lines that are currently displayed
+      (forward-line (- (term-current-row)))
+      (beginning-of-line)
+      (while (not (eobp))
+        (let* ((bol (line-beginning-position))
+               (eol (line-end-position))
+               (len (- eol bol)))
+          (when (> len width)
+            (goto-char (+ bol width))
+            (let (buffer-read-only)
+              (term-insert-fake-newline)))
+          (unless (eobp)
+            (forward-char)))))))
 
 ;; Recursive routine used to check if any string in term-kill-echo-list
 ;; matches part of the buffer before point.
@@ -2906,6 +2945,7 @@ term-emulate-terminal
                       (delete-region (point) (line-end-position))
                       (term-down 1 t)
                       (term-move-columns (- (term-current-column)))
+                      (put-text-property (1- (point)) (point) 'term-newline t)
                       (setq decoded-substring
                             (substring decoded-substring (- term-width 
old-column)))
                       (setq old-column 0)))
@@ -3719,7 +3759,10 @@ term-down
 ;; if the line above point wraps around, add a ?\n to undo the wrapping.
 ;; FIXME:  Probably should be called more than it is.
 (defun term-unwrap-line ()
-  (when (not (bolp)) (insert-before-markers ?\n)))
+  (when (not (bolp))
+    (let ((old-point (point)))
+      (insert-before-markers ?\n)
+      (put-text-property old-point (point) 'term-newline t))))
 
 (defun term-erase-in-line (kind)
   (when (= kind 1) ;; erase left of point
-- 
2.19.2