From: John Shahid <jvshahid@gmail.com>
To: emacs-devel@gnu.org
Subject: Re: Feedback on getting rid of `term-suppress-hard-newline'
Date: Sun, 20 Jan 2019 19:14:04 -0500 [thread overview]
Message-ID: <871s56sw98.fsf@gmail.com> (raw)
In-Reply-To: <jwv36psimfa.fsf-monnier+gmane.emacs.devel@gnu.org>
[-- 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
next prev parent reply other threads:[~2019-01-21 0:14 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-12 12:14 Feedback on getting rid of `term-suppress-hard-newline' John Shahid
2019-01-16 14:14 ` John Shahid
2019-01-16 16:51 ` Stefan Monnier
2019-01-21 0:14 ` John Shahid [this message]
2019-01-21 3:04 ` Stefan Monnier
2019-01-21 20:32 ` John Shahid
2019-02-20 14:54 ` John Shahid
2019-02-21 14:55 ` Stefan Monnier
2019-02-24 18:00 ` John Shahid
[not found] ` <jwvh8cs6fzt.fsf-monnier+emacs@gnu.org>
[not found] ` <87k1ho26vc.fsf@gmail.com>
[not found] ` <87h8cr9is1.fsf@gmail.com>
2019-02-27 12:54 ` Fwd: " John Shahid
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871s56sw98.fsf@gmail.com \
--to=jvshahid@gmail.com \
--cc=emacs-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.