From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#63825: 29.0.90; The header line should be hidden when empty Date: Sun, 04 Jun 2023 10:03:33 +0300 Message-ID: <838rcz7lq2.fsf@gnu.org> References: <838rd3cg7u.fsf@gnu.org> <83353bc97b.fsf@gnu.org> <83mt1iayfj.fsf@gnu.org> <83jzwl9k1h.fsf@gnu.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="18401"; mail-complaints-to="usenet@ciao.gmane.io" Cc: sbaugh@janestreet.com, 63825@debbugs.gnu.org To: Eshel Yaron Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Jun 04 09:03:30 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1q5hly-0004YQ-C7 for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 04 Jun 2023 09:03:30 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1q5hla-0002yP-AD; Sun, 04 Jun 2023 03:03:06 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1q5hlX-0002yC-7U for bug-gnu-emacs@gnu.org; Sun, 04 Jun 2023 03:03:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1q5hlW-0000kB-56 for bug-gnu-emacs@gnu.org; Sun, 04 Jun 2023 03:03:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1q5hlW-0005aO-18 for bug-gnu-emacs@gnu.org; Sun, 04 Jun 2023 03:03:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 04 Jun 2023 07:03:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 63825 X-GNU-PR-Package: emacs Original-Received: via spool by 63825-submit@debbugs.gnu.org id=B63825.168586217221454 (code B ref 63825); Sun, 04 Jun 2023 07:03:01 +0000 Original-Received: (at 63825) by debbugs.gnu.org; 4 Jun 2023 07:02:52 +0000 Original-Received: from localhost ([127.0.0.1]:44787 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1q5hlL-0005Zy-Ct for submit@debbugs.gnu.org; Sun, 04 Jun 2023 03:02:51 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:59392) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1q5hlI-0005Zi-Aj for 63825@debbugs.gnu.org; Sun, 04 Jun 2023 03:02:50 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1q5hlC-0000hT-RH; Sun, 04 Jun 2023 03:02:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=oOeiazGxj48iUbXEWc+DUUuDlC0lutGYf52ka4oTT0U=; b=hFxMjIEowFG0 FpehQrh0AZ3rDK96dZdQb6WmNUshWVuocNZr61/gIDOF9r7r9uSwWk8aX37/8Lo0yb2U6cUXUsE0b crB6crrUhekJw3lXPngS81/C2xCw2G1Kzzssh4rr+3iJGn4KNvmt3G++BYGqgn7fnsjxoIe2dbCwm dYE3msn6fG92W7XPWwp32aeXUAK+h0PpHWETaesVdgMn8chRT4a/GPWC8oJNoFEWYymnAjBviFANq 8tzpLInzpHFNjCrhiOw8UgwAoJjDjKkrqdNt2li9nU+Q5HAZtxpcPI3CJBwUBJwj6vDOQLgJuUDkR 2CUc/X+R7FOLQHNslLoEsg==; Original-Received: from [87.69.77.57] (helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1q5hlB-0007UY-9f; Sun, 04 Jun 2023 03:02:42 -0400 In-Reply-To: (message from Eshel Yaron on Sat, 03 Jun 2023 10:28:20 +0300) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:262902 Archived-At: > From: Eshel Yaron > Cc: sbaugh@janestreet.com, 63825@debbugs.gnu.org > Date: Sat, 03 Jun 2023 10:28:20 +0300 > > Eli Zaretskii writes: > > >> For example, @code{(format-mode-line header-line-format)} returns the > >> -text that would appear in the selected window's header line (@code{""} > >> -if it has no header line). @code{(format-mode-line header-line-format > >> -'header-line)} returns the same text, with each character > >> -carrying the face that it will have in the header line itself, and also > >> -redraws the header line. > >> +text that would appear in the selected window's header line. > >> +@code{(format-mode-line header-line-format 'header-line)} returns the > >> +same text, with each character carrying the face that it will have in > >> +the header line itself, and also redraws the header line. > > > > I'm not sure why you removed the part about an empty string. There's > > no change in format-mode-line to justify that, AFAICT, and neither > > should there be. > > Indeed, my patch doesn't change `format-mode-line`. I removed this part > because AFAICT it's wrong: it suggests that if `format-mode-line` > returns the empty string with some argument, then using that argument as > the value of `header-line-format` will result in no header line at all. > But that's not the case (and it wasn't before my patch), because > `(format-mode-line header-line-format)` returning an empty string means > that the header line is either absent or empty, not necessarily absent. This is not what the text wants to convey. It wants to say that if a window has no header-line, i.e. header-line-format is nil, format-mode-line returns an empty string. This is true, before and after your changes. > >> + if (CONSP (fmt)) > >> + { > >> + car = XCAR (fmt); > >> + if (SYMBOLP (car)) > >> + { > >> + if (EQ (car, QCeval) > >> + && NILP (Feval (XCAR (XCDR (fmt)), Qnil))) > >> + return true; > > > > This should use safe__eval (or something similar), not Feval, because > > it is called as part of redisplay, where we cannot allow any errors to > > throw to top-level. > > Got it, here's an updated patch: Thanks. > + if (CONSP (fmt)) > + { > + car = XCAR (fmt); > + if (SYMBOLP (car)) > + { > + if (EQ (car, QCeval) > + && NILP (safe_eval_inhibit_quit (XCAR (XCDR (fmt))))) > + return true; The indentation of "return true;" seems incorrect here. Are you using a non-default setup of C indentation levels? > @@ -5495,8 +5534,9 @@ window_wants_header_line (struct window *w) > && !MINI_WINDOW_P (w) > && !WINDOW_PSEUDO_P (w) > && !EQ (window_header_line_format, Qnone) > - && (!NILP (window_header_line_format) > - || !NILP (BVAR (XBUFFER (WINDOW_BUFFER (w)), header_line_format))) > + && (!null_header_line_format (window_header_line_format) > + || !null_header_line_format (BVAR (XBUFFER (WINDOW_BUFFER (w)), > + header_line_format))) > && (WINDOW_PIXEL_HEIGHT (w) > > (window_wants_mode_line (w) > ? 2 * WINDOW_FRAME_LINE_HEIGHT (w) One more issue (sorry I didn't notice it before): the :eval form can potentially delete the window's frame. See this part of display_mode_element: if (CONSP (XCDR (elt))) { Lisp_Object spec; spec = safe__eval (true, XCAR (XCDR (elt))); /* The :eval form could delete the frame stored in the iterator, which will cause a crash if we try to access faces and other fields (e.g., FRAME_KBOARD) on that frame. This is a nonsensical thing to do, and signaling an error from redisplay might be dangerous, but we cannot continue with an invalid frame. */ if (!FRAME_LIVE_P (it->f)) signal_error (":eval deleted the frame being displayed", elt); n += display_mode_element (it, depth, field_width - n, precision - n, spec, props, risky); } So I think we need to add to null_header_line_format a test for the window's frame to be live, to be on the safe side. For that to work, the function should accept an additional argument: a pointer to the frame of the window whose header-line we are considering. > @@ -27408,7 +27414,7 @@ display_mode_element (struct it *it, int depth, int field_width, int precision, > if (CONSP (XCDR (elt))) > { > Lisp_Object spec; > - spec = safe__eval (true, XCAR (XCDR (elt))); > + spec = safe_eval_inhibit_quit (XCAR (XCDR (elt))); > /* The :eval form could delete the frame stored in the > iterator, which will cause a crash if we try to > access faces and other fields (e.g., FRAME_KBOARD) I see no reason to make this replacement. Calling a static function lets the compiler optimize more aggressively than calling an non-static one. Thanks.