GNU bug report logs - #63825
29.0.90; The header line should be hidden when empty

Previous Next

Full log

Message #29 received at 63825 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: sbaugh <at> janestreet.com, 63825 <at> debbugs.gnu.org
Subject: Re: bug#63825: 29.0.90; The header line should be hidden when empty
Date: Fri, 02 Jun 2023 14:36:16 +0300

> From: Eshel Yaron <me <at> eshelyaron.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  63825 <at> debbugs.gnu.org
> Date: Fri, 02 Jun 2023 09:19:17 +0300
> 
> This might be a bit too coarse IMO, because it would make it a lot
> harder to create an empty header line.  Namely, setting
> `header-line-format` to an empty string would no longer create an empty
> header line.

Yes.

> I'm far from fluent in Emacs's internals, but AFAIU from skimming
> src/xdisp.c there's another issue which is that Emacs checks whether a
> window should have a header line in many circumstances, as it affects
> the window's effective dimensions.  So formatting the entire header line
> each time Emacs just wants to know whether there is or isn't a header
> line might not be ideal in terms of efficiency.

Right.

> The way I read Eli's above message, it boils down to extending the C
> function `window_wants_mode_line` with a couple of special cases.
> Something like the following (mildly tested):

Thanks, this is an elegant solution.  A few minor comments:

> +/**
> + * null_header_line_format:
> + *
> + * Return 1 when header line format F indicates that the header line
> + * should not be displayed at all.

We usually say "Return non-zero", not 1.

> + * This is when F is nil, or F is a cons cell and either its car is a
> + * symbol whose value as a variable is nil, or its car is the symbol
> + * ':eval' and its cddr evaluates to nil.
> + */
> +static bool
> +null_header_line_format (Lisp_Object f)

The argument should be called fmt or somesuch.  When I see 'f' in the
C sources, I assume it's a pointe to 'struct frame'.

> +  if (NILP (f))
> +    return 1;
       ^^^^^^^^^
This should say "return true;" instead, since the function is declared
as returning a 'bool'.

> +  if (CONSP (f)) {

This is not our style: we put the opening and closing braces on lines
of their own; see the rest of the code around.

> +    car = XCAR (f);
> +    return (SYMBOLP (car)
> +	    && ((EQ (car, QCeval)
> +		 && NILP (Feval (XCAR (XCDR (f)), Qnil)))
> +		|| NILP (find_symbol_value (car))));

find_symbol_value can return Qunbound, which is non-nil, but this
function should treat it as if it were nil (see the documentation of
mode-line-format).

> +  return 0;

"return false;"

> @@ -5495,8 +5525,8 @@ 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)))

This line is too long, and should be broken, probably before
header_line_format.

And finally, please accompany the change with a suitable commit log
message, an entry in NEWS, and a change for the ELisp manual.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.