Re: defadvice file-expand-wildcards ad-do-it value

[Kevin Ryde]

Michael Albinus <address@hidden> writes:
>
> That's true as well. Something like ad-remove-advice would be more
> precise.

Yep, per below, tested a bit.

Is there a reason to change a nil return to (list name)?  For a
non-tramp name it sounds like it could harm code expecting nil for no
matches.

2009-10-27  Kevin Ryde  <address@hidden>

        * tramp.el (defadvice file-expand-wildcards): Cannot rely on value of
        ad-do-it, must look at ad-return-value.  For non-tramp names don't
        change return nil to (list name), leave it per the original func.
        (tramp-unload-hook): `ad-remove-advice' of own advice only, not
        `ad-deactivate' everything.

--- tramp.el.~2.745.~   2009-10-28 08:49:29.000000000 +1100
+++ tramp.el    2009-10-28 08:50:59.000000000 +1100
@@ -8355,12 +8355,18 @@
          (if (string-match
               "[[*?]"
               (tramp-file-name-localname (tramp-dissect-file-name name)))
-             (setq ad-return-value (or ad-do-it (list name)))
+             (progn
+               ad-do-it
+               (unless ad-return-value
+                 (setq ad-return-value (list name))))
            (setq ad-return-value (list name)))
        ;; If it is not a Tramp file, just run the original function.
-       (setq ad-return-value (or ad-do-it (list name))))))
+       ad-do-it)))
   (add-hook 'tramp-unload-hook
-           (lambda () (ad-unadvise 'file-expand-wildcards))))
+           (lambda ()
+              (ad-remove-advice 'file-expand-wildcards
+                                'around 'tramp-advice-file-expand-wildcards)
+              (ad-activate 'file-expand-wildcards))))
 
 ;; Checklist for `tramp-unload-hook'
 ;; - Unload all `tramp-*' packages