From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: John Shahid Newsgroups: gmane.emacs.devel Subject: Re: Feedback on getting rid of `term-suppress-hard-newline' Date: Wed, 20 Feb 2019 09:54:18 -0500 Message-ID: <87sgwildd1.fsf@gmail.com> References: <87efanc576.fsf@gmail.com> <87sgxsr8p0.fsf@gmail.com> <871s56sw98.fsf@gmail.com> <87lg3d3g70.fsf@gmail.com> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="103462"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: mu4e 1.1.0; emacs 27.0.50 To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Feb 20 16:58:09 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1gwUG8-000Qmw-PM for ged-emacs-devel@m.gmane.org; Wed, 20 Feb 2019 16:58:09 +0100 Original-Received: from localhost ([127.0.0.1]:41478 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gwUG7-0002mm-Hc for ged-emacs-devel@m.gmane.org; Wed, 20 Feb 2019 10:58:07 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:56469) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gwU1a-0006gl-Ua for emacs-devel@gnu.org; Wed, 20 Feb 2019 10:43:08 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gwU1Z-0001XG-FT for emacs-devel@gnu.org; Wed, 20 Feb 2019 10:43:06 -0500 Original-Received: from mail-qt1-x836.google.com ([2607:f8b0:4864:20::836]:45754) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gwU1Z-0001WX-8t for emacs-devel@gnu.org; Wed, 20 Feb 2019 10:43:05 -0500 Original-Received: by mail-qt1-x836.google.com with SMTP id d18so17612772qtg.12 for ; Wed, 20 Feb 2019 07:43:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=references:user-agent:from:to:subject:in-reply-to:date:message-id :mime-version; bh=UX+1N45lnHed8A76ABPzZslGCxbwQajAMoqm3A8jV8g=; b=K4Y5jwD0YAu8a0lC19/CdyXY1QNnxPWfsDgYRfexLs3x4M7O8e1Ud4VQ8/V1y8wnSG tSlWGkITrz23CauN5yCcujURqPpJB9FPKm3QMQRRuSVRi8xGrz5yVjCb3uoEAwmbcqMj OdlvdSWwqRi5eZ4u/LdxOfB2ZbNxQdIB9a4NT0CQ5JN/dlKjInoKS34LIhhxNaxm+ZmU PbZ/U7U36GFe0NozkfPZtRa/mlwlx247xjFJDrLeE/YJCl4WVzwJkejZO3BA39suowQK FBKfN0aJC7s+BW5B9qPuj3nVhK9N0s4hjvSQWCaMI/th/ygiBv1yldzVBoNCOrUviWKM +GWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:references:user-agent:from:to:subject :in-reply-to:date:message-id:mime-version; bh=UX+1N45lnHed8A76ABPzZslGCxbwQajAMoqm3A8jV8g=; b=Kxin4CRjjZh23fR/QhJqM/BhI6brVL9P+eAgZapRFQoxxDXMhqG3Y3kXfpE3DLW5pH +2pjQkLEwKxcXpv4bxQRj9TPhSQ7ABOE22qMSFKdajpL51csGVYDAiDgxYaDPbWy4uZo fjgq8MLLJUIFJ4xLWV04ruoCSrXEpzGSw/itnGaFAdGVHZEFMxl9EUAhNrlt3udbY7cV tqkU1mWxTmIAZbTBxIxo+mUUs2gVKjxEthAg1m7zIPlrPXizM87A8tMNXAvM7X0nVUIE jJce2vnb1/8XSV8WH0389FzivUzi1OvUPtNjg+HMecE45z5m+2txzRRG1kvmyB1P7CZr LLZw== X-Gm-Message-State: AHQUAuZQ6IJtz89mgJjKuibxFk/4b2LOk32NGzuFaX+FdY/R/BgKnGMW jfGkr0uK0uBwhzNUHckt35y9ng6O X-Google-Smtp-Source: AHgI3IaN8OEJPj95D9n+mT8lHXtG58bR/hFLpLaOYXSWID31VwKAlVlIDJgD+KXCwpAjX9tvfbCjPg== X-Received: by 2002:a0c:b524:: with SMTP id d36mr10227655qve.48.1550674462836; Wed, 20 Feb 2019 06:54:22 -0800 (PST) Original-Received: from amun (cpe-104-162-85-46.nyc.res.rr.com. [104.162.85.46]) by smtp.gmail.com with ESMTPSA id t123sm9826697qkc.6.2019.02.20.06.54.21 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 20 Feb 2019 06:54:21 -0800 (PST) In-reply-to: <87lg3d3g70.fsf@gmail.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4864:20::836 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:233484 Archived-At: Are there more changes to the previously attached patch in order to merge it into master ? JS John Shahid writes: > Stefan Monnier 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 > 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)) > > +(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