bug#5615: 23.1.92; [PATCH] term.el: Calculation of window height is bad (new patch)

bug#5615: 23.1.92; [PATCH] term.el: Calculation of window height is bad (new patch)

[IRIE Shinsuke]

Sorry, the patch I sent with previous mail is bad, because the previous
patch was made without considering `line-spacing' specified by a
floating point number or frame-parameter.

So I wrote the new patch. Please check it.


IRIE Shinsuke

bug#5615: 23.1.92; [PATCH] term.el: Calculation of window height is bad (new patch)

[IRIE Shinsuke]

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

Sorry, the patch I sent with previous mail is bad, because the previous
patch was made without considering `line-spacing' specified by a
floating point number or frame-parameter.

So I wrote the new patch. Please check it.


IRIE Shinsuke


[-- Attachment #2: term-window-height-newpatch.patch --]
[-- Type: text/x-diff, Size: 2034 bytes --]

=== modified file 'lisp/term.el'
*** old/lisp/term.el	2010-03-16 14:48:29 +0000
--- new/lisp/term.el	2010-03-19 09:53:36 +0000
***************
*** 942,947 ****
--- 942,958 ----
  	(window-width)
        (1- (window-width)))))
  
+ (defun term-window-height ()
+   (if (display-graphic-p)
+       (let ((e (window-inside-pixel-edges))
+ 	    (s (or (with-current-buffer (window-buffer) line-spacing)
+ 		   (frame-parameter nil 'line-spacing)
+ 		   0)))
+ 	(if (floatp s)
+ 	    (setq s (truncate (* (frame-char-height) s))))
+ 	(/ (+ (- (nth 3 e) (cadr e)) s)
+ 	   (+ (frame-char-height) s)))
+     (window-text-height)))
  \f
  (put 'term-mode 'mode-class 'special)
  
***************
*** 1039,1045 ****
    (make-local-variable 'term-height)
    (make-local-variable 'term-width)
    (setq term-width (term-window-width))
!   (setq term-height (1- (window-height)))
    (make-local-variable 'term-terminal-parameter)
    (make-local-variable 'term-saved-cursor)
    (make-local-variable 'term-last-input-start)
--- 1050,1056 ----
    (make-local-variable 'term-height)
    (make-local-variable 'term-width)
    (setq term-width (term-window-width))
!   (setq term-height (term-window-height))
    (make-local-variable 'term-terminal-parameter)
    (make-local-variable 'term-saved-cursor)
    (make-local-variable 'term-last-input-start)
***************
*** 1182,1190 ****
      found))
  
  (defun term-check-size (process)
!   (when (or (/= term-height (1- (window-height)))
  	    (/= term-width (term-window-width)))
!     (term-reset-size (1- (window-height)) (term-window-width))
      (set-process-window-size process term-height term-width)))
  
  (defun term-send-raw-string (chars)
--- 1193,1201 ----
      found))
  
  (defun term-check-size (process)
!   (when (or (/= term-height (term-window-height))
  	    (/= term-width (term-window-width)))
!     (term-reset-size (term-window-height) (term-window-width))
      (set-process-window-size process term-height term-width)))
  
  (defun term-send-raw-string (chars)

bug#5615: 23.1.92; [PATCH] term.el: Calculation of window height is bad (new patch)

[IRIE Shinsuke]

Oops, I missed attaching the patch to the mail... Sorry.


IRIE Shinsuke

bug#5615: 23.1.92; [PATCH] term.el: Calculation of window height is bad (new patch)

[Ken Hori]

Haven't tried yet, but thanks for the patch.

I'm sure this "off-by-2-lines" problem has been an annoyance to most
forks who use term.el off the bzr head.

2010/3/19 IRIE Shinsuke <irieshinsuke@yahoo.co.jp>:
> Oops, I missed attaching the patch to the mail... Sorry.
>
>
> IRIE Shinsuke
>
>
>
>
>
>

bug#5615: 23.1.92; [PATCH] term.el: Calculation of window height is bad (new patch)

[Lars Ingebrigtsen]

IRIE Shinsuke <irieshinsuke@yahoo.co.jp> writes:

> Sorry, the patch I sent with previous mail is bad, because the previous
> patch was made without considering `line-spacing' specified by a
> floating point number or frame-parameter.
>
> So I wrote the new patch. Please check it.

[...]

Is this still a problem in the current Emacs?

> + (defun term-window-height ()
> +   (if (display-graphic-p)
> +       (let ((e (window-inside-pixel-edges))
> + 	    (s (or (with-current-buffer (window-buffer) line-spacing)
> + 		   (frame-parameter nil 'line-spacing)
> + 		   0)))
> + 	(if (floatp s)
> + 	    (setq s (truncate (* (frame-char-height) s))))
> + 	(/ (+ (- (nth 3 e) (cadr e)) s)
> + 	   (+ (frame-char-height) s)))
> +     (window-text-height)))
>   \f
>   (put 'term-mode 'mode-class 'special)

[...]

>   (defun term-check-size (process)
> !   (when (or (/= term-height (1- (window-height)))
>   	    (/= term-width (term-window-width)))
> !     (term-reset-size (1- (window-height)) (term-window-width))
>       (set-process-window-size process term-height term-width)))
>   
>   (defun term-send-raw-string (chars)
> --- 1193,1201 ----
>       found))
>   
>   (defun term-check-size (process)
> !   (when (or (/= term-height (term-window-height))
>   	    (/= term-width (term-window-width)))
> !     (term-reset-size (term-window-height) (term-window-width))
>       (set-process-window-size process term-height term-width)))
>   
>   (defun term-send-raw-string (chars)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#5615: 23.1.92; [PATCH] term.el: Calculation of window height is bad (new patch)

[npostavs]

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

tags 5615 + patch
quit

Lars Ingebrigtsen <larsi@gnus.org> writes:

> IRIE Shinsuke <irieshinsuke@yahoo.co.jp> writes:
>
>> Sorry, the patch I sent with previous mail is bad, because the previous
>> patch was made without considering `line-spacing' specified by a
>> floating point number or frame-parameter.
>>
>> So I wrote the new patch. Please check it.
>
> [...]
>
> Is this still a problem in the current Emacs?

Current Emacs still uses the same (1- (window-height)) expression, but I
can't understand from the description when exactly this gives the wrong
result.  Furthermore, I don't see any justification which would explain
why the new proposed significantly more complicated computation is more
correct.  I think we should just use window-text-height.


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

From b22407f5fedea77f34ca1efb5469e368164f9084 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Thu, 10 Aug 2017 20:43:13 -0400
Subject: [PATCH v2] * lisp/term.el (term-mode): Use `window-text-height'
 (Bug#5615).

---
 lisp/term.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/term.el b/lisp/term.el
index 5eb7b3e8ed..12a37cafbe 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -1007,7 +1007,7 @@ term-mode
   (setq indent-tabs-mode nil)
   (setq buffer-display-table term-display-table)
   (set (make-local-variable 'term-home-marker) (copy-marker 0))
-  (set (make-local-variable 'term-height) (1- (window-height)))
+  (set (make-local-variable 'term-height) (window-text-height))
   (set (make-local-variable 'term-width) (window-max-chars-per-line))
   (set (make-local-variable 'term-last-input-start) (make-marker))
   (set (make-local-variable 'term-last-input-end) (make-marker))
-- 
2.11.1

bug#5615: 23.1.92; [PATCH] term.el: Calculation of window height is bad (new patch)

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Date: Thu, 10 Aug 2017 20:45:16 -0400
> Cc: IRIE Shinsuke <irieshinsuke@yahoo.co.jp>, 5615@debbugs.gnu.org
> 
> Current Emacs still uses the same (1- (window-height)) expression, but I
> can't understand from the description when exactly this gives the wrong
> result.  Furthermore, I don't see any justification which would explain
> why the new proposed significantly more complicated computation is more
> correct.  I think we should just use window-text-height.

I think you are right.  If no one objects in a few days, please push
your change.

Thanks.

bug#5615: 23.1.92; [PATCH] term.el: Calculation of window height is bad (new patch)

[npostavs]

tags 5615 fixed
close 5615 26.1
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> From: npostavs@users.sourceforge.net
>> Date: Thu, 10 Aug 2017 20:45:16 -0400
>> Cc: IRIE Shinsuke <irieshinsuke@yahoo.co.jp>, 5615@debbugs.gnu.org
>> 
>> Current Emacs still uses the same (1- (window-height)) expression, but I
>> can't understand from the description when exactly this gives the wrong
>> result.  Furthermore, I don't see any justification which would explain
>> why the new proposed significantly more complicated computation is more
>> correct.  I think we should just use window-text-height.
>
> I think you are right.  If no one objects in a few days, please push
> your change.

Pushed to master.

[1: dbd3a17cb0]: 2017-08-19 23:29:28 -0400
  * lisp/term.el (term-mode): Use `window-text-height' (Bug#5615).
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=dbd3a17cb068148bd49e288eb0b44ca7eb4a4e3c