Defining your own defun breaks find-function-search-for-symbol

Defining your own defun breaks find-function-search-for-symbol

[Bill Wohler]

We've (the MH-E team) developed mh-defun-compat (and mh-defmacro-compat)
to provide either an alias or a definition to a function depending on
the availability of the function. I've also created functions like
mh-defcustom to strip :package-version from the option definition on
systems that do not support it.

The problem is that if perform describe-function or describe-variable on
the functions and variables that these functions define, and then click
on the file in which they are defined, you get an error:

  Cannot find definition of `mh-letter-fill-column' in library `mh-e.el'

This can be fixed by modifying the find-*-regexp variables as follows:

  (eval-after-load "find-func"
    '(progn
       (setq find-function-regexp
	     (replace-regexp-in-string
	      "(def\\\\(" "(\\(mh-\\)?def\\(\\(un\\|macro\\)-compat\\|"
	      find-function-regexp t t))
       (setq find-variable-regexp
	     (replace-regexp-in-string
	      "(def" "(\\(mh-\\)?def" find-variable-regexp t t))
       (if (boundp 'find-face-regexp)
	   (setq find-face-regexp
		 (replace-regexp-in-string
		  "(def" "(\\(mh-\\)?def" find-face-regexp t t)))))

This code is in mh-e.el (not yet checked in) and works in Emacs 22 and
Emacs 21. Does anyone see any pitfalls of this approach? Can you think
of a better solution to this or the original problem?

p.s. Font-lock is broken for mh-defun-compat and friends too. Pity that
font-lock and find-function didn't share the same regexp for recognizing
a function/variable/face.

-- 
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.

Re: Defining your own defun breaks find-function-search-for-symbol

[Richard Stallman]

    We've (the MH-E team) developed mh-defun-compat (and mh-defmacro-compat)
    to provide either an alias or a definition to a function depending on
    the availability of the function. I've also created functions like
    mh-defcustom to strip :package-version from the option definition on
    systems that do not support it.

    The problem is that if perform describe-function or describe-variable on
    the functions and variables that these functions define, and then click
    on the file in which they are defined, you get an error:

If you rename your macro to defun-mh-compat, does that fix the problem?

Re: Defining your own defun breaks find-function-search-for-symbol

[Bill Wohler]

Richard Stallman <rms@gnu.org> wrote:

>     We've (the MH-E team) developed mh-defun-compat (and mh-defmacro-compat)
>     to provide either an alias or a definition to a function depending on
>     the availability of the function. I've also created functions like
>     mh-defcustom to strip :package-version from the option definition on
>     systems that do not support it.
> 
>     The problem is that if perform describe-function or describe-variable on
>     the functions and variables that these functions define, and then click
>     on the file in which they are defined, you get an error:
> 
> If you rename your macro to defun-mh-compat, does that fix the problem?

No, but defunmhcompat works. I think that's a bug in
find-function-regexp which uses \\w+ after "def". In contract,
find-variable-regexp uses \\(\\w\\|\\s_\\)+ so that renaming
mh-defcustom to defcustom-mh would work. Any objections if I substitute
\\(\\w\\|\\s_\\)+ for \\w+ in find-function-regexp?

However, would we want to rename the functions in this manner? It would
go against the namespace convention.

Another option would be to add another %s escape to the find-*-regexp
variables which would be used for the package of the symbol like this:

  ...\\(%s-\\)?def...

For example, when looking up mh-assoc-string, a definition with
mh-defun-compat would be considered. The advantage of this is that it
would work with any package. A disadvantage is this would break user's
settings of this option.

To summarize, we have three proposed solutions:

1. Add MH-E-specific patterns to the regexp in mh-e.el.

2. Rename the functions to match the existing patterns.

3. Add an escape for the package name to the patterns.

Please let me know which one you prefer.

-- 
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.

Re: Defining your own defun breaks find-function-search-for-symbol

[Richard Stallman]

     Any objections if I substitute
    \\(\\w\\|\\s_\\)+ for \\w+ in find-function-regexp?

Please do.

    However, would we want to rename the functions in this manner?

Yes.

								   It would
    go against the namespace convention.

No, this is normal.

    For example, when looking up mh-assoc-string, a definition with
    mh-defun-compat would be considered. The advantage of this is that it
    would work with any package. A disadvantage is this would break user's
    settings of this option.

That is too far-reaching to be considered now.

Re: Defining your own defun breaks find-function-search-for-symbol

[Stefan Monnier]

> We've (the MH-E team) developed mh-defun-compat (and mh-defmacro-compat)
[...]
> The problem is that if perform describe-function or describe-variable on
> the functions and variables that these functions define, and then click
> on the file in which they are defined, you get an error:

>   Cannot find definition of `mh-letter-fill-column' in library `mh-e.el'

I believe the patch below which I've just installed fixes the problem.


        Stefan


--- orig/lisp/emacs-lisp/find-func.el
+++ mod/lisp/emacs-lisp/find-func.el
@@ -229,7 +229,7 @@
 	    (goto-char (point-min))
 	    (if (or (re-search-forward regexp nil t)
 		    (re-search-forward
-		     (concat "^([^ ]+" find-function-space-re "['(]"
+		     (concat "^([^ ]+" find-function-space-re "['(]?"
 			     (regexp-quote (symbol-name symbol))
 			     "\\_>")
 		     nil t))

Re: Defining your own defun breaks find-function-search-for-symbol

[Bill Wohler]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> We've (the MH-E team) developed mh-defun-compat (and mh-defmacro-compat)
> [...]
>> The problem is that if perform describe-function or describe-variable on
>> the functions and variables that these functions define, and then click
>> on the file in which they are defined, you get an error:
>
>>   Cannot find definition of `mh-letter-fill-column' in library `mh-e.el'
>
> I believe the patch below which I've just installed fixes the problem.
...
> -		     (concat "^([^ ]+" find-function-space-re "['(]"
> +		     (concat "^([^ ]+" find-function-space-re "['(]?"

Indeed. Nicely done, thanks!

-- 
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.

Re: Defining your own defun breaks find-function-search-for-symbol

[Bill Wohler]

Richard Stallman <rms@gnu.org> wrote:

>      Any objections if I substitute
>     \\(\\w\\|\\s_\\)+ for \\w+ in find-function-regexp?
> 
> Please do.

Done.

>     However, would we want to rename the functions in this manner?
> 
> Yes.

With Stefan's patch, this renaming won't be necessary and we're in good
shape.

-- 
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.

Re: Defining your own defun breaks find-function-search-for-symbol

[Richard Stallman]

    >     However, would we want to rename the functions in this manner?
    > 
    > Yes.

    With Stefan's patch, this renaming won't be necessary and we're in good
    shape.

I do not follow.  Could you explain the connection between them?

Re: Defining your own defun breaks find-function-search-for-symbol

[Bill Wohler]

Richard Stallman <rms@gnu.org> wrote:

>     >     However, would we want to rename the functions in this manner?
>     > 
>     > Yes.
> 
>     With Stefan's patch, this renaming won't be necessary and we're in good
>     shape.
> 
> I do not follow.  Could you explain the connection between them?

With his patch, functions and variables defined with our mh-defun-compat
and mh-defcustom functions are found by find-func.

I was focusing on the primary regexp which is used by that package
(`regexp' in the following code fragment). Your suggestion of renaming
mh-defun-compat, for example, to defun-mh-compat (with my fix) matches
`regexp'.

Stefan fixed what looks like a more general secondary, backup, regexp
which matches mh-defun-compat definitions.

Here's the code:

    (or (re-search-forward regexp nil t)
	(re-search-forward
	 (concat "^([^ ]+" find-function-space-re "['(]?"
		 (regexp-quote (symbol-name symbol))
		 "\\_>")
	 nil t))

Does that help?

-- 
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.