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

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

IRIE Shinsuke
When I use term-mode (ansi-term), the bottom row of the terminal
often disappears from the window. For example, if I execute Emacs in
ansi-term with -nw option, the minibuffer is unusable because it becomes
completely hidden. I investigated this problem and found the solution.

In term.el, the number of rows of the terminal window is calculated as:

(1- (window-height))

This expression, however, doesn't always return exact value from lack
of consideration to the header line height and the spacing of each row.
So I tested the better expression instead:

(if (display-graphic-p)
    (let ((e (window-inside-pixel-edges))
          (s (or line-spacing 0)))
      (/ (+ (- (nth 3 e) (cadr e)) s)
         (+ (frame-char-height) s)))
  (window-text-height))

and it seems to work fine. We can test this expression by evaluating
the following code before starting ansi-term:

(add-hook 'term-mode-hook
          (lambda ()
            (fset 'term-window-height
                  #'(lambda ()
                      (if (display-graphic-p)
                          (let ((e (window-inside-pixel-edges))
                                (s (or line-spacing 0)))
                            (/ (+ (- (nth 3 e) (cadr e)) s)
                               (+ (frame-char-height) s)))
                        (window-text-height))))
            (fset 'term-check-size
                  #'(lambda (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))))
            (setq term-height (term-window-height))))

or applying the patch I attached.

term-window-height.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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





Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

IRIE Shinsuke
In reply to this post by 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


term-window-height-newpatch.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

IRIE Shinsuke
In reply to this post by IRIE Shinsuke
Oops, I missed attaching the patch to the mail... Sorry.


IRIE Shinsuke





Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 <[hidden email]>:

> Oops, I missed attaching the patch to the mail... Sorry.
>
>
> IRIE Shinsuke
>
>
>
>
>
>



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Lars Ingebrigtsen
In reply to this post by IRIE Shinsuke
IRIE Shinsuke <[hidden email]> 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)))
>  
>   (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



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Noam Postavsky-2
tags 5615 + patch
quit

Lars Ingebrigtsen <[hidden email]> writes:

> IRIE Shinsuke <[hidden email]> 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.


From b22407f5fedea77f34ca1efb5469e368164f9084 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <[hidden email]>
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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Eli Zaretskii
> From: [hidden email]
> Date: Thu, 10 Aug 2017 20:45:16 -0400
> Cc: IRIE Shinsuke <[hidden email]>, [hidden email]
>
> 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.



Loading...