Is this correct?

Is this correct?

[Tim X]

I'm currently on a big bug fix cycle. Some of the bugs I'm trying to fix
relate to menus and I've noticed some code which I'm not convinced is
"correct" based on the documentation. I'd like some
feedback from others as I suspect it is something I am missing or don't
understand. 

force-mode-line-update has the following documentation -

,----[ C-h f force-mode-line-update RET ]
| force-mode-line-update is a compiled Lisp function in `subr.el'.
| 
| (force-mode-line-update &optional ALL)
| 
| Force redisplay of the current buffer's mode line and header line.
| With optional non-nil ALL, force redisplay of all mode lines and
| header lines.  This function also forces recomputation of the
| menu bar menus and the frame title.
| 
| [back]
`----

OK, that seems straight-forward enough. However, the code is defined as 
(I've removed the doc string to make it more concise)

(defun force-mode-line-update (&optional all)
  (if all (with-current-buffer (other-buffer)))
  (set-buffer-modified-p (buffer-modified-p)))

The bit that doesn't look quite right to me is 

(if all (with-current-buffer (other-buffer)))

I'm assuming that by calling with-current-buffer, this will temporarily
make 'other-buffer' active and as a side effect, the mode-line etc will get
updated. No problem with that. However, the documentation states that if
the arguement ALL is non-NIL if will force update of ALL mode lines and
buffer header lines. I don't see how this can be. Consider if I have
multiple frames or more than two buffers in one frame, how would this
update ALL of these?

Looking at the second line

  (set-buffer-modified-p (buffer-modified-p))

the code that implements set-buffer-modified-p appears, as a side
effect, to update the mode-line etc of all visible buffers in all frames
(judging by comments in the code). i.e. 

,----
|   /* Set update_mode_lines only if buffer is displayed in some window.
|      Packages like jit-lock or lazy-lock preserve a buffer's modified
|      state by recording/restoring the state around blocks of code.
|      Setting update_mode_lines makes redisplay consider all windows
|      (on all frames).  Stealth fontification of buffers not displayed
|      would incur additional redisplay costs if we'd set
|      update_modes_lines unconditionally.
`----

OK, now if that is the case, I'm now very confused. If
set-buffer-modified-p has the side effect of updating all visible
buffers in all frames, then what is the point of the previous line? In
fact, what is the point of having the ALL argument at all? Is this to
try and force update of non-visible buffers? If so, I don't see how it
would be all, but only the 'next' window, which may or may not be visible.

In summary, I don't understand how the ALL argument actually affects ALL
buffers - either only the next buffer is affected plus the last statement
seems to indicate that all visible buffers will be updated anyway. If I
was to call this function with no argument, I would expect, based on the
documentation, that only the current buffer mode-line would be updated,
but it seems that all visible buffers would be updated. If on the other
hand, I supplied a non-NIL argument, I would expect that ALL buffers
would be updated, but it seems only visible buffers and the buffer
returned by next-windows (which is what other-window uses to select the
next window) would be affected (together with all visible buffers).

I don't feel I understand or am clear on exactly what buffers would have
their mode-lines updated when I call the function either with a non-NIL
argument or without an argument. 

BACKGROUND
----------

The bug I'm trying to fix occurs with some code which updates the buffer
menu. There is a mode specific menu which only has mode specific menu
items. Under the help menu, is an item to "Switch to the Emacs Menubar".
When selected, it replaces the mode specific menu with the standard
Emacs menu and adds a new menu with one item that allows the user to
select the Mode specific menu again. 

The problem is that when you first select the menu option to restore the
emacs menu, initially, nothing happens. Some other event needs to occur
(either move the cursor, switch frames, click with the mouse etc) before
the new menu is displayed. I suspect it is some sort of update problem.
My first attempt to fix was to add a call to run the update-menu=-hooks,
but that doesn't seem to do anything. There is a call to a function
which essentially calls update-mode-line, which I suspect originally was
supposed to force redisplay of the menu bar for precisely this issue,
but it doesn't seem to work.

As a consequence, I want to understand update-mode-line in order to work
out what might be the issue here. It is likely it has nothing to do with
this function, but until I understand how it works, I cannot eliminate
it as a possible candidate. 

Tim

-- 
tcross (at) rapttech dot com dot au

Re: Is this correct?

[Kevin Rodgers]

On 2/25/11 11:54 PM, Tim X wrote:
>
> I'm currently on a big bug fix cycle. Some of the bugs I'm trying to fix
> relate to menus and I've noticed some code which I'm not convinced is
> "correct" based on the documentation. I'd like some
> feedback from others as I suspect it is something I am missing or don't
> understand.
>
> force-mode-line-update has the following documentation -
>
> ,----[ C-h f force-mode-line-update RET ]
> | force-mode-line-update is a compiled Lisp function in `subr.el'.
> |
> | (force-mode-line-update&optional ALL)
> |
> | Force redisplay of the current buffer's mode line and header line.
> | With optional non-nil ALL, force redisplay of all mode lines and
> | header lines.  This function also forces recomputation of the
> | menu bar menus and the frame title.
> |
> | [back]
> `----

Before digging into the details of whether the code implements the behavior
described in the doc string, do we understand the doc string?

* What if the current buffer is not displayed?

* Does ALL mean all buffers?  All windows?  All visible windows?  All frames?
   All visible frames?  Or some other interpretation?

* The menu bar and frame title are frame-local, right?  If so, I think that
   would imply that force-mode-line-update should by default (when ALL is nil)
   update all displayed buffers in the current frame, and should update all
   displayed buffers on all frames (when ALL is non-nil).

-- 
Kevin Rodgers
Denver, Colorado, USA

Re: Is this correct?

[Tim X]

Kevin Rodgers <kevin.d.rodgers@gmail.com> writes:

> On 2/25/11 11:54 PM, Tim X wrote:
>>
>> I'm currently on a big bug fix cycle. Some of the bugs I'm trying to fix
>> relate to menus and I've noticed some code which I'm not convinced is
>> "correct" based on the documentation. I'd like some
>> feedback from others as I suspect it is something I am missing or don't
>> understand.
>>
>> force-mode-line-update has the following documentation -
>>
>> ,----[ C-h f force-mode-line-update RET ]
>> | force-mode-line-update is a compiled Lisp function in `subr.el'.
>> |
>> | (force-mode-line-update&optional ALL)
>> |
>> | Force redisplay of the current buffer's mode line and header line.
>> | With optional non-nil ALL, force redisplay of all mode lines and
>> | header lines.  This function also forces recomputation of the
>> | menu bar menus and the frame title.
>> |
>> | [back]
>> `----
>
> Before digging into the details of whether the code implements the behavior
> described in the doc string, do we understand the doc string?
>
> * What if the current buffer is not displayed?
>

It isn't very clear. My assumption would be that mode-line, menu and
header lines are irrelevant for buffers that are not displayed, so would
assume if the current buffer is not displayed, do nothing?
 
> * Does ALL mean all buffers?  All windows?  All visible windows?  All frames?
>   All visible frames?  Or some other interpretation?
>

Yes, its not clear to me. I have been assuming (without much
justification BTW) that ALL would mean all buffers displayed in all
frames (including frames that may be on 'other' virtual desktops (and
therefore not visible) and/or frames that may be under the current
frame. I'm not even sure if 'visible' means frames that are on other
desktops (or consoles with emacsclient etc). 

> * The menu bar and frame title are frame-local, right?  If so, I think that
>   would imply that force-mode-line-update should by default (when ALL is nil)
>   update all displayed buffers in the current frame, and should update all
>   displayed buffers on all frames (when ALL is non-nil).

That would seem like a reasonable interpretation. Would those other
frames be considered visible (I've always thought so).

Tim

-- 
tcross (at) rapttech dot com dot au

Re: Is this correct?

[Eli Zaretskii]

> From: Tim X <timx@nospam.dev.null>
> Date: Sat, 26 Feb 2011 19:59:39 +1100
> 
> Kevin Rodgers <kevin.d.rodgers@gmail.com> writes:
> 
> >> ,----[ C-h f force-mode-line-update RET ]
> >> | force-mode-line-update is a compiled Lisp function in `subr.el'.
> >> |
> >> | (force-mode-line-update&optional ALL)
> >> |
> >> | Force redisplay of the current buffer's mode line and header line.
> >> | With optional non-nil ALL, force redisplay of all mode lines and
> >> | header lines.  This function also forces recomputation of the
> >> | menu bar menus and the frame title.
> >> |
> >> | [back]
> >> `----
> >
> > Before digging into the details of whether the code implements the behavior
> > described in the doc string, do we understand the doc string?
> >
> > * What if the current buffer is not displayed?
> >
> 
> It isn't very clear. My assumption would be that mode-line, menu and
> header lines are irrelevant for buffers that are not displayed, so would
> assume if the current buffer is not displayed, do nothing?

Why is this an issue?  A buffer that is not displayed will have its
mode line not displayed as well.  Whether Emacs wastes CPU cycles
recomputing a mode line that isn't displayed or not should be of no
consequence to the Lisp programmer.

Or are we talking about some subtle magic that runs off mode-line
updates, which wants to be run even in buffers that are not displayed?

> > * Does ALL mean all buffers?  All windows?  All visible windows?  All frames?
> >   All visible frames?  Or some other interpretation?
> >
> 
> Yes, its not clear to me.

All buffers are considered, but only those that are displayed will
have any effect on the screen.

> > * The menu bar and frame title are frame-local, right?  If so, I think that
> >   would imply that force-mode-line-update should by default (when ALL is nil)
> >   update all displayed buffers in the current frame, and should update all
> >   displayed buffers on all frames (when ALL is non-nil).
> 
> That would seem like a reasonable interpretation. Would those other
> frames be considered visible (I've always thought so).

The menu bar is computed in the context of the current buffer, so it's
not really frame-local.

I'm not sure what does this have to do with the issue at hand, though.

Re: Is this correct?

[Uday Reddy]

On 2/26/2011 6:54 AM, Tim X wrote:

>
> (defun force-mode-line-update (&optional all)
>    (if all (with-current-buffer (other-buffer)))
>    (set-buffer-modified-p (buffer-modified-p)))
>
> The bit that doesn't look quite right to me is
>
> (if all (with-current-buffer (other-buffer)))
>
> I'm assuming that by calling with-current-buffer, this will temporarily
> make 'other-buffer' active and as a side effect, the mode-line etc will get
> updated.

That would be quite magical if it were true.  My guess is that the 
parentheses have been misplaced.  It should have been

     (if all (with-current-buffer (other-buffer)
                 (set-buffer-modified-p (buffer-modified-p)))
     (set-buffer-modified-p (buffer-modified-p))

This doesn't make it right of course.  The "other-buffer" doesn't equate 
to "all" buffers, and pretty often, the "other-buffer" is an invisible 
buffer.  It is not guaranteed that the current-buffer is a visible 
buffer either.  So, this code would seem to be wrong at many levels!

Cheers,
Uday

Re: Is this correct?

[Eli Zaretskii]

> Date: Sat, 26 Feb 2011 12:44:15 +0000
> From: Uday Reddy <uDOTsDOTreddy@cs.bham.ac.uk>
> 
> On 2/26/2011 6:54 AM, Tim X wrote:
> 
> >
> > (defun force-mode-line-update (&optional all)
> >    (if all (with-current-buffer (other-buffer)))
> >    (set-buffer-modified-p (buffer-modified-p)))
> >
> > The bit that doesn't look quite right to me is
> >
> > (if all (with-current-buffer (other-buffer)))
> >
> > I'm assuming that by calling with-current-buffer, this will temporarily
> > make 'other-buffer' active and as a side effect, the mode-line etc will get
> > updated.
> 
> That would be quite magical if it were true.

??

> My guess is that the parentheses have been misplaced.

????? Doesn't force-mode-line-update work for you?

> It should have been
> 
>      (if all (with-current-buffer (other-buffer)
>                  (set-buffer-modified-p (buffer-modified-p)))
>      (set-buffer-modified-p (buffer-modified-p))
> 
> This doesn't make it right of course.  The "other-buffer" doesn't equate 
> to "all" buffers, and pretty often, the "other-buffer" is an invisible 
> buffer.  It is not guaranteed that the current-buffer is a visible 
> buffer either.  So, this code would seem to be wrong at many levels!

I gets better: `(set-buffer-modified-p (buffer-modified-p))'
apparently does nothing, because it sets the buffer's modified flag to
the same value it has already.  So this function actually does nothing
at all, right?

Re: Is this correct?

[Uday Reddy]

On 2/26/2011 2:07 PM, Eli Zaretskii wrote:

>> My guess is that the parentheses have been misplaced.
>
> ????? Doesn't force-mode-line-update work for you?

No, it isn't working.  We had an outstanding bug report in VM and Tim 
has finally tracked it down to this problem.

 
http://groups.google.com/group/gnu.emacs.vm.info/browse_thread/thread/20a83d2fd969386c/b81b15ca3baa29d2?lnk=gst&q=menu+bar#b81b15ca3baa29d2

    https://bugs.launchpad.net/vm/+bug/605799

VM is calling force-mode-line-update in a Folder buffer, which is 
generally invisible.  The function is updating its menubar, but not that 
of the currently visible buffer.  Even after I change it the way I 
mentioned in my previous post, it still doesn't work because the 
"other-buffer" also turns out to be the Folder buffer for some reason or 
the other.

> I gets better: `(set-buffer-modified-p (buffer-modified-p))'
> apparently does nothing, because it sets the buffer's modified flag to
> the same value it has already.  So this function actually does nothing
> at all, right?

That would be logical, but I am guessing from force-mode-line-update's 
doc string that this has the effect of updating the mode line, menu bar 
and the rest.

It is a pity that set-buffer-modified-p's doc string doesn't do justice 
to all that it does!

Cheers,
Uday

Re: Is this correct?

[Eli Zaretskii]

> Date: Sat, 26 Feb 2011 15:38:26 +0000
> From: Uday Reddy <uDOTsDOTreddy@cs.bham.ac.uk>
> 
> On 2/26/2011 2:07 PM, Eli Zaretskii wrote:
> 
> >> My guess is that the parentheses have been misplaced.
> >
> > ????? Doesn't force-mode-line-update work for you?
> 
> No, it isn't working.  We had an outstanding bug report in VM and Tim 
> has finally tracked it down to this problem.

Does it fail to work at all, in all circumstances, or just in this
special case?  If the latter, then your arguments seem to be
inaccurate, because they seem to say that it should never work.

Re: Is this correct?

[Uday Reddy]

On 2/26/2011 3:47 PM, Eli Zaretskii wrote:

> Does it fail to work at all, in all circumstances, or just in this
> special case?  If the latter, then your arguments seem to be
> inaccurate, because they seem to say that it should never work.

By "this special case", you mean the case when the other-buffer happens 
to be the same as the current-buffer?  Yeah, I suppose that is an odd 
special case.  You are saying that it works fine otherwise?

Cheers,
Uday

Re: Is this correct?

[Eli Zaretskii]

> Date: Sat, 26 Feb 2011 16:22:01 +0000
> From: Uday Reddy <uDOTsDOTreddy@cs.bham.ac.uk>
> 
> You are saying that it works fine otherwise?

I'm asking _you_ if it does.

Re: Is this correct?

[Tim X]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sat, 26 Feb 2011 12:44:15 +0000
>> From: Uday Reddy <uDOTsDOTreddy@cs.bham.ac.uk>
>> 
>> On 2/26/2011 6:54 AM, Tim X wrote:
>> 
>> >
>> > (defun force-mode-line-update (&optional all)
>> >    (if all (with-current-buffer (other-buffer)))
>> >    (set-buffer-modified-p (buffer-modified-p)))
>> >
>> > The bit that doesn't look quite right to me is
>> >
>> > (if all (with-current-buffer (other-buffer)))
>> >
>> > I'm assuming that by calling with-current-buffer, this will temporarily
>> > make 'other-buffer' active and as a side effect, the mode-line etc will get
>> > updated.
>> 
>> That would be quite magical if it were true.
>
> ??
>
>> My guess is that the parentheses have been misplaced.
>
> ????? Doesn't force-mode-line-update work for you?
>
>> It should have been
>> 
>>      (if all (with-current-buffer (other-buffer)
>>                  (set-buffer-modified-p (buffer-modified-p)))
>>      (set-buffer-modified-p (buffer-modified-p))
>> 
>> This doesn't make it right of course.  The "other-buffer" doesn't equate 
>> to "all" buffers, and pretty often, the "other-buffer" is an invisible 
>> buffer.  It is not guaranteed that the current-buffer is a visible 
>> buffer either.  So, this code would seem to be wrong at many levels!
>
> I gets better: `(set-buffer-modified-p (buffer-modified-p))'
> apparently does nothing, because it sets the buffer's modified flag to
> the same value it has already.  So this function actually does nothing
> at all, right?
>

I looked at the C code that implements set-buffer-modified-p at it seems
to me (who freely admits not understanding the low C level of emacs!)
that as a side effect, the function updates mode-lines etc regardless of
the setting/value for buffer-modified-p. Therefore, my interpretation
was that this would call the function without actually changing the
state of the modified flag and by side effect, result in an update of
the mode-line. 

I do agree it doesn't look right and personally don't like code which
does something via a non-intuitive/obvious side effect. While possibly
convenient, from a maintenance standpoint, it would be better if it was
more deliberate/obvious IMO.

Tim

-- 
tcross (at) rapttech dot com dot au

Re: Is this correct?

[Tim X]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Tim X <timx@nospam.dev.null>
>> Date: Sat, 26 Feb 2011 19:59:39 +1100
>> 
>> Kevin Rodgers <kevin.d.rodgers@gmail.com> writes:
>> 
>> >> ,----[ C-h f force-mode-line-update RET ]
>> >> | force-mode-line-update is a compiled Lisp function in `subr.el'.
>> >> |
>> >> | (force-mode-line-update&optional ALL)
>> >> |
>> >> | Force redisplay of the current buffer's mode line and header line.
>> >> | With optional non-nil ALL, force redisplay of all mode lines and
>> >> | header lines.  This function also forces recomputation of the
>> >> | menu bar menus and the frame title.
>> >> |
>> >> | [back]
>> >> `----
>> >
>> > Before digging into the details of whether the code implements the behavior
>> > described in the doc string, do we understand the doc string?
>> >
>> > * What if the current buffer is not displayed?
>> >
>> 
>> It isn't very clear. My assumption would be that mode-line, menu and
>> header lines are irrelevant for buffers that are not displayed, so would
>> assume if the current buffer is not displayed, do nothing?
>
> Why is this an issue?  A buffer that is not displayed will have its
> mode line not displayed as well.  Whether Emacs wastes CPU cycles
> recomputing a mode line that isn't displayed or not should be of no
> consequence to the Lisp programmer.
>
> Or are we talking about some subtle magic that runs off mode-line
> updates, which wants to be run even in buffers that are not displayed?
>
>> > * Does ALL mean all buffers?  All windows?  All visible windows?  All frames?
>> >   All visible frames?  Or some other interpretation?
>> >
>> 
>> Yes, its not clear to me.
>
> All buffers are considered, but only those that are displayed will
> have any effect on the screen.
>
>> > * The menu bar and frame title are frame-local, right?  If so, I think that
>> >   would imply that force-mode-line-update should by default (when ALL is nil)
>> >   update all displayed buffers in the current frame, and should update all
>> >   displayed buffers on all frames (when ALL is non-nil).
>> 
>> That would seem like a reasonable interpretation. Would those other
>> frames be considered visible (I've always thought so).
>
> The menu bar is computed in the context of the current buffer, so it's
> not really frame-local.
>
> I'm not sure what does this have to do with the issue at hand, though.
>
Hi Eli,

thanks for response. I think, form the other thread where you have been
discussing this with Uday, this is probably clearer now. Just for the
record ....

We have an issue with a package that modifies the menu. After chaning
the menu, it calls force-mode-line-update. However, this is not actually
working to update the display of the menu-bar. Once the menu-bar has
been modified, some other event must occur before the menu-bar is
updated. That even could be another mouse click, movement of the cursor,
switching frames or even just waiting a minute or so (I'm guessing that
in that time, some other event triggers the update, such as updating the
load display in the mode-line, gc or something else). 

To try and track this down, I was trying to understand exactly how
force-mode-line-update works and what pre-conditions it may have re:
visible/current buffers etc. So, my questions were supposed to help
clarify my understanding of both how force-update-mode-line was supposed
to work and how it actually works.

Tim

-- 
tcross (at) rapttech dot com dot au

Re: Is this correct?

[Eli Zaretskii]

> From: Tim X <timx@nospam.dev.null>
> Date: Sun, 27 Feb 2011 09:38:14 +1100
> 
> > I gets better: `(set-buffer-modified-p (buffer-modified-p))'
> > apparently does nothing, because it sets the buffer's modified flag to
> > the same value it has already.  So this function actually does nothing
> > at all, right?
> >
> 
> I looked at the C code that implements set-buffer-modified-p at it seems
> to me (who freely admits not understanding the low C level of emacs!)
> that as a side effect, the function updates mode-lines etc regardless of
> the setting/value for buffer-modified-p. Therefore, my interpretation
> was that this would call the function without actually changing the
> state of the modified flag and by side effect, result in an update of
> the mode-line. 

Yes.  My text was ironic: it was supposed to hint that there _is_ some
magic here.

> I do agree it doesn't look right and personally don't like code which
> does something via a non-intuitive/obvious side effect. While possibly
> convenient, from a maintenance standpoint, it would be better if it was
> more deliberate/obvious IMO.

Nothing about the Emacs redisplay is intuitive or obvious.

Re: Is this correct?

[Eli Zaretskii]

> From: Tim X <timx@nospam.dev.null>
> Date: Sun, 27 Feb 2011 09:48:24 +1100
> 
> We have an issue with a package that modifies the menu. After chaning
> the menu, it calls force-mode-line-update. However, this is not actually
> working to update the display of the menu-bar. Once the menu-bar has
> been modified, some other event must occur before the menu-bar is
> updated. That even could be another mouse click, movement of the cursor,
> switching frames or even just waiting a minute or so (I'm guessing that
> in that time, some other event triggers the update, such as updating the
> load display in the mode-line, gc or something else). 

If you can show a small reproducible test case for this problem, I
could try looking into it.

Btw, this discussion now moved to emacs-devel (which is where it
should have been in the first place, IMO).

Re: Is this correct?

[Tim X]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Tim X <timx@nospam.dev.null>
>> Date: Sun, 27 Feb 2011 09:48:24 +1100
>> 
>> We have an issue with a package that modifies the menu. After chaning
>> the menu, it calls force-mode-line-update. However, this is not actually
>> working to update the display of the menu-bar. Once the menu-bar has
>> been modified, some other event must occur before the menu-bar is
>> updated. That even could be another mouse click, movement of the cursor,
>> switching frames or even just waiting a minute or so (I'm guessing that
>> in that time, some other event triggers the update, such as updating the
>> load display in the mode-line, gc or something else). 
>
> If you can show a small reproducible test case for this problem, I
> could try looking into it.
>

It is hard to reproduce in a small recipe. I've even noticed that
while the problem occurs most of the time, now and again, it works as it
should. 

> Btw, this discussion now moved to emacs-devel (which is where it
> should have been in the first place, IMO).
>

I didn't put it on emacs-dev initially as I wanted to be sure it wasn't
just something I misunderstood before adding to the traffic on that
list. I stated this in the original post. 

Uday has progressed things further and moved it to the dev list. He has
a better understanding of the internals of the package where we have
encountered the problem, so it is probably best left in his hands rather
than have me muddy the waters with ignorance. 

Tim
-- 
tcross (at) rapttech dot com dot au