Re: Emacs pretest 22.1.91

[Juanma Barranquero]

On Wed, Feb 27, 2008 at 5:14 AM, Eli Zaretskii <address@hidden> wrote:

>  How about if you make that change and see if someone hollers?

Sounds like a plan.

However, all the tool-bar-setup kerfuffle is a bit weird. Károly
Lőrentey added an optional frame argument, but then also added a guard
so tool-bar-setup's innards only run once; seems like the whole point
of having a frame as an argument is not wasting effort in doing
"(with-selected-frame frame (tool-bar-setup))" when in all cases
except the first one tool-bar-setup is not going do to anything at
all...

The cleanest fix I can think of at this moment is moving the
tool-bar-setup call, from `x-create-frame-with-faces' to
`frame-notice-user-settings', which runs at the right time and already
does tool-bar work. That also allows to remove the frame argument to
tool-bar-setup, which would not really be needed and just adds
cognitive overhead.

The alternative would be to maintain the call to tool-bar-setup in
`x-create-frame-with-faces', but adding it to `after-init-hook' only
if we haven't yet loaded the init files. That seems hackish in the
extreme. (And, how do you detect that you have loaded the init files?
I suppose "(when after-init-time ...)" is a way...)

The patch below (with diff -b to exclude the space changes in
tool-bar-setup) does all the right things on Windows: fixes the
original XPM-loading problem, respects settings of `tool-bar-lines'
frame parameter and/or calls to `tool-bar-mode' in .emacs, etc.

I'd like someone who's using Emacs on GNU/Linux (in window and -nw
modes) to give it a little try before commiting it, though.

             Juanma


2008-02-27  Juanma Barranquero  <address@hidden>

        * faces.el (x-create-frame-with-faces): Don't call `tool-bar-setup'.
        * frame.el (frame-notice-user-settings): Call `tool-bar-setup'.
        * tool-bar.el (tool-bar-setup): Remove optional `frame' argument.

Index: lisp/faces.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/faces.el,v
retrieving revision 1.394
diff -u -2 -b -r1.394 faces.el
--- lisp/faces.el       22 Feb 2008 23:34:57 -0000      1.394
+++ lisp/faces.el       27 Feb 2008 11:16:57 -0000
@@ -1999,8 +1999,4 @@
          (frame-set-background-mode frame)
          (face-set-after-frame-default frame)
-         ;; Make sure the tool-bar is ready to be enabled.  The
-         ;; `tool-bar-lines' frame parameter will not take effect
-         ;; without this call.
-         (tool-bar-setup frame)
          (if (null visibility-spec)
              (make-frame-visible frame)
Index: lisp/frame.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/frame.el,v
retrieving revision 1.269
diff -u -2 -b -r1.269 frame.el
--- lisp/frame.el       14 Feb 2008 21:16:36 -0000      1.269
+++ lisp/frame.el       27 Feb 2008 11:17:25 -0000
@@ -285,5 +285,9 @@
          (setq default-frame-alist
                (cons (cons 'tool-bar-lines (if tool-bar-mode 1 0))
-                     default-frame-alist))))))
+                     default-frame-alist))))
+      ;; Make sure the tool-bar is ready to be enabled.  The
+      ;; `tool-bar-lines' frame parameter will not take effect
+      ;; without this call.
+      (tool-bar-setup)))

   ;; Creating and deleting frames may shift the selected frame around,
Index: lisp/tool-bar.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/tool-bar.el,v
retrieving revision 1.11
diff -u -2 -b -r1.11 tool-bar.el
--- lisp/tool-bar.el    27 Feb 2008 10:17:06 -0000      1.11
+++ lisp/tool-bar.el    27 Feb 2008 11:19:31 -0000
@@ -232,7 +232,6 @@
   "Set to t if the tool-bar has been set up by `tool-bar-setup'.")

-(defun tool-bar-setup (&optional frame)
+(defun tool-bar-setup ()
   (unless tool-bar-setup
-    (with-selected-frame (or frame (selected-frame))
       ;; People say it's bad to have EXIT on the tool bar, since users
       ;; might inadvertently click that button.
@@ -287,5 +286,5 @@
                       'help
                       :help "Pop up the Help menu"))
-  (setq tool-bar-setup t))))
+    (setq tool-bar-setup t)))