emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Few enhancements to ansi-term


From: John Shahid
Subject: Re: Few enhancements to ansi-term
Date: Tue, 30 Apr 2019 17:50:54 -0400
User-agent: mu4e 1.1.0; emacs 27.0.50

Stefan Monnier <address@hidden> writes:

>> * lisp/term.el (term-set-scroll-region): Do not set
>>   term-scroll-with-delete when the region is set to the height of the
>>   terminal.
>> * lisp/term.el (term-reset-terminal): Reset term-scroll-with-delete.
>
> Better write these as:
>
>> * lisp/term.el (term-set-scroll-region): Do not set
>> term-scroll-with-delete when the region is set to the height of the
>> terminal.
>> (term-reset-terminal): Reset term-scroll-with-delete.

Fixed in the attached patch.  I ended up changing more functions.  I
hope that the commit message still follow the guidelines.

>
> [ I read the first entry and thought the next was about another file so
>   I didn't even look at it.  ]
>
> The patch also changes term-reset-size but the ChangeLog doesn't explain
> what this change does.
>
>> @@ -3439,7 +3440,7 @@ term-set-scroll-region
>>    (setq term-scroll-with-delete
>>      (or (term-using-alternate-sub-buffer)
>>          (not (and (= term-scroll-start 0)
>> -                  (= term-scroll-end term-height)))))
>> +                      (= term-scroll-end (1- term-height))))))
>
> The code just above does:
>
>     (setq term-scroll-end
>           (if (or (<= bottom term-scroll-start) (> bottom term-height))
>               term-height
>             bottom))
>
> Should this also be changed to (1- term-height)?
> And/or should the test use <= as in:
>
>           (not (and (<= term-scroll-start 0)
>                       (>= term-scroll-end (1- term-height))))))
>
> ?

Good catch.  I fixed it in the latest patch and added more tests.

>
>> * lisp/term.el (term-unwrap-line, term-emulate-terminal): Add
>>   rear-nonsticky text property to the newlines used for line wrapping.
>
> The diff basically say that already, so I'd rather write it as "Prevent
> the `term-line-wrap` property of newlines from spreading accidentally
> when inserting text next to it" or something like that.

I stole your above wording, thanks!

> As for the code, it looks good to me.

Thanks for reviewing the changes.

JS

Attachment: 0001-Fix-setting-and-resetting-of-scroll-with-delete.patch
Description: Text Data

Attachment: 0001-Prevent-accidental-edits-in-the-ansi-term-buffer-fro.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]