search

nicferrier / emacs-noflet

noflet - nic's overriding flet, for fleting functions for the purpose of decorating them
72 stars 17 forks source link

this-fn is void only sometimes? #24

Open alphapapa opened 7 years ago

alphapapa commented 7 years ago

Using Emacs 25.2.2. I'm bewildered by this.

This works fine:

(noflet ((find-file-noselect (file-name)
                             (if (string-match-p "^#.*" file-name)
                                 (this-fn "/tmp/mytest")
                               (this-fn file-name)))
         (expand-file-name (file-name &optional thing)
                           (if (string-match-p "^#.*" file-name)
                               (concat "/tmp" (file-name-as-directory 
                                               (substring file-name 1)))
                             (funcall this-fn file-name thing))))
  (expand-file-name "#/thing"))  ; => "/tmp/thing/"

But this doesn't:

(noflet ((format-time-string (format-string &optional time zone)
                             (if time
                                 (this-fn format-string time zone)
                               "no time")))
  (format-time-string "%Y-%M-%d" "2017-08-23 12:00"))  
;; => 
;; Debugger entered--Lisp error: (void-function this-fn)
;; (this-fn format-string time zone)
;; (if time (this-fn format-string time zone) "no time")
;; (let ((this-fn saved-func-format-time-string)) (if time (this-fn format-string time zone) "no time"))
;; format-time-string("%Y-%M-%d" "2017-08-23 12:00")
;; (progn (progn (fset (quote format-time-string) (function (lambda (format-string &optional time zone) (let ((this-fn saved-func-format-time-string)) (if time (this-fn format-string time zone) "no time")))))) (format-time-string "%Y-%M-%d" "2017-08-23 12:00"))
;; (unwind-protect (progn (progn (fset (quote format-time-string) (function (lambda (format-string &optional time zone) (let (...) (if time ... "no time")))))) (format-time-string "%Y-%M-%d" "2017-08-23 12:00")) (progn (if (eq (symbol-function (quote noflet|base)) saved-func-format-time-string) (fmakunbound (quote format-time-string)) (fset (quote format-time-string) saved-func-format-time-string))))
;; (let ((saved-func-format-time-string (condition-case err (symbol-function (quote format-time-string)) (void-function (symbol-function (quote noflet|base)))))) (unwind-protect (progn (progn (fset (quote format-time-string) (function (lambda (format-string &optional time zone) (let ... ...))))) (format-time-string "%Y-%M-%d" "2017-08-23 12:00")) (progn (if (eq (symbol-function (quote noflet|base)) saved-func-format-time-string) (fmakunbound (quote format-time-string)) (fset (quote format-time-string) saved-func-format-time-string)))))
;; eval((let ((saved-func-format-time-string (condition-case err (symbol-function (quote format-time-string)) (void-function (symbol-function (quote noflet|base)))))) (unwind-protect (progn (progn (fset (quote format-time-string) (function (lambda (format-string &optional time zone) (let ... ...))))) (format-time-string "%Y-%M-%d" "2017-08-23 12:00")) (progn (if (eq (symbol-function (quote noflet|base)) saved-func-format-time-string) (fmakunbound (quote format-time-string)) (fset (quote format-time-string) saved-func-format-time-string))))) nil)
;; elisp--eval-last-sexp(nil)

Even this doesn't work:

(noflet ((message (&rest args)
                  (this-fn args)))
  (message "what"))  ; => let: Symbol’s function definition is void: this-fn

But this does work:

(noflet ((message (&rest args)
                  (apply this-fn args)))
  (message "what"))  ; => "what"

I've stared at the working macro call from the readme's example, which works fine, and at my format-time-string one, which doesn't work, but I can't figure out why it isn't working.

Thanks for your help.

npostavs commented 5 years ago 
(noflet ((find-file-noselect (file-name)
                             (if (string-match-p "^#.*" file-name)
                                 (this-fn "/tmp/mytest")
                               (this-fn file-name)))

I think this part of the example is wrong: this-fn is boundp, but not fboundp, so you can only call it with funcall. But the example never calls find-file-noselect so the error is not triggered.

alphapapa commented 5 years ago

Hi Noam,

I think you're right. The macro sets the rebound function symbol's definition like this, and it sets this-fn's value cell rather than its function cell:

                    (fset (quote ,name)
                          (cl-function
                           (lambda ,args
                             (let ((this-fn ,saved-func-namev))
                               ,@body))))

Other than the obvious slight performance impact, is there any reason to not set this-fn's function cell as well, so it could be called directly rather than with funcall?

If that's not possible, it should probably be promimently stated in the docstring that this-fn only has its value cell set, not its function cell.

nicferrier commented 5 years ago

I feel like that might have an impact on behaviour? Feel nervous about it. Maybe write me a test and send me a PR and I'll pull it?

Perhaps a second implementation that sets the function cell as well would be good? Or an option or something?

Sorry about the mahoosive delay.