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

[John Shahid]

ping

John Shahid <address@hidden> 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 <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