From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Window tree and window's internal height Date: Sun, 18 Nov 2018 17:53:42 +0200 Message-ID: <83bm6mtm9l.fsf@gnu.org> References: <83o9anuahg.fsf@gnu.org> <5BF06068.60103@gmx.at> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: blaine.gmane.org 1542556305 21403 195.159.176.226 (18 Nov 2018 15:51:45 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 18 Nov 2018 15:51:45 +0000 (UTC) Cc: emacs-devel@gnu.org To: martin rudalics Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Nov 18 16:51:41 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gOPMK-0005Tn-Hb for ged-emacs-devel@m.gmane.org; Sun, 18 Nov 2018 16:51:41 +0100 Original-Received: from localhost ([::1]:52741 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gOPOQ-0002ug-P7 for ged-emacs-devel@m.gmane.org; Sun, 18 Nov 2018 10:53:50 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:38530) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gOPOK-0002uM-8R for emacs-devel@gnu.org; Sun, 18 Nov 2018 10:53:45 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gOPOG-0000Yj-8R for emacs-devel@gnu.org; Sun, 18 Nov 2018 10:53:44 -0500 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:57546) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gOPOG-0000Yf-5I; Sun, 18 Nov 2018 10:53:40 -0500 Original-Received: from [176.228.60.248] (port=3624 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1gOPOF-0004ob-Ry; Sun, 18 Nov 2018 10:53:40 -0500 In-reply-to: <5BF06068.60103@gmx.at> (message from martin rudalics on Sat, 17 Nov 2018 19:39:36 +0100) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e 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:231221 Archived-At: > Date: Sat, 17 Nov 2018 19:39:36 +0100 > From: martin rudalics > CC: emacs-devel@gnu.org > > > First, we have this in the ELisp manual: > > > > A minibuffer window (*note Minibuffer Windows::) is not part of its > > frame’s window tree unless the frame is a minibuffer-only frame. > > Nonetheless, most of the functions in this section accept the minibuffer > > window as an argument. Also, the function ‘window-tree’ described at > > the end of this section lists the minibuffer window alongside the actual > > window tree. > > > > The first sentence is incorrect, isn't it? Because Emacs disagrees: > > > > emacs -Q > > M-: (window-next-sibling) RET > > => # > > > > And if that sentence is incorrect, we don't need the next two > > "exceptions", right? > > Right, graph theoretically. For a long time, however, the Elisp manual > has used the term "root", for example, in Emacs 22 as follows > > The return value is a list of the form `(ROOT MINI)', where ROOT > represents the window tree of the frame's root window, and MINI is > the frame's minibuffer window. > > which implies that Emacs' window tree is not free but a "rooted tree" > and in a rooted tree every node except the root must have a unique > parent node. But the minibuffer window does not have a parent so we > would have two nodes without parent. So maybe we should simply say that the mini-window doesn't have a parent, instead of what we say now? > Which means that our window tree concept per se is valid but using > 'window-next-sibling' to get the minibuffer window from the root > window is an incorrect naming convention. It's simply there because > 'window-next-sibling' is a 1-to-1 reflection of the w->next field of > the window structure in window.h. Looking at w->next _was_ what tripped me in the first place: I didn't expect a sole window on a TTY frame to have a non-nil object pointed by its 'next' pointer. I don't think we should assume that the ELisp manual is only for people who work on the Lisp level. > > int > > window_internal_height (struct window *w) > > { > > int ht = w->total_lines; > > > > if (!MINI_WINDOW_P (w)) > > { > > if (!NILP (w->parent) > > || WINDOWP (w->contents) > > || !NILP (w->next) > > || !NILP (w->prev) > > || window_wants_mode_line (w)) > > --ht; > > > > if (window_wants_header_line (w)) > > --ht; > > } > > > > return ht; > > } > > > > I don't understand any of the conditions except window_wants_mode_line > > when we decide whether to decrease the height due to the mode line. > > What do the other conditions have to do with the window's height? I > > could perhaps understand the "WINDOWP (w->contents)" part, as some > > kind of optimization for non-leaf windows, which assumes the default > > of having the mode line, but the rest seem simply wrong, don't they? > > (The WINDOWP condition is not really interesting, as I don't see any > > call of this function for a non-leaf window.) > > Whatever these three conditions were meant for is beyond my > imagination. Well, I guess that answers my question: it's simply a very old bug. > window_internal_height was always off-limit for me. I > wonder how these miscalculations would affect window_scroll_line_based I don't think they do, because the result is either used as a limit to clip some value, or for computing half the window-full, which errs by half a line half of the time anyway (due to integer division). > (I hardly ever use modeline-less windows) though. Sounds like almost no one does, or we would have heard about bug#33363 and its ilk a long time ago. > > This is the immediate cause of bug#33363, which I could fix either by > > a local change in try_window_id, or by modifying > > window_internal_height to leave only the window_wants_mode_line > > condition there. If the latter might be unsafe, maybe I should do the > > former on the release branch and the latter on master. WDYT? > > I would call window_body_height on the release branch and on master. window_body_height attempts to calculate the mode-line and header-line heights in pixels, which might be expensive and if so entirely a waste of CPU cycles on TTY frames. But I guess this indirectly answers my question, because my proposed change in window_internal_height will make it the exact equivalent of window_body_height for TTY frames. So I think I will make that change on the release branch. > And I would use window_body_height in window_scroll_line_based too. > It should then take care of dividers and horizontal scroll bars on GUI > windows as well. window_scroll_line_based is never used on GUI frames, and dividers and scroll bars always have exactly zero size on TTY frames. Right? I see only one GUI client of window_internal_height: ns-win.el, where it calls compute-motion. But it calls that function in a way that doesn't trigger the call to window_internal_height, so I think we are safe. However, maybe ns-win.el should switch to using window-body-height instead. Thanks.