GNU bug report logs - #23568
25.0.94; Mode line menus appear incorrectly in some monitor configurations

Previous Next

Full log

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex <agrambot <at> gmail.com>
Cc: rudalics <at> gmx.at, 23568 <at> debbugs.gnu.org
Subject: Re: bug#23568: 25.0.94;
 Mode line menus appear incorrectly in some monitor configurations
Date: Sun, 04 Jun 2017 17:28:48 +0300

> From: Alex <agrambot <at> gmail.com>
> Cc: rudalics <at> gmx.at,  23568 <at> debbugs.gnu.org
> Date: Sat, 03 Jun 2017 13:19:50 -0600
> 
> > If this is supposed to be used as part of mode-line display, then no.
> > Signaling errors in the middle of redisplay is generally a bad idea,
> > because they cause another redisplay cycle, which again signals an
> > error, and Emacs just freezes.
> 
> I see. In the general case, is it recommended to silently fallback to a
> different method, or should there be a log message?
> 
> I changed the procedure so that it only uses the coordinates if they're
> numbers.

It depends.  For Lisp code that we write, it is indeed better to avoid
the problem altogether.  For user-defined Lisp, about which we can
make no assumptions, there should be a log message in case of errors.

> >> +(defun display-monitor-attribute (attribute &optional display x y)
> >> +  "Return the value of the ATTRIBUTE of the current monitor.
> >
> > The doc string should say something about what "the current monitor"
> > means, or have a link to where that is explained.
> 
> I expanded that section, copying a chunk of it from
> `frame-monitor-attributes'.
> 
> >> +DISPLAY can be a display name, a terminal name, or a frame.
> >
> > "Terminal name" or "terminal object"?
> 
> `display-monitor-attributes' can take both, which is where I copied that
> line from.
> 
> I don't think I should have done that, actually. I don't believe this
> function makes sense for a terminal/display input, as a
> terminal/display, as I understand it, can have multiple physical
> monitors.
> 
> I changed all instances of `display' to `frame'. I'm still not sure on
> the name, though. Perhaps a better prefix would be `physical-monitor-*',
> or just `monitor-*'?
> 
> What do you think?

LGTM, thanks.

> > Why is this in xmenu.c?  Is the problem unique to X window system?
> 
> The xmenu.c part contains the actual bugfix, while the frame.el part is
> just a partial abstraction for the fix so that other procedures can use
> it (such as compute_tip_xy in xfns.c).
> 
> The procedure I'm editing (menu_position_func) is inside an #ifdef
> USE_GTK, and uses x_display_pixel_{height,width} currently. I'm not sure
> if it's useful outside of X currently.
> 
> I haven't tested the issue on other systems, but I don't think it's an
> issue in Windows as half of the code came from Windows code according to
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=22549#61.

Ah, okay.  Then maybe just add a comment there saying that the w32
case is handled elsewhere, perhaps with a pointer to the function
which does that.

> Here's an updated diff:

Thanks, it would be nice if you could also provide a ChangeLog-style
commit log message.  That'd make the job of pushing the changes much
easier.

Previous Next

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