Re: image-load-path/load-path function

[Bill Wohler <wohler@newt.com>]

Reiner Steib <reinersteib+gmane@imap.cc> wrote:

> On Fri, Mar 03 2006, Bill Wohler wrote:
> 
> > I think we're ready to move it to image.el. Let me know if you think I
> > should proceed.
> >
> > I think the name image-load-path would be fine: If you ask for the
> > variable image-load-path, you get the image load path, if you ask for
> > the function image-load-path, you get the image load path. Seems
> > consistent. What do you think?
> 
> Maybe `image-relative-load-path' or `image-load-path-relative' to make
> it more clear?

Hmmm, I'm not entirely enamored with these. If not image-load-path, then
perhaps something like image-load-path-for-library since library is the
first argument.

> > (defun mh-image-load-path (library image &optional path)
> >   "Return a suitable search path for images of LIBRARY.
> >
> > Images for LIBRARY are searched for in \"../../etc/images\" and
> > \"../etc/images\" relative to the files in \"lisp/LIBRARY\", in
> > `image-load-path', or in `load-path'.
> >
> > This function returns value of `load-path' augmented with the
> > path to IMAGE.  If PATH is given, it is used instead of
> > `load-path'."
> 
> Adding a calling example to doc string won't hurt.  Maybe emphasize
> that IMAGE must include the extension, e.g. .xpm (maybe not necessary
> when we give an example).

I was thinking the very same thing.

> >   (let ((mh-image-directory))
> [...]
> >      ((let (mh-library-name d1ei d2ei)
> 
> The local variables mh-image-directory and mh-library-name should have
> a non-prefixed name e.g. image-dir and library-name.  This would also
> make synching more easy.

Agreed. That occurred to me too, and it seems that there aren't any
variables in the functions we call that we might clobber. I'd prefer the
non-abbreviated name image-directory, if you don't mind.

>                           See the attached patch against
> emacs/lisp/mh-e/mh-utils.el.

Thanks.

> >     (unless (file-exists-p mh-image-directory)
> >       (error "Directory %s in mh-image-directory does not exist"
> > 	     mh-image-directory))
> >     (unless (file-exists-p (expand-file-name image mh-image-directory))
> >       (error "Directory %s in mh-image-directory does not contain image %s"
> >              mh-image-directory image))
> 
> As mh-image-directory cannot be not a user-specified directory
> anymore, I doubt that these errors are very useful.  But if you'd like
> to keep these two `error' calls, we should add an optional no-error
> argument as suggested by Katsumi Yamaoka.  We would set this to t in
> Gnus and the function would use `message' or `error' depending on its
> value.

Good catch. Actually, I just found that there was a bug in this code
since if the given image couldn't be found, this code would throw an
error with (file-exists-p nil).

There still needs to be an error check in case the provided image wasn't
found at all in ../../etc/images, image-load-path, or load-path. This
can be a default case in the cond. I don't think a NOERROR argument
belongs here; it belongs higher up in the call-chain.

Here's a patch coming back at you (but I'm sending another note
immediately after this one).

--- mh-utils.el.orig	2006-03-03 12:07:57.000000000 -0800
+++ mh-utils.el	2006-03-03 12:39:25.000000000 -0800
@@ -82,7 +82,7 @@
   (delete-region (point) (progn (forward-line lines) (point))))
 
 ;;;###mh-autoload
-(defun mh-image-load-path (library image &optional path)
+(defun mh-image-load-path-for-library (library image &optional path)
   "Return a suitable search path for images of LIBRARY.
 
 Images for LIBRARY are searched for in \"../../etc/images\" and
@@ -91,10 +91,20 @@
 
 This function returns value of `load-path' augmented with the
 path to IMAGE.  If PATH is given, it is used instead of
-`load-path'."
+`load-path'.
+
+Here is an example that uses a common idiom to provide
+compatibility with versions of Emacs that lack the variable
+`image-load-path':
+
+  (let ((load-path
+         (image-load-path-for-library \"mh-e\" \"mh-logo.xpm\" 'load-path))
+        (image-load-path
+         (image-load-path-for-library \"mh-e\" \"mh-logo.xpm\" 'image-load-path)))
+    (mh-tool-bar-folder-buttons-init))"
   (unless library (error "No library specified"))
   (unless image   (error "No image specified"))
-  (let ((image-dir))
+  (let ((image-directory))
     (cond
      ;; Try relative setting.
      ((let (library-name d1ei d2ei)
@@ -102,17 +112,15 @@
         (setq library-name (locate-library library))
         (if (not library-name)
             (error "Cannot find library %s in load-path" library))
-        ;; And then set image-dir relative to that.
+        ;; And then set image-directory relative to that.
         (setq
          ;; Go down 2 levels.
          d2ei (expand-file-name
-               (concat (file-name-directory library-name)
-                       "../../etc/images"))
+               (concat (file-name-directory library-name) "../../etc/images"))
          ;; Go down 1 level.
          d1ei (expand-file-name
-               (concat (file-name-directory library-name)
-                       "../etc/images")))
-        (setq image-dir
+               (concat (file-name-directory library-name) "../etc/images")))
+        (setq image-directory
               ;; Set it to nil if image is not found.
               (cond ((file-exists-p (expand-file-name image d2ei)) d2ei)
                     ((file-exists-p (expand-file-name image d1ei)) d1ei)))))
@@ -124,8 +132,8 @@
                   ;; Images in load-path.
                   (locate-library image)))
             parent)
-        ;; Since the image might be in a nested directory
-        ;; (for example, mail/attach.pbm), adjust `image-dir'
+        ;; Since the image might be in a nested directory (for
+        ;; example, mail/attach.pbm), adjust `image-directory'
         ;; accordingly.
         (and dir
              (setq dir (file-name-directory dir))
@@ -133,25 +141,20 @@
                (while (setq parent (file-name-directory img))
                  (setq img (directory-file-name parent)
                        dir (expand-file-name "../" dir)))
-               (setq image-dir dir))))))
-    ;;
-    (unless (file-exists-p image-dir)
-      (error "Directory %s in image-dir does not exist"
-	     image-dir))
-    (unless (file-exists-p (expand-file-name image image-dir))
-      (error "Directory %s in image-dir does not contain image %s"
-             image-dir image))
+               (setq image-directory dir)))))
+     (t
+      (error "Could not find image %s for library %s" image library)))
+
     ;; Return augmented `image-load-path' or `load-path'.
     (cond ((and path (symbolp path))
-           (nconc (list image-dir)
-                  (delete image-dir
+           (nconc (list image-directory)
+                  (delete image-directory
                           (if (boundp path)
                               (copy-sequence (symbol-value path))
                             nil))))
           (t
-           (nconc (list image-dir)
-                  (delete image-dir
-                          (copy-sequence load-path)))))))
+           (nconc (list image-directory)
+                  (delete image-directory (copy-sequence load-path)))))))
 
 ;;;###mh-autoload
 (defun mh-make-local-vars (&rest pairs)

-- 
Bill Wohler <wohler@newt.com>  http://www.newt.com/wohler/  GnuPG ID:610BD9AD
Maintainer of comp.mail.mh FAQ and MH-E. Vote Libertarian!
If you're passed on the right, you're in the wrong lane.


-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642