Re: Multiple runs of menu-bar-update-hook

[Richard Stallman]

The function update_menu_bar does various things, such as running
menu-bar-update-hook.  Then it calls set_frame_menubar, with DEEP_P =
0.  This is supposed to tell set_frame_menubar not to run
menu-bar-update-hook, and not to do various other things that would
be redundant.

However, set_frame_menubar sets DEEP_P to 1 in certain conditions.
I suspect there is a bug in that logic.  Could someone study it?
You could put a breakpoint inside set_frame_menubar and step thru
and see if DEEP_P gets set to 1, and if so, why.

Note that the case of multiple windows showing one buffer is inefficient
in various ways.  I made various optimizations which are limited
to the case of one window, because handling them in multiple windows
is harder.

The whole menu bar update code could use a complete rewrite.  It got
to be the way it is because I had to fix, over and over again, bugs in
code that I had not written and did not totally understand.

As for running menu-bar-update-hook multiple times when multiple
frames update their menu bars, I see there's no way for the hook
functions to tell which frame they are being called "for".  So there
is no possible use in running it once for each frame.

This patch should get rid of that.  Does it?


*** xdisp.c     17 Jul 2006 16:31:56 -0400      1.1111
--- xdisp.c     27 Jul 2006 16:55:47 -0400      
***************
*** 900,906 ****
  static Lisp_Object redisplay_window_error ();
  static Lisp_Object redisplay_window_0 P_ ((Lisp_Object));
  static Lisp_Object redisplay_window_1 P_ ((Lisp_Object));
! static void update_menu_bar P_ ((struct frame *, int));
  static int try_window_reusing_current_matrix P_ ((struct window *));
  static int try_window_id P_ ((struct window *));
  static int display_line P_ ((struct it *));
--- 900,906 ----
  static Lisp_Object redisplay_window_error ();
  static Lisp_Object redisplay_window_0 P_ ((Lisp_Object));
  static Lisp_Object redisplay_window_1 P_ ((Lisp_Object));
! static int update_menu_bar P_ ((struct frame *, int, int));
  static int try_window_reusing_current_matrix P_ ((struct window *));
  static int try_window_id P_ ((struct window *));
  static int display_line P_ ((struct it *));
***************
*** 9036,9041 ****
--- 9036,9042 ----
      {
        Lisp_Object tail, frame;
        int count = SPECPDL_INDEX ();
+       int menu_bar_hooks_run = 0;
  
        record_unwind_save_match_data ();
  
***************
*** 9067,9073 ****
            }
  
          GCPRO1 (tail);
!         update_menu_bar (f, 0);
  #ifdef HAVE_WINDOW_SYSTEM
          update_tool_bar (f, 0);
  #ifdef MAC_OS
--- 9068,9074 ----
            }
  
          GCPRO1 (tail);
!         menu_bar_hooks_run = update_menu_bar (f, 0, menu_bar_hooks_run);
  #ifdef HAVE_WINDOW_SYSTEM
          update_tool_bar (f, 0);
  #ifdef MAC_OS
***************
*** 9082,9088 ****
    else
      {
        struct frame *sf = SELECTED_FRAME ();
!       update_menu_bar (sf, 1);
  #ifdef HAVE_WINDOW_SYSTEM
        update_tool_bar (sf, 1);
  #ifdef MAC_OS
--- 9083,9089 ----
    else
      {
        struct frame *sf = SELECTED_FRAME ();
!       update_menu_bar (sf, 1, 0);
  #ifdef HAVE_WINDOW_SYSTEM
        update_tool_bar (sf, 1);
  #ifdef MAC_OS
***************
*** 9103,9114 ****
     before we start to fill in any display lines, because it can call
     eval.
  
!    If SAVE_MATCH_DATA is non-zero, we must save and restore it here.  */
  
! static void
! update_menu_bar (f, save_match_data)
       struct frame *f;
       int save_match_data;
  {
    Lisp_Object window;
    register struct window *w;
--- 9104,9121 ----
     before we start to fill in any display lines, because it can call
     eval.
  
!    If SAVE_MATCH_DATA is non-zero, we must save and restore it here.
  
!    If HOOKS_RUN is 1, that means a previous call to update_menu_bar
!    already ran the menu bar hooks for this redisplay, so there
!    is no need to run them again.  The return value is the
!    updated value of this flag, to pass to the next call.  */
! 
! static int
! update_menu_bar (f, save_match_data, hooks_run)
       struct frame *f;
       int save_match_data;
+      int hooks_run;
  {
    Lisp_Object window;
    register struct window *w;
***************
*** 9173,9187 ****
              specbind (Qoverriding_local_map, Qnil);
            }
  
!         /* Run the Lucid hook.  */
!         safe_run_hooks (Qactivate_menubar_hook);
  
!         /* If it has changed current-menubar from previous value,
!            really recompute the menu-bar from the value.  */
!         if (! NILP (Vlucid_menu_bar_dirty_flag))
!           call0 (Qrecompute_lucid_menubar);
  
-         safe_run_hooks (Qmenu_bar_update_hook);
          FRAME_MENU_BAR_ITEMS (f) = menu_bar_items (FRAME_MENU_BAR_ITEMS (f));
  
          /* Redisplay the menu bar in case we changed it.  */
--- 9180,9200 ----
              specbind (Qoverriding_local_map, Qnil);
            }
  
!         if (!hooks_run)
!           {
!             /* Run the Lucid hook.  */
!             safe_run_hooks (Qactivate_menubar_hook);
! 
!             /* If it has changed current-menubar from previous value,
!                really recompute the menu-bar from the value.  */
!             if (! NILP (Vlucid_menu_bar_dirty_flag))
!               call0 (Qrecompute_lucid_menubar);
! 
!             safe_run_hooks (Qmenu_bar_update_hook);
  
!             hooks_run = 1;
!           }
  
          FRAME_MENU_BAR_ITEMS (f) = menu_bar_items (FRAME_MENU_BAR_ITEMS (f));
  
          /* Redisplay the menu bar in case we changed it.  */
***************
*** 9210,9215 ****
--- 9223,9230 ----
          set_buffer_internal_1 (prev);
        }
      }
+ 
+   return hooks_run;
  }