Unable to patch a function

[diamond-lizard]

I'd like to make a simple, two-line change to a rather involved function. The original is here and my attempt to patch it using el-patch and el-patch-swap is here.

The function comes from w3m-form.el of emacs-w3m [1]. It's what gets called when point is over something like a github clone web form on a github repo web page, where the user can use the c key to copy the contents of the web form to Emacs' kill ring. My patch just uses a custom function of mine (set-clipboard-contents) to also copy it to the X primary selection.

The change is to just line 40 in the original, using el-patch-swap in lines 40-43 in the patched version.

If I simply redefine this function using defun instead of el-patch or el-patch-swap but just changing

(define-key keymap "c" `(lambda nil (interactive) (kill-new ,value)))

to

(define-key keymap "c" `(lambda nil (interactive) (kill-new ,value)
                          (set-clipboard-contents ,value)))

it works just fine -- the contents of the form get copied to the X primary selection as expected.

But if I use el-patch and el-patch-swap to do it, nothing happens when I press the c key in a github clone form.

Also, after the regular re-define of the function, (describe-function 'w3m-form-expand-form) tells me just that w3m-form-expand-form is a Lisp function. and does not tell me where it was defined.

But, if instead of re-defining the function using defun I instead used el-patch, when I (describe-function 'w3m-form-expand-form) emacs tells me that w3m-form-expand-form is a Lisp function in ‘w3m-form.el’. so I'm not sure if emacs is even detecting that the function has been re-defined using el-patch (though maybe that's how el-patch is supposed to work).

I'm not sure how to troubleshoot this further, so I thought I'd ask here.

Thanks in advance for any help you can provide.

[1] - cvs -q -d:pserver:anonymous@cvs.namazu.org:/storage/cvsroot co emacs-w3m

[diamond-lizard]

I just tried M-x el-patch-ediff-patch and that showed the change I made as expected.

Then I thought that maybe the behavior I'm seeing is due to w3m-form.el being byte-compiled, so I tried evaling the original defun of w3m-form-expand-form and then evaling my el-patch'ed version, but didn't have any more luck. It's still not copying the contents of the web form under point to the clipboard when I type c.

[raxod502]

My first question would be whether you are making sure to load your patch inside a with-eval-after-load 'w3m-form. (Although it sounds like something else is wrong if evaluating your el-patch form after the original definition has already been loaded doesn't fix the problem.) I can try to debug it, but I would need a way to reproduce the problem locally (I don't use w3m normally).

To debug further on your end, you could check (symbol-function 'w3m-form-expand-form) and see whether the code is the normal definition or your modified one (or some byte-compiled version). You can also look at the source code of el-patch. All an el-patch-defun does is resolve the directives inside, and then evaluate the resulting defun form. So if you're observing a discrepancy between using defun and el-patch-defun, you could pp-macroexpand-last-sexp on the el-patch-defun and see if that is generating the same defun that you are testing with. I get this; what if you use the macroexpanded output instead of the el-patch-defun? Either way, it should be easy to bisect between it and either the original el-patch-defun or the defun you were testing with.

As a side note, wouldn't interprogram-cut-function be the correct way to accomplish what you're trying to do?

[diamond-lizard]

Wrapping the el-patch call within a with-eval-after-load 'w3m-form did it! Now it works.

I'd had it wrapped in with-eval-after-load 'w3m thinking that'd be enough, but obviously not.

Regarding why I'm not just using interprogram-cut-function: it's because if I set it, then every copy/kill that I made would wind up in the X primary selection, while I want only certain specifically sepected copies/kills to go there.

On the other hand, now that you mention it, another way of approaching this problem would have been to advise w3m-form-expand-form so that it sets interprogram-cut-function before w3m-form-expand-form runs and set it back to nil afterwards. That might actually be cleaner, since then I wouldn't be patching this function.

Thanks for the idea!

[diamond-lizard]

It looks like the plan to advise w3m-form-expand-form doesn't work because all that function does is set up a keybinding: (define-key keymap "c" '(lambda nil (interactive) (kill-new ,value))) which is not really affected by setting interprogram-cut-function right before w3m-form-expand-form runs, as what matters is when the user types c (by which time interprogram-cut-function will have been set back to nil). So it looks like I'll have to patch w3m-form-expand-form after all.

Anyway, thank you for all your help!

[raxod502]

In a599953, I updated the "Basic usage" section to better reflect best practices for using el-patch.

sets interprogram-cut-function before w3m-form-expand-form runs and set it back to nil afterwards

Sounds like let to me. But as you said, this approach won't work since there's no (sane) way to intercept the temporary keymap before it's assigned directly to a text property except by patching the function definition. It might be worthwhile to make a feature request upstream.

[diamond-lizard]

Thank you. That documentation is very helpful.

I might make a feature request, as you recommend, but unfortunately I don't think emacs-w3m is actively maintained any more, so I'm not sure how much good it'll do.