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

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

[John Shahid]

[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]


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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch demonstrating terminal text reflow --]
[-- Type: text/x-patch, Size: 3871 bytes --]

From 2b6332a66b56fea987fe70d336c1742ae6352ffa Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
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))
 \f
+(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

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

[John Shahid]

ping

John Shahid <jvshahid@gmail.com> writes:

> 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 <jvshahid@gmail.com>
> 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))
>  \f
> +(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

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

[Stefan Monnier]

> ping

FWIW, I think this makes a lot of sense, so please go ahead.
A few comments below,


        Stefan


>> @@ -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)

Please use `add-function` to modify filter-buffer-substring-function.

>> +    (put-text-property old-point (point) 'term-newline t)))

I'd recommend you use a more explicit name which doesn't just state the
obvious "this is a newline in term mode".  E.g. something like
`term-wrap-newline`.

>> +    (let (buffer-read-only)
>> +      (delete-char 1))))

Never let-bind `buffer-read-only`: let-bind `inhibit-read-only` instead.

>> +    ;; delete all fake newlines

Please capitalize and punctuate your comments.

>> +          (when (> len width)
>> +            (goto-char (+ bol width))

Not sure how well term deals with wide chars (e.g. TAB or chinese
chars), but this code of yours clearly won't help.  I think you
want to test current-column rather than `len` and then you want
move-to-column rather than advancing by N chars.

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

[John Shahid]

[-- Attachment #1: Type: text/plain, Size: 2097 bytes --]


Sorry it took me a while to get back to you.  I fixed most of the issues
you mentioned but I had a few questions (see below).

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> ping
>
> FWIW, I think this makes a lot of sense, so please go ahead.
> A few comments below,
>
>
>         Stefan
>
>
>>> @@ -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)
>
> Please use `add-function` to modify filter-buffer-substring-function.

Done.

>
>>> +    (put-text-property old-point (point) 'term-newline t)))
>
> I'd recommend you use a more explicit name which doesn't just state the
> obvious "this is a newline in term mode".  E.g. something like
> `term-wrap-newline`.

I changed the property name to 'term-line-wrap'.

>
>>> +    (let (buffer-read-only)
>>> +      (delete-char 1))))
>
> Never let-bind `buffer-read-only`: let-bind `inhibit-read-only`
> instead.

Done.  I am curious to know why I shouldn't let-bind 'buffer-read-only' ?

>
>>> +    ;; delete all fake newlines
>
> Please capitalize and punctuate your comments.

Done.

>
>>> +          (when (> len width)
>>> +            (goto-char (+ bol width))
>
> Not sure how well term deals with wide chars (e.g. TAB or chinese
> chars), but this code of yours clearly won't help.  I think you
> want to test current-column rather than `len` and then you want
> move-to-column rather than advancing by N chars.

I made the change in the attached patch.  I don't have any fonts locally
that are suitable for displaying Chinese characters so I couldn't test
those.  I tested with Arabic characters (e.g. 'C-x 8 RET ARABIC LIGATURE
AKBAR ISOLATED FORM') and I don't think 'current-column' is reporting
the right column number.  In this case, it reports column count of 1
which isn't right.  It does work though with tab characters.

One more question, should I deprecate 'term-suppress-hard-newline' as
part of this changeset ?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Adjust-line-wrapping-on-window-resize-and-killing-te.patch --]
[-- Type: text/x-patch, Size: 3939 bytes --]

From 8efc90d9a3a406dc795842bd86174da8e3944874 Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
Date: Sun, 20 Jan 2019 19:08:17 -0500
Subject: [PATCH] Adjust line wrapping on window resize and killing text

* lisp/term.el (term-mode): Advice filter-buffer-substring-function to remove
line wrapping from killed text.
(term-reset-size): Add or remove line wrapping depending on the new terminal
width.
(term-emulate-terminal):
(term-unwrap-line):
---
 lisp/term.el | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index f49777f94c..1406b80fd5 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -1116,6 +1116,9 @@ term-mode
 
   (set (make-local-variable 'font-lock-defaults) '(nil t))
 
+  (add-function :filter-return
+                (local 'filter-buffer-substring-function)
+                'term--filter-buffer-substring)
   (add-function :filter-return
                 (local 'window-adjust-process-window-size-function)
                 (lambda (size)
@@ -1132,9 +1135,32 @@ term-mode
       (setq term-input-ring (make-ring term-input-ring-size)))
   (term-update-mode-line))
 \f
+(defun term--insert-fake-newline (&optional count)
+  (let ((old-point (point)))
+    (term-insert-char ?\n count)
+    (put-text-property old-point (point) 'term-line-wrap t)))
+
+(defun term--remove-fake-newlines ()
+  (goto-char (point-min))
+  (while (setq fake-newline (next-single-property-change (point)
+                                                         'term-line-wrap))
+    (goto-char fake-newline)
+    (let ((inhibit-read-only t))
+      (delete-char 1))))
+
+(defun term--filter-buffer-substring (content)
+  (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 newlines used for wrapping
+    (when (/= width term-width)
+      (save-excursion
+        (term--remove-fake-newlines)))
     (let ((point (point)))
       (setq term-height height)
       (setq term-width width)
@@ -1147,7 +1173,19 @@ 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 newlines to wrap long lines that are currently displayed
+      (forward-line (- (term-current-row)))
+      (beginning-of-line)
+      (while (not (eobp))
+        (let* ((eol (save-excursion (end-of-line) (current-column))))
+          (when (> eol width)
+            (move-to-column width)
+            (let ((inhibit-read-only t))
+              (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 +2944,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-line-wrap t)
                       (setq decoded-substring
                             (substring decoded-substring (- term-width old-column)))
                       (setq old-column 0)))
@@ -3719,7 +3758,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-line-wrap t))))
 
 (defun term-erase-in-line (kind)
   (when (= kind 1) ;; erase left of point
-- 
2.20.1

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

[Stefan Monnier]

> I changed the property name to 'term-line-wrap'.

Thanks,

>>>> +    (let (buffer-read-only)
>>>> +      (delete-char 1))))
>> Never let-bind `buffer-read-only`: let-bind `inhibit-read-only`
>> instead.
> Done.  I am curious to know why I shouldn't let-bind 'buffer-read-only' ?

Various minor reasons:
- it also allows modifying text with `read-only` text-property.
- It allows the wrapped code to change read-only-mode if it wants/needs to,
  whereas let-binding buffer-read-only means that if the wrapped code
  changes read-only-mode, we'll silently undo this change at the end.
- `buffer-read-only` is a variable to *set* or *unset* rather than
  to let-bind temporarily.  Using inhibit-read-only clarifies that you
  just want to temporarily override the read-only-ness rather than to
  change the read-only-mode.
Depending on the situation, different things matter more.
Here it likely doesn't make much of a difference in practice, but the
normal style is to bind inhibit-read-only: that's what it's for.

> One more question, should I deprecate 'term-suppress-hard-newline' as
> part of this changeset ?

I think it should mark it as obsolete, yes.


        Stefan

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

[John Shahid]

[-- Attachment #1: Type: text/plain, Size: 1426 bytes --]


Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I changed the property name to 'term-line-wrap'.
>
> Thanks,
>
>>>>> +    (let (buffer-read-only)
>>>>> +      (delete-char 1))))
>>> Never let-bind `buffer-read-only`: let-bind `inhibit-read-only`
>>> instead.
>> Done.  I am curious to know why I shouldn't let-bind 'buffer-read-only' ?
>
> Various minor reasons:
> - it also allows modifying text with `read-only` text-property.
> - It allows the wrapped code to change read-only-mode if it wants/needs to,
>   whereas let-binding buffer-read-only means that if the wrapped code
>   changes read-only-mode, we'll silently undo this change at the end.
> - `buffer-read-only` is a variable to *set* or *unset* rather than
>   to let-bind temporarily.  Using inhibit-read-only clarifies that you
>   just want to temporarily override the read-only-ness rather than to
>   change the read-only-mode.
> Depending on the situation, different things matter more.
> Here it likely doesn't make much of a difference in practice, but the
> normal style is to bind inhibit-read-only: that's what it's for.

That makes sense.  Thanks for the detailed answer.

>
>> One more question, should I deprecate 'term-suppress-hard-newline' as
>> part of this changeset ?
>
> I think it should mark it as obsolete, yes.

Awesome.  I made the change and attached the latest patch.  FYI, I also
went removed all usages of that variable.

JS


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Adjust-line-wrapping-on-window-resize-and-killing-te.patch --]
[-- Type: text/x-patch, Size: 6066 bytes --]

From f4a9e0f4968fa253da40eaf7f6fd84fa52c650b5 Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
Date: Sun, 20 Jan 2019 19:08:17 -0500
Subject: [PATCH] Adjust line wrapping on window resize and killing text

* lisp/term.el (term-mode): Advice filter-buffer-substring-function to remove
line wrapping from killed text.
(term-reset-size): Add or remove line wrapping depending on the new terminal
width.
(term-suppress-hard-newline): Mark obsolete.
(term-emulate-terminal): Remove usage of 'term-suppress-hard-newline'
(term-unwrap-line):
---
 lisp/term.el | 77 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 61 insertions(+), 16 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index f49777f94c..e9ed4a688f 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -545,6 +545,9 @@ term-suppress-hard-newline
   :version "24.4"
   :type 'boolean
   :group 'term)
+(make-obsolete-variable 'term-suppress-hard-newline nil
+                        "term removes newlines used for wrapping on resize and when text is copied"
+                        "27.1")
 
 ;; Where gud-display-frame should put the debugging arrow.  This is
 ;; set by the marker-filter, which scans the debugger's output for
@@ -1116,6 +1119,9 @@ term-mode
 
   (set (make-local-variable 'font-lock-defaults) '(nil t))
 
+  (add-function :filter-return
+                (local 'filter-buffer-substring-function)
+                'term--filter-buffer-substring)
   (add-function :filter-return
                 (local 'window-adjust-process-window-size-function)
                 (lambda (size)
@@ -1132,9 +1138,33 @@ term-mode
       (setq term-input-ring (make-ring term-input-ring-size)))
   (term-update-mode-line))
 \f
+(defun term--insert-fake-newline (&optional count)
+  (let ((old-point (point)))
+    (term-insert-char ?\n count)
+    (put-text-property old-point (point) 'term-line-wrap t)))
+
+(defun term--remove-fake-newlines ()
+  (goto-char (point-min))
+  (let (fake-newline)
+    (while (setq fake-newline (next-single-property-change (point)
+                                                           'term-line-wrap))
+      (goto-char fake-newline)
+      (let ((inhibit-read-only t))
+        (delete-char 1)))))
+
+(defun term--filter-buffer-substring (content)
+  (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 newlines used for wrapping
+    (when (/= width term-width)
+      (save-excursion
+        (term--remove-fake-newlines)))
     (let ((point (point)))
       (setq term-height height)
       (setq term-width width)
@@ -1147,7 +1177,19 @@ 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 newlines to wrap long lines that are currently displayed
+      (forward-line (- (term-current-row)))
+      (beginning-of-line)
+      (while (not (eobp))
+        (let* ((eol (save-excursion (end-of-line) (current-column))))
+          (when (> eol width)
+            (move-to-column width)
+            (let ((inhibit-read-only t))
+              (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.
@@ -2895,20 +2937,20 @@ term-emulate-terminal
                 (let ((old-column (term-horizontal-column))
                       (old-point (point))
                       columns)
-                  (unless term-suppress-hard-newline
-                    (while (> (+ (length decoded-substring) old-column)
-                              term-width)
-                      (insert (substring decoded-substring 0
-                                         (- term-width old-column)))
-                      ;; Since we've enough text to fill the whole line,
-                      ;; delete previous text regardless of
-                      ;; `term-insert-mode's value.
-                      (delete-region (point) (line-end-position))
-                      (term-down 1 t)
-                      (term-move-columns (- (term-current-column)))
-                      (setq decoded-substring
-                            (substring decoded-substring (- term-width old-column)))
-                      (setq old-column 0)))
+                  (while (> (+ (length decoded-substring) old-column)
+                            term-width)
+                    (insert (substring decoded-substring 0
+                                       (- term-width old-column)))
+                    ;; Since we've enough text to fill the whole line,
+                    ;; delete previous text regardless of
+                    ;; `term-insert-mode's value.
+                    (delete-region (point) (line-end-position))
+                    (term-down 1 t)
+                    (term-move-columns (- (term-current-column)))
+                    (put-text-property (1- (point)) (point) 'term-line-wrap t)
+                    (setq decoded-substring
+                          (substring decoded-substring (- term-width old-column)))
+                    (setq old-column 0))
                   (insert decoded-substring)
                   (setq term-current-column (current-column)
                         columns (- term-current-column old-column))
@@ -3719,7 +3761,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-line-wrap t))))
 
 (defun term-erase-in-line (kind)
   (when (= kind 1) ;; erase left of point
-- 
2.20.1

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

[John Shahid]

Are there more changes to the previously attached patch in order to
merge it into master ?

JS

John Shahid <jvshahid@gmail.com> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> I changed the property name to 'term-line-wrap'.
>>
>> Thanks,
>>
>>>>>> +    (let (buffer-read-only)
>>>>>> +      (delete-char 1))))
>>>> Never let-bind `buffer-read-only`: let-bind `inhibit-read-only`
>>>> instead.
>>> Done.  I am curious to know why I shouldn't let-bind 'buffer-read-only' ?
>>
>> Various minor reasons:
>> - it also allows modifying text with `read-only` text-property.
>> - It allows the wrapped code to change read-only-mode if it wants/needs to,
>>   whereas let-binding buffer-read-only means that if the wrapped code
>>   changes read-only-mode, we'll silently undo this change at the end.
>> - `buffer-read-only` is a variable to *set* or *unset* rather than
>>   to let-bind temporarily.  Using inhibit-read-only clarifies that you
>>   just want to temporarily override the read-only-ness rather than to
>>   change the read-only-mode.
>> Depending on the situation, different things matter more.
>> Here it likely doesn't make much of a difference in practice, but the
>> normal style is to bind inhibit-read-only: that's what it's for.
>
> That makes sense.  Thanks for the detailed answer.
>
>>
>>> One more question, should I deprecate 'term-suppress-hard-newline' as
>>> part of this changeset ?
>>
>> I think it should mark it as obsolete, yes.
>
> Awesome.  I made the change and attached the latest patch.  FYI, I also
> went removed all usages of that variable.
>
> JS
>
> From f4a9e0f4968fa253da40eaf7f6fd84fa52c650b5 Mon Sep 17 00:00:00 2001
> From: John Shahid <jvshahid@gmail.com>
> Date: Sun, 20 Jan 2019 19:08:17 -0500
> Subject: [PATCH] Adjust line wrapping on window resize and killing text
>
> * lisp/term.el (term-mode): Advice filter-buffer-substring-function to remove
> line wrapping from killed text.
> (term-reset-size): Add or remove line wrapping depending on the new terminal
> width.
> (term-suppress-hard-newline): Mark obsolete.
> (term-emulate-terminal): Remove usage of 'term-suppress-hard-newline'
> (term-unwrap-line):
> ---
>  lisp/term.el | 77 +++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 61 insertions(+), 16 deletions(-)
>
> diff --git a/lisp/term.el b/lisp/term.el
> index f49777f94c..e9ed4a688f 100644
> --- a/lisp/term.el
> +++ b/lisp/term.el
> @@ -545,6 +545,9 @@ term-suppress-hard-newline
>    :version "24.4"
>    :type 'boolean
>    :group 'term)
> +(make-obsolete-variable 'term-suppress-hard-newline nil
> +                        "term removes newlines used for wrapping on resize and when text is copied"
> +                        "27.1")
>  
>  ;; Where gud-display-frame should put the debugging arrow.  This is
>  ;; set by the marker-filter, which scans the debugger's output for
> @@ -1116,6 +1119,9 @@ term-mode
>  
>    (set (make-local-variable 'font-lock-defaults) '(nil t))
>  
> +  (add-function :filter-return
> +                (local 'filter-buffer-substring-function)
> +                'term--filter-buffer-substring)
>    (add-function :filter-return
>                  (local 'window-adjust-process-window-size-function)
>                  (lambda (size)
> @@ -1132,9 +1138,33 @@ term-mode
>        (setq term-input-ring (make-ring term-input-ring-size)))
>    (term-update-mode-line))
>  \f
> +(defun term--insert-fake-newline (&optional count)
> +  (let ((old-point (point)))
> +    (term-insert-char ?\n count)
> +    (put-text-property old-point (point) 'term-line-wrap t)))
> +
> +(defun term--remove-fake-newlines ()
> +  (goto-char (point-min))
> +  (let (fake-newline)
> +    (while (setq fake-newline (next-single-property-change (point)
> +                                                           'term-line-wrap))
> +      (goto-char fake-newline)
> +      (let ((inhibit-read-only t))
> +        (delete-char 1)))))
> +
> +(defun term--filter-buffer-substring (content)
> +  (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 newlines used for wrapping
> +    (when (/= width term-width)
> +      (save-excursion
> +        (term--remove-fake-newlines)))
>      (let ((point (point)))
>        (setq term-height height)
>        (setq term-width width)
> @@ -1147,7 +1177,19 @@ 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 newlines to wrap long lines that are currently displayed
> +      (forward-line (- (term-current-row)))
> +      (beginning-of-line)
> +      (while (not (eobp))
> +        (let* ((eol (save-excursion (end-of-line) (current-column))))
> +          (when (> eol width)
> +            (move-to-column width)
> +            (let ((inhibit-read-only t))
> +              (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.
> @@ -2895,20 +2937,20 @@ term-emulate-terminal
>                  (let ((old-column (term-horizontal-column))
>                        (old-point (point))
>                        columns)
> -                  (unless term-suppress-hard-newline
> -                    (while (> (+ (length decoded-substring) old-column)
> -                              term-width)
> -                      (insert (substring decoded-substring 0
> -                                         (- term-width old-column)))
> -                      ;; Since we've enough text to fill the whole line,
> -                      ;; delete previous text regardless of
> -                      ;; `term-insert-mode's value.
> -                      (delete-region (point) (line-end-position))
> -                      (term-down 1 t)
> -                      (term-move-columns (- (term-current-column)))
> -                      (setq decoded-substring
> -                            (substring decoded-substring (- term-width old-column)))
> -                      (setq old-column 0)))
> +                  (while (> (+ (length decoded-substring) old-column)
> +                            term-width)
> +                    (insert (substring decoded-substring 0
> +                                       (- term-width old-column)))
> +                    ;; Since we've enough text to fill the whole line,
> +                    ;; delete previous text regardless of
> +                    ;; `term-insert-mode's value.
> +                    (delete-region (point) (line-end-position))
> +                    (term-down 1 t)
> +                    (term-move-columns (- (term-current-column)))
> +                    (put-text-property (1- (point)) (point) 'term-line-wrap t)
> +                    (setq decoded-substring
> +                          (substring decoded-substring (- term-width old-column)))
> +                    (setq old-column 0))
>                    (insert decoded-substring)
>                    (setq term-current-column (current-column)
>                          columns (- term-current-column old-column))
> @@ -3719,7 +3761,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-line-wrap t))))
>  
>  (defun term-erase-in-line (kind)
>    (when (= kind 1) ;; erase left of point

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

[Stefan Monnier]

John Shahid <jvshahid@gmail.com> writes:
> Are there more changes to the previously attached patch in order to
> merge it into master ?

Oh, sorry, I dropped the ball.
Yes, it looks good, thank you very much.

Of course I couldn't resist making further comments, but feel free to
push it,


        Stefan


> +(make-obsolete-variable 'term-suppress-hard-newline nil
> +                        "term removes newlines used for wrapping on resize and when text is copied"
> +                        "27.1")

Please remove the third argument above (it should be the version).

> +  (add-function :filter-return
> +                (local 'filter-buffer-substring-function)
> +                'term--filter-buffer-substring)
                  ^
I'd put a # there.

> +(defun term--insert-fake-newline (&optional count)
> +  (let ((old-point (point)))
> +    (term-insert-char ?\n count)
> +    (put-text-property old-point (point) 'term-line-wrap t)))

`count` is always nil, AFAICT, so you can just drop this argument.

> +(defun term--remove-fake-newlines ()
> +  (goto-char (point-min))
> +  (let (fake-newline)
> +    (while (setq fake-newline (next-single-property-change (point)
> +                                                           'term-line-wrap))
> +      (goto-char fake-newline)
> +      (let ((inhibit-read-only t))
> +        (delete-char 1)))))

I'd add a test (or an assertion) that (eq ?\n (char-after))), in case
the text-property ends up applied somewhere we didn't expect
(e.g. because it was inherited via insert-and-inherit).

> @@ -1147,7 +1177,19 @@ 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 newlines to wrap long lines that are currently displayed
> +      (forward-line (- (term-current-row)))
> +      (beginning-of-line)
> +      (while (not (eobp))
> +        (let* ((eol (save-excursion (end-of-line) (current-column))))
> +          (when (> eol width)
> +            (move-to-column width)
> +            (let ((inhibit-read-only t))
> +              (term--insert-fake-newline)))
> +          (unless (eobp)
> +            (forward-char)))))))

I'd move this to a separate function.

More importantly, I don't quite understand why (- (term-current-row)))
should be the exactly correct number of lines to move back at the
beginning, so please add a comment explaining it (or if it's not exactly
right, the comment could explain why it's "right enough").

Also, I think you can simplify the loop by always calling
`move-to-column` and never `current-column`, as in the 100% untested
code below:

    (while (not (eobp))
      (move-to-column width)
      (if (eolp)
          (forward-char)
        (let ((inhibit-read-only t))
          (term--insert-fake-newline))))

> @@ -2895,20 +2937,20 @@ term-emulate-terminal
>                  (let ((old-column (term-horizontal-column))
>                        (old-point (point))
>                        columns)
> -                  (unless term-suppress-hard-newline
> -                    (while (> (+ (length decoded-substring) old-column)
> -                              term-width)
> -                      (insert (substring decoded-substring 0
> -                                         (- term-width old-column)))
> -                      ;; Since we've enough text to fill the whole line,
> -                      ;; delete previous text regardless of
> -                      ;; `term-insert-mode's value.
> -                      (delete-region (point) (line-end-position))
> -                      (term-down 1 t)
> -                      (term-move-columns (- (term-current-column)))
> -                      (setq decoded-substring
> -                            (substring decoded-substring (- term-width old-column)))
> -                      (setq old-column 0)))
> +                  (while (> (+ (length decoded-substring) old-column)
> +                            term-width)
> +                    (insert (substring decoded-substring 0
> +                                       (- term-width old-column)))
> +                    ;; Since we've enough text to fill the whole line,
> +                    ;; delete previous text regardless of
> +                    ;; `term-insert-mode's value.
> +                    (delete-region (point) (line-end-position))
> +                    (term-down 1 t)
> +                    (term-move-columns (- (term-current-column)))
> +                    (put-text-property (1- (point)) (point) 'term-line-wrap t)
> +                    (setq decoded-substring
> +                          (substring decoded-substring (- term-width old-column)))
> +                    (setq old-column 0))
>                    (insert decoded-substring)
>                    (setq term-current-column (current-column)
>                          columns (- term-current-column old-column))

I think I'd leave the term-suppress-hard-newline test in for now (that's
the point of marking is as obsolete).

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

[John Shahid]

[-- Attachment #1: Type: text/plain, Size: 4312 bytes --]


Stefan Monnier <monnier@iro.umontreal.ca> writes:

> John Shahid <jvshahid@gmail.com> writes:
>> Are there more changes to the previously attached patch in order to
>> merge it into master ?
>
> Oh, sorry, I dropped the ball.
> Yes, it looks good, thank you very much.

No worries and thanks for taking the time to review this patch.

> Of course I couldn't resist making further comments, but feel free to
> push it,

Great, I made some changes to the patch, see below.

>         Stefan
>
>
>> +(make-obsolete-variable 'term-suppress-hard-newline nil
>> +                        "term removes newlines used for wrapping on resize and when text is copied"
>> +                        "27.1")
>
> Please remove the third argument above (it should be the version).

Done.  I can't remember what I was thinking at the time when I wrote
this, it was a long time ago.

>> +  (add-function :filter-return
>> +                (local 'filter-buffer-substring-function)
>> +                'term--filter-buffer-substring)
>                   ^
> I'd put a # there.

Done.

>> +(defun term--insert-fake-newline (&optional count)
>> +  (let ((old-point (point)))
>> +    (term-insert-char ?\n count)
>> +    (put-text-property old-point (point) 'term-line-wrap t)))
>
> `count` is always nil, AFAICT, so you can just drop this argument.

I got rid of this function.  Replaced it with calls to
`term-unwrap-line'.

>> +(defun term--remove-fake-newlines ()
>> +  (goto-char (point-min))
>> +  (let (fake-newline)
>> +    (while (setq fake-newline (next-single-property-change (point)
>> +                                                           'term-line-wrap))
>> +      (goto-char fake-newline)
>> +      (let ((inhibit-read-only t))
>> +        (delete-char 1)))))
>
> I'd add a test (or an assertion) that (eq ?\n (char-after))), in case
> the text-property ends up applied somewhere we didn't expect
> (e.g. because it was inherited via insert-and-inherit).

I added an assertion.  I don't see any uses of `insert-and-inherit' in
term.el and based on my limited testing this test passes, in other words
the assertion didn't fail.

>> @@ -1147,7 +1177,19 @@ 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 newlines to wrap long lines that are currently displayed
>> +      (forward-line (- (term-current-row)))
>> +      (beginning-of-line)
>> +      (while (not (eobp))
>> +        (let* ((eol (save-excursion (end-of-line) (current-column))))
>> +          (when (> eol width)
>> +            (move-to-column width)
>> +            (let ((inhibit-read-only t))
>> +              (term--insert-fake-newline)))
>> +          (unless (eobp)
>> +            (forward-char)))))))
>
> I'd move this to a separate function.

Done, introduced a new function `term--unwrap-visible-long-lines'.

> More importantly, I don't quite understand why (- (term-current-row)))
> should be the exactly correct number of lines to move back at the
> beginning, so please add a comment explaining it (or if it's not exactly
> right, the comment could explain why it's "right enough").

I replaced this call with using `term-home-marker'.  I believe the two
approaches are equivalent.

I'm assuming that this marker will correctly account for the home
position and I'm also assuming that terminal programs won't try to go
past that marker.  I think that the accounting could be off, specially
during resizing, but when I was testing, that assumption didn't cause
any noticeable issues.

BTW, I tried to look for documentation on how terminals handle resize
but couldn't find one.  Maybe I should pick a terminal emulator and
inspect the code.

> Also, I think you can simplify the loop by always calling
> `move-to-column` and never `current-column`, as in the 100% untested
> code below:
>
>     (while (not (eobp))
>       (move-to-column width)
>       (if (eolp)
>           (forward-char)
>         (let ((inhibit-read-only t))
>           (term--insert-fake-newline))))

Done.

[...]

> I think I'd leave the term-suppress-hard-newline test in for now (that's
> the point of marking is as obsolete).

I reverted this change.

Cheers,

JS


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Adjust-line-wrapping-on-window-resize-and-killing-te.patch --]
[-- Type: text/x-patch, Size: 4265 bytes --]

From 966edc96b34b998b04d1d9de8529c6d01dae8192 Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
Date: Sun, 20 Jan 2019 19:08:17 -0500
Subject: [PATCH] Adjust line wrapping on window resize and killing text

* lisp/term.el (term-mode): Advice filter-buffer-substring-function to
remove line unwrapping from killed text.
(term-reset-size): Add or remove line unwrapping depending on the new
terminal width.
(term-suppress-hard-newline): Mark obsolete.
(term-unwrap-line): Use text properties to be able to find the
newlines later.
---
 lisp/term.el | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 2 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index f49777f94c..fdcc39de72 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -545,6 +545,9 @@ term-suppress-hard-newline
   :version "24.4"
   :type 'boolean
   :group 'term)
+(make-obsolete-variable 'term-suppress-hard-newline nil
+                        "27.1"
+                        'set)
 
 ;; Where gud-display-frame should put the debugging arrow.  This is
 ;; set by the marker-filter, which scans the debugger's output for
@@ -1116,6 +1119,9 @@ term-mode
 
   (set (make-local-variable 'font-lock-defaults) '(nil t))
 
+  (add-function :filter-return
+                (local 'filter-buffer-substring-function)
+                #'term--filter-buffer-substring)
   (add-function :filter-return
                 (local 'window-adjust-process-window-size-function)
                 (lambda (size)
@@ -1132,9 +1138,51 @@ term-mode
       (setq term-input-ring (make-ring term-input-ring-size)))
   (term-update-mode-line))
 \f
+(defun term--remove-fake-newlines ()
+  (goto-char (point-min))
+  (let (fake-newline)
+    (while (setq fake-newline (next-single-property-change (point)
+                                                           'term-line-wrap))
+      (goto-char fake-newline)
+      (assert (eq ?\n (char-after)))
+      (let ((inhibit-read-only t))
+        (delete-char 1)))))
+
+(defun term--filter-buffer-substring (content)
+  (with-temp-buffer
+    (insert content)
+    (term--remove-fake-newlines)
+    (buffer-string)))
+
+(defun term--unwrap-visible-long-lines (width)
+  ;; Unwrap lines longer than width using fake newlines.  Only do it
+  ;; for lines that are currently visible (i.e. following the home
+  ;; marker).  Invisible lines don't have to be unwrapped since they
+  ;; are unreachable using the cursor movement anyway.  Not having to
+  ;; unwrap the entire buffer means the runtime of this function is
+  ;; bounded by the size of the screen instead of the buffer size.
+
+  (save-excursion
+    ;; We will just assume that our accounting for the home marker is
+    ;; correct, i.e. programs will not try to reach any position
+    ;; earlier than this marker.
+    (goto-char term-home-marker)
+
+    (move-to-column width)
+    (while (not (eobp))
+      (if (eolp)
+          (forward-char)
+        (let ((inhibit-read-only t))
+          (term-unwrap-line)))
+      (move-to-column width))))
+
 (defun term-reset-size (height width)
   (when (or (/= height term-height)
             (/= width term-width))
+    ;; Delete all newlines used for wrapping
+    (when (/= width term-width)
+      (save-excursion
+        (term--remove-fake-newlines)))
     (let ((point (point)))
       (setq term-height height)
       (setq term-width width)
@@ -1147,7 +1195,8 @@ 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))
+    (term--unwrap-visible-long-lines width)))
 
 ;; Recursive routine used to check if any string in term-kill-echo-list
 ;; matches part of the buffer before point.
@@ -3719,7 +3768,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-line-wrap t))))
 
 (defun term-erase-in-line (kind)
   (when (= kind 1) ;; erase left of point
-- 
2.20.1

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

[John Shahid]

I just realized that my previous email was just to Stefan. Re-sending to
emacs-devel.

As I mentioned below, I noticed some missing logic when I was wrapping
up the final patch that was pushed to master a few days ago.  I fixed
the problem in the patch attached below.  Can someone please push it to
master.

Thanks,

JS

John Shahid <jvshahid@gmail.com> writes:

> Looks like I removed an important piece of logic when I reverted the
> removal of `(unless term-suppress-hard-newline...`.  The change seemed
> innocent enough, that I thought it didn't require testing.
>
> I added back the required logic in the attached patch.  Can you please
> install it ?
>
> Thanks,
>
> JS
>
> From d20013c6fdd9d84d216d77135f5ef012f8fb3f06 Mon Sep 17 00:00:00 2001
> From: John Shahid <jvshahid@gmail.com>
> Date: Tue, 26 Feb 2019 01:06:53 -0500
> Subject: [PATCH] Add text properties to newlines used to unwrap long lines.
>
> * lisp/term.el (term-emulate-terminal): do it.
> ---
>  lisp/term.el | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lisp/term.el b/lisp/term.el
> index e759bb8e4f..693362cc73 100644
> --- a/lisp/term.el
> +++ b/lisp/term.el
> @@ -2935,6 +2935,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-line-wrap t)
>                        (setq decoded-substring
>                              (substring decoded-substring (- term-width old-column)))
>                        (setq old-column 0)))