bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Ihor Radchenko]

Hi,

There have been a discussion [1] about including some of the helpful
package [2] features into built-in *Help* buffers.

One of the discussed features was displaying function source code right
in *Help* buffers. This feature usefulness have been objected at that
time, on the grounds that showing function code may be too long and
cause large *Help* buffers.

Recently, we have stumbled upon a situation when showing function code
is not only useful, but one of the few sane ways to examine it [3].
Functions that are not directly written in the source but rather
generated programmatically cannot currently be easily examined using
built-in help functionality (using describe-function or other means).
The help buffer only links to the (point-min) in the library defining
the generated function - not very helpful. In contrast, helpful extracts
function body from symbol function slot.

To illustrate, one can try the following:

1. emacs -Q
2. M-: (require 'ob-shell)
3. <F1> f org-babel-execute:sh <RET>
4. Click on the source code link in *Help* buffer
5. Observe point jumping to (point-min) with no obvious way to find the
   function definition.

Would it be possible to provide function body info via *Help* system in
Emacs?

[1]
https://yhetil.org/emacs-devel/AM9PR09MB4977A7738F83C5C24EF5829196D59@AM9PR09MB4977.eurprd09.prod.outlook.com

[2] https://github.com/Wilfred/helpful

[3] https://orgmode.org/list/87h6x8kluc.fsf@localhost

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.35, cairo version 1.17.6) of 2023-01-02 built on localhost
Repository revision: 9377a3c889aa3b178a11a3b849c3d1665da096d6
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Gentoo Linux


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Date: Thu, 05 Jan 2023 07:56:11 +0000
> 
> There have been a discussion [1] about including some of the helpful
> package [2] features into built-in *Help* buffers.
> 
> One of the discussed features was displaying function source code right
> in *Help* buffers. This feature usefulness have been objected at that
> time, on the grounds that showing function code may be too long and
> cause large *Help* buffers.
> 
> Recently, we have stumbled upon a situation when showing function code
> is not only useful, but one of the few sane ways to examine it [3].
> Functions that are not directly written in the source but rather
> generated programmatically cannot currently be easily examined using
> built-in help functionality (using describe-function or other means).
> The help buffer only links to the (point-min) in the library defining
> the generated function - not very helpful. In contrast, helpful extracts
> function body from symbol function slot.
> 
> To illustrate, one can try the following:
> 
> 1. emacs -Q
> 2. M-: (require 'ob-shell)
> 3. <F1> f org-babel-execute:sh <RET>
> 4. Click on the source code link in *Help* buffer
> 5. Observe point jumping to (point-min) with no obvious way to find the
>    function definition.
> 
> Would it be possible to provide function body info via *Help* system in
> Emacs?

Provide how?

You basically ignore the valid objections voiced against such a
feature, and present a specific case where (a) the source code is
extremely short, and (b) the existing Help facilities are less than
helpful because they have a deficiency.  How does it follow from the
specific use case you present that we must show the source code in
Help buffers?  An alternative solution would be to teach Help
functions to find the place where the source code is generated and let
users examine that if they want.  I could even argue that this
alternative is better, because with your proposal we will show a
source code that doesn't actually exist anywhere in the form we show
it.

If we do decide to show the source code in some circumstances, we need
to do that selectively and in a way that doesn't display unnecessarily
large Help buffers that would then run the risk of "drowning" useful
information, making it easy to miss.  So any such proposal should
suggest how to avoid the downsides, at least.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> Would it be possible to provide function body info via *Help* system in
>> Emacs?
>
> Provide how?

For example, clicking source code link for function without definition
may display a new separate buffer with function body stored in the
function symbol.

> You basically ignore the valid objections voiced against such a
> feature, and present a specific case where (a) the source code is
> extremely short, and (b) the existing Help facilities are less than
> helpful because they have a deficiency.  How does it follow from the
> specific use case you present that we must show the source code in
> Help buffers?

Sorry, I did not imply that the function body will be _displayed_ in
*Help* buffers. Just accessible via some means.

As an alternative to my above suggestion, the function body might be
displayed upon clicking on some button in help buffer. Maybe using
outlines to fold/unfold the definition as necessary.

(BTW, don't *Help* buffer already suffer from too long variable values
being displayed? I don't mean it as an argument to add function body -
rather pointing that we might want some ideas how to deal with long
sexps displayed in *Help*)

> An alternative solution would be to teach Help
> functions to find the place where the source code is generated and let
> users examine that if they want.  I could even argue that this
> alternative is better, because with your proposal we will show a
> source code that doesn't actually exist anywhere in the form we show
> it.

Yes, for the particular use case I mentioned here. But how? I do not see
any obvious ways how to know where the function was generated unless
`defun' stores some extra auxiliary information when defining a
function.

> If we do decide to show the source code in some circumstances, we need
> to do that selectively and in a way that doesn't display unnecessarily
> large Help buffers that would then run the risk of "drowning" useful
> information, making it easy to miss.  So any such proposal should
> suggest how to avoid the downsides, at least.

I agree that too long function bodies, similar to too long variable
values are objectively degrading readability.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: 60568@debbugs.gnu.org
> Date: Thu, 05 Jan 2023 08:20:21 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Would it be possible to provide function body info via *Help* system in
> >> Emacs?
> >
> > Provide how?
> 
> For example, clicking source code link for function without definition
> may display a new separate buffer with function body stored in the
> function symbol.

That is less desirable, IMO, then teaching Help commands to be smarter
with showing the source code.  One of the main reasons to go to the
source code is to examine the surrounding code, and consider the
function in its context.  This will be impossible with your proposal.

> (BTW, don't *Help* buffer already suffer from too long variable values
> being displayed? I don't mean it as an argument to add function body -
> rather pointing that we might want some ideas how to deal with long
> sexps displayed in *Help*)

Yes, they do.  Patches are welcome to deal with these cases.

> > An alternative solution would be to teach Help
> > functions to find the place where the source code is generated and let
> > users examine that if they want.  I could even argue that this
> > alternative is better, because with your proposal we will show a
> > source code that doesn't actually exist anywhere in the form we show
> > it.
> 
> Yes, for the particular use case I mentioned here. But how? I do not see
> any obvious ways how to know where the function was generated unless
> `defun' stores some extra auxiliary information when defining a
> function.

Storing more information is one way.  Looking for more sophisticated
regexps (if the "usual" ones fail) is another.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Gregory Heytings]

>
> Would it be possible to provide function body info via *Help* system in 
> Emacs?
>

Would this fit the bill?

diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index bf64d032b6..049eb2fa5a 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -281,7 +281,14 @@ help-function-def--button-function
              (unless (= (point) position)
                (push-mark nil t))
              (goto-char position))
-        (message "Unable to find location in file")))))
+       (with-help-window (help-buffer)
+         (insert (format "Function `%s' could not be found in " fun))
+         (if file
+              (insert (format "`%s'." file))
+            (insert " this file."))
+         (insert "\n\n")
+         (insert (format "Function definition:\n\n" fun))
+         (insert (format "%S" (symbol-function fun))))))))

  (define-button-type 'help-function-def
    :supertype 'help-xref

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Gregory Heytings]

>> Would it be possible to provide function body info via *Help* system in 
>> Emacs?
>
> Would this fit the bill?
>

Shorter and better:

diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index bf64d032b6..75ccd93938 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -281,7 +281,11 @@ help-function-def--button-function
              (unless (= (point) position)
                (push-mark nil t))
              (goto-char position))
-        (message "Unable to find location in file")))))
+       (with-help-window (help-buffer)
+         (insert (format "Function `%s' could not be found in `%s'."
+                          fun file))
+         (insert "\n\nFunction definition:\n\n")
+         (insert (pp-to-string (symbol-function fun))))))))

  (define-button-type 'help-function-def
    :supertype 'help-xref

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ihor Radchenko <yantar92@posteo.net> writes:

> Recently, we have stumbled upon a situation when showing function code
> is not only useful, but one of the few sane ways to examine it [3].
> Functions that are not directly written in the source but rather
> generated programmatically cannot currently be easily examined using
> built-in help functionality (using describe-function or other means).
> The help buffer only links to the (point-min) in the library defining
> the generated function - not very helpful. In contrast, helpful extracts
> function body from symbol function slot.

I'd prefer we teach the Help facilities how to go to the source code of
that kind of functions.  Some years ago, I added support for go to
definition from *Help* buffers when the symbol was generated by a Lisp
macro; there's no reason we couldn't extend the code to also handle this
particular case.

Showing the source code inline is something I feel more suitable for
third-party packages.  It is probably more interesting from a core Emacs
perspective to make sure that help-mode provides the appropriate hooks
for users and packages to customize what to show without needing to
reimplement *Help* from scratch.  I don't know what other people think,
my opinion about this may be a minority.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Ihor Radchenko]

Daniel Martín <mardani29@yahoo.es> writes:

> I'd prefer we teach the Help facilities how to go to the source code of
> that kind of functions.  Some years ago, I added support for go to
> definition from *Help* buffers when the symbol was generated by a Lisp
> macro; there's no reason we couldn't extend the code to also handle this
> particular case.

Could you point to the implementation?

> Showing the source code inline is something I feel more suitable for
> third-party packages.  It is probably more interesting from a core Emacs
> perspective to make sure that help-mode provides the appropriate hooks
> for users and packages to customize what to show without needing to
> reimplement *Help* from scratch.  I don't know what other people think,
> my opinion about this may be a minority.

+1 from me.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> Cc: 60568@debbugs.gnu.org
> Date: Thu, 05 Jan 2023 11:45:15 +0100
> From:  Daniel Martín via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Ihor Radchenko <yantar92@posteo.net> writes:
> 
> > Recently, we have stumbled upon a situation when showing function code
> > is not only useful, but one of the few sane ways to examine it [3].
> > Functions that are not directly written in the source but rather
> > generated programmatically cannot currently be easily examined using
> > built-in help functionality (using describe-function or other means).
> > The help buffer only links to the (point-min) in the library defining
> > the generated function - not very helpful. In contrast, helpful extracts
> > function body from symbol function slot.
> 
> I'd prefer we teach the Help facilities how to go to the source code of
> that kind of functions.

Agreed.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Gregory Heytings]

>> I'd prefer we teach the Help facilities how to go to the source code of 
>> that kind of functions.
>
> Agreed.
>

What is missing in the five-line patch that does this?  With the recipe, 
the file in which org-babel-execute:sh was defined (ob-shell.el) is 
opened, and the function definition is displayed.  I don't think we can do 
much better (e.g. reliably find the place where the function was defined).

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> Date: Thu, 05 Jan 2023 12:33:26 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Daniel Martín <mardani29@yahoo.es>, yantar92@posteo.net, 
>     60568@debbugs.gnu.org
> 
> 
> >> I'd prefer we teach the Help facilities how to go to the source code of 
> >> that kind of functions.
> >
> > Agreed.
> >
> 
> What is missing in the five-line patch that does this?  With the recipe, 
> the file in which org-babel-execute:sh was defined (ob-shell.el) is 
> opened, and the function definition is displayed.  I don't think we can do 
> much better (e.g. reliably find the place where the function was defined).

Why not?  A function definition usually has some specific keywords in
it.  It is true that someone could have obfuscated the code which
generates a function to the degree that it would be infeasible to
recognize, but we don't need to support such arbitrary measures, we
only need to support what's really being used.  IOW, "reliably"
doesn't need to mean with 110% reliability.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Gregory Heytings]

>> What is missing in the five-line patch that does this?  With the 
>> recipe, the file in which org-babel-execute:sh was defined 
>> (ob-shell.el) is opened, and the function definition is displayed.  I 
>> don't think we can do much better (e.g. reliably find the place where 
>> the function was defined).
>
> Why not?  A function definition usually has some specific keywords in 
> it.  It is true that someone could have obfuscated the code which 
> generates a function to the degree that it would be infeasible to 
> recognize, but we don't need to support such arbitrary measures, we only 
> need to support what's really being used.  IOW, "reliably" doesn't need 
> to mean with 110% reliability.
>

Hmmm, you mean try to find heuristically where e.g. in the ob-shell.el 
file the org-babel-execute:sh function was defined?

The function body is this:

(lambda
   (body params)
   "Execute a block of sh commands with Babel."
   (let
       ((shell-file-name "sh")
        (org-babel-prompt-command
 	(or
 	 (alist-get "sh" org-babel-shell-set-prompt-commands)
 	 (alist-get t org-babel-shell-set-prompt-commands))))
     (org-babel-execute:shell body params)))

and it has been created here:

(defun org-babel-shell-initialize ()
   "Define execution functions associated to shell names.
This function has to be called whenever `org-babel-shell-names'
is modified outside the Customize interface."
   (interactive)
   (dolist (name org-babel-shell-names)
     (eval `(defun ,(intern (concat "org-babel-execute:" name))
 	       (body params)
 	     ,(format "Execute a block of %s commands with Babel." name)
 	     (let ((shell-file-name ,name)
                    (org-babel-prompt-command
                     (or (alist-get ,name org-babel-shell-set-prompt-commands)
                         (alist-get t org-babel-shell-set-prompt-commands))))
 	       (org-babel-execute:shell body params))))
     (eval `(defalias ',(intern (concat "org-babel-variable-assignments:" name))
 	     'org-babel-variable-assignments:shell
 	     ,(format "Return list of %s statements assigning to the block's \
variables."
 		      name)))
     (eval `(defvar ,(intern (concat "org-babel-default-header-args:" name)) '()))))

Do you think there is a way to find that place?  AFAIU, all elements in 
the function definition could be variables in the form that is evalled.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> Date: Thu, 05 Jan 2023 14:27:41 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: mardani29@yahoo.es, yantar92@posteo.net, 60568@debbugs.gnu.org
> 
> 
> >> What is missing in the five-line patch that does this?  With the 
> >> recipe, the file in which org-babel-execute:sh was defined 
> >> (ob-shell.el) is opened, and the function definition is displayed.  I 
> >> don't think we can do much better (e.g. reliably find the place where 
> >> the function was defined).
> >
> > Why not?  A function definition usually has some specific keywords in 
> > it.  It is true that someone could have obfuscated the code which 
> > generates a function to the degree that it would be infeasible to 
> > recognize, but we don't need to support such arbitrary measures, we only 
> > need to support what's really being used.  IOW, "reliably" doesn't need 
> > to mean with 110% reliability.
> 
> Hmmm, you mean try to find heuristically where e.g. in the ob-shell.el 
> file the org-babel-execute:sh function was defined?

Yes.

> The function body is this:
> 
> (lambda
>    (body params)
>    "Execute a block of sh commands with Babel."
>    (let
>        ((shell-file-name "sh")
>         (org-babel-prompt-command
>  	(or
>  	 (alist-get "sh" org-babel-shell-set-prompt-commands)
>  	 (alist-get t org-babel-shell-set-prompt-commands))))
>      (org-babel-execute:shell body params)))
> 
> and it has been created here:
> 
> (defun org-babel-shell-initialize ()
>    "Define execution functions associated to shell names.
> This function has to be called whenever `org-babel-shell-names'
> is modified outside the Customize interface."
>    (interactive)
>    (dolist (name org-babel-shell-names)
>      (eval `(defun ,(intern (concat "org-babel-execute:" name))
>  	       (body params)
>  	     ,(format "Execute a block of %s commands with Babel." name)
>  	     (let ((shell-file-name ,name)
>                     (org-babel-prompt-command
>                      (or (alist-get ,name org-babel-shell-set-prompt-commands)
>                          (alist-get t org-babel-shell-set-prompt-commands))))
>  	       (org-babel-execute:shell body params))))
>      (eval `(defalias ',(intern (concat "org-babel-variable-assignments:" name))
>  	     'org-babel-variable-assignments:shell
>  	     ,(format "Return list of %s statements assigning to the block's \
> variables."
>  		      name)))
>      (eval `(defvar ,(intern (concat "org-babel-default-header-args:" name)) '()))))
> 
> Do you think there is a way to find that place?

Well, searching for the regexp "defun .*org-babel-execute:" would have
done that, right?

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Gregory Heytings]

>
> Well, searching for the regexp "defun .*org-babel-execute:" would have 
> done that, right?
>

In this specific case, yes.  But in general?

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> Date: Thu, 05 Jan 2023 15:13:52 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: mardani29@yahoo.es, yantar92@posteo.net, 60568@debbugs.gnu.org
> 
> 
> >
> > Well, searching for the regexp "defun .*org-babel-execute:" would have 
> > done that, right?
> >
> 
> In this specific case, yes.  But in general?

I hope that in a more general case we could use something similar,
since every definition of a function should start from "defun "
followed by some part of its name, right?

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Jean Louis]

* Ihor Radchenko <yantar92@posteo.net> [2023-01-05 10:57]:
> Recently, we have stumbled upon a situation when showing function code
> is not only useful, but one of the few sane ways to examine it [3].
> Functions that are not directly written in the source but rather
> generated programmatically cannot currently be easily examined using
> built-in help functionality (using describe-function or other means).
> The help buffer only links to the (point-min) in the library defining
> the generated function - not very helpful. In contrast, helpful extracts
> function body from symbol function slot.

> Would it be possible to provide function body info via *Help* system in
> Emacs?

That would be helpful. 

When there is some on the fly defined function I have used
`symbol-function' function to extract data and save the function in the file for
later examination.

;;; SAVE FUNCTION

(defun save-function ()
  "Saves function at point"
  (interactive)
  (let ((function (function-called-at-point)))
    (when (and (symbolp function)
	       (y-or-n-p (format "Save `%s'" (symbol-name function))))
      (save-function-1 function))))

(defun save-function-1 (function)
  (let* ((fun (symbol-function function))
	 (type (car fun))
	 (file (concat rcd-temp-file-directory
		       "function-" (symbol-name function) ".el")))
    (cond ((eq type 'lambda) (setq fun (append (list 'defun function) (cdr fun))))
	  ((eq type 'closure) (setq fun (append (list 'defun function) (nthcdr 2 fun)))))
    (message (string-to-file-force (prin1-to-string fun) file))))

(defun save-function-delete ()
  (interactive)
  (save-function)
  (beginning-of-line)
  (mark-sexp)
  (delete-active-region))

Then I would get this result, for example:

  -rw-r--r--   1  668 Jan  5 19:54 function-hyperscope-add-new-note-hyperdocument.el

with body being:

(defun hyperscope-add-new-note-hyperdocument nil (interactive) (let* ((parent (hyperscope-set-parent-or-select)) (markup (hyperscope-markup-select)) (related-person-by-parent (if parent (progn (hyperscope-related-person-id parent)))) (related-person (if (and related-person-by-parent (y-or-n-p (format "Is Hyperdocument related to `%s'? " (cf-people-name related-person-by-parent)))) (progn related-person-by-parent))) (prompt (format "New `%s' hyperdocument name: " type-name)) (name (rcd-ask prompt)) (hyperdocument (hyperscope-add-generic name "" markup id nil parent nil nil nil related-person))) (hyperscope-ring hyperdocument) (hyperscope-revert hyperdocument)))

Which I know is not very readable, but is still something useful.

Thus the help buffer shall try to find function in source, but if it
is not there, it shall at least show it's definition by using
`symbol-function'.

Problem is with byte-compiled functions:

(symbol-function 'rcd-iso-date) ➜ #[0 "\300\301!\207" [format-time-string "%Y-%m-%d"] 2]

--
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

In support of Richard M. Stallman
https://stallmansupport.org/

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Jean Louis]

* Eli Zaretskii <eliz@gnu.org> [2023-01-05 11:11]:
> You basically ignore the valid objections voiced against such a
> feature, and present a specific case where (a) the source code is
> extremely short, and (b) the existing Help facilities are less than
> helpful because they have a deficiency.  How does it follow from the
> specific use case you present that we must show the source code in
> Help buffers?

I have understood that as a button, hyperlink to source, like

ignore is an interactive byte-compiled Lisp function in ‘subr.el’.

whereby ‘subr.el’ would be the button to the source, and if it can't
be found in the file, then temporary buffer may be displayed to tell
what or how function is defined, like by using `symbol-function'
function. 

--
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

In support of Richard M. Stallman
https://stallmansupport.org/

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Jean Louis]

* Eli Zaretskii <eliz@gnu.org> [2023-01-05 11:33]:
> > From: Ihor Radchenko <yantar92@posteo.net>
> > Cc: 60568@debbugs.gnu.org
> > Date: Thu, 05 Jan 2023 08:20:21 +0000
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > >> Would it be possible to provide function body info via *Help* system in
> > >> Emacs?
> > >
> > > Provide how?
> > 
> > For example, clicking source code link for function without definition
> > may display a new separate buffer with function body stored in the
> > function symbol.
> 
> That is less desirable, IMO, then teaching Help commands to be smarter
> with showing the source code.  One of the main reasons to go to the
> source code is to examine the surrounding code, and consider the
> function in its context.  This will be impossible with your proposal.

It would be better to find the place where generated functions have
been generated.

But not bad, to show how function looks like in same time.

1. Help buffer, with button link

2. Link goes to source, but if it does not find function, it shows
   where it was generated and pops up temporary buffer with function
   definition using `symbol-function'

--
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

In support of Richard M. Stallman
https://stallmansupport.org/

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Gregory Heytings]

[-- Attachment #1: Type: text/plain, Size: 459 bytes --]


>>> Well, searching for the regexp "defun .*org-babel-execute:" would have 
>>> done that, right?
>>
>> In this specific case, yes.  But in general?
>
> I hope that in a more general case we could use something similar, since 
> every definition of a function should start from "defun " followed by 
> some part of its name, right?
>

Not always, I can imagine functions defined with fset for example.

What about the attached patch?  It seems to work well.

[-- Attachment #2: Try-to-find-where-dynamically-defined-functions-were.patch --]
[-- Type: text/x-diff, Size: 5424 bytes --]

From 38d0941bb87c0b46f27eb4cf496f24bba071d982 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Thu, 5 Jan 2023 20:41:37 +0000
Subject: [PATCH] Try to find where dynamically defined functions were defined

* lisp/help-mode.el
(help-function-def--find-probable-definition-place): New function.
(help-function-def--button-function): Use it.  See bug#60568.
---
 lisp/help-mode.el | 94 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 93 insertions(+), 1 deletion(-)

diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index bf64d032b6..542c477789 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -252,6 +252,87 @@ 'help-customize-face
 		   (customize-face v))
   'help-echo (purecopy "mouse-2, RET: customize face"))
 
+(defun help-function-def--find-probable-definition-place (fun)
+  "Find the function in which FUN was likely defined.
+FUN is the body of a function, returned by `symbol-function'.
+The current buffer must be visiting the file in which the
+function was defined (see `symbol-file').
+If the function is found, return a cons with its name and its
+beginning position.
+Otherwise, return nil."
+  (save-excursion
+    ;; Build a list of strings with the symbols (and strings) of FUN.
+    (let ((names
+           (mapcar (lambda (el)
+                     (concat
+                      "\\_<"
+                      (regexp-quote (format "%s" el))
+                      "\\_>"))
+                   (flatten-tree (symbol-function fun))))
+          results)
+      (delete-dups names)
+      (when names
+        ;; Build an alist with all functions in which each symbol is
+        ;; found.
+        (dolist (el names)
+          ;; Exclude symbols that are 3 characters or less.
+          (when (> (length el) 9)
+            (let (result)
+              (goto-char (point-min))
+              (while (re-search-forward el nil t)
+                (push (lisp-current-defun-name) result))
+              (delete-dups result)
+              (when result
+                (push (cons el result) results)))))
+        (when results
+          ;; First check if one of the symbols is found in a single
+          ;; function.
+          (let ((el (catch 'found
+                      (mapcar
+                       (lambda (el)
+                         (if (= (length el) 2)
+                             (throw 'found (car el))))
+                       results))))
+            (if (stringp el)
+                ;; If one such function was found, it's the function
+                ;; we are after.
+                (progn
+                  (goto-char (point-min))
+                  (re-search-forward el nil t)
+                  (beginning-of-defun)
+                  (cons (lisp-current-defun-name) (point)))
+              ;; Otherwise, find which function contains the most
+              ;; symbols in FUN.
+              (let (places)
+                (dolist (el results)
+                  (dolist (e (cdr el))
+                    (push e places)))
+                (let* ((sorted-places (sort places #'string<))
+                       (ptr sorted-places)
+                       (count 1)
+                       (max 0)
+                       result)
+                  (while ptr
+                    (if (string= (car ptr) (cadr ptr))
+                        (setq count (1+ count))
+                      (when (> count max)
+                        (setq max count)
+                        (setq result (car ptr)))
+                      (setq count 1))
+                    (setq ptr (cdr ptr)))
+                  (when result
+                    ;; Determine the beginning position of that
+                    ;; function.
+                    (goto-char (point-min))
+                    (catch 'found
+                      (while (re-search-forward
+                              (concat "\\_<" (regexp-quote result) "\\_>"))
+                        (when (string= (lisp-current-defun-name)
+                                       result)
+                          (throw 'found nil))))
+                    (beginning-of-defun)
+                    (cons result (point))))))))))))
+
 (defun help-function-def--button-function (fun &optional file type)
   (or file
       (setq file (find-lisp-object-file-name fun type)))
@@ -281,7 +362,18 @@ help-function-def--button-function
             (unless (= (point) position)
               (push-mark nil t))
             (goto-char position))
-        (message "Unable to find location in file")))))
+        (let ((probable-definition-place
+               (help-function-def--find-probable-definition-place fun)))
+          (when probable-definition-place
+            (goto-char (cdr probable-definition-place)))
+	  (with-help-window (help-buffer)
+	    (insert (format "Function %s could not be found in %s.\n\n"
+                            fun (file-name-nondirectory file)))
+            (when probable-definition-place
+              (insert (format "It was probably defined by %s.\n\n"
+                              (car probable-definition-place))))
+	    (insert "Function definition:\n\n")
+	    (insert (pp-to-string (symbol-function fun)))))))))
 
 (define-button-type 'help-function-def
   :supertype 'help-xref
-- 
2.39.0

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> Date: Thu, 05 Jan 2023 20:44:21 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: yantar92@posteo.net, 60568@debbugs.gnu.org, mardani29@yahoo.es
> 
> > I hope that in a more general case we could use something similar, since 
> > every definition of a function should start from "defun " followed by 
> > some part of its name, right?
> >
> 
> Not always, I can imagine functions defined with fset for example.
> 
> What about the attached patch?  It seems to work well.

SGTM.  With which cases did you test this?

Adding Stefan, in case he has any comments.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Ihor Radchenko]

Jean Louis <bugs@gnu.support> writes:

> with body being:
>
> (defun hyperscope-add-new-note-hyperdocument nil (interactive) (let* ((parent (hyperscope-set-parent-or-select)) (markup (hyperscope-markup-select)) (related-person-by-parent (if parent (progn (hyperscope-related-person-id parent)))) (related-person (if (and related-person-by-parent (y-or-n-p (format "Is Hyperdocument related to `%s'? " (cf-people-name related-person-by-parent)))) (progn related-person-by-parent))) (prompt (format "New `%s' hyperdocument name: " type-name)) (name (rcd-ask prompt)) (hyperdocument (hyperscope-add-generic name "" markup id nil parent nil nil nil related-person))) (hyperscope-ring hyperdocument) (hyperscope-revert hyperdocument)))
>
> Which I know is not very readable, but is still something useful.

Try `pp' to auto-format the body.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Gregory Heytings]

[-- Attachment #1: Type: text/plain, Size: 529 bytes --]


>>> I hope that in a more general case we could use something similar, 
>>> since every definition of a function should start from "defun " 
>>> followed by some part of its name, right?
>>
>> Not always, I can imagine functions defined with fset for example.
>>
>> What about the attached patch?  It seems to work well.
>
> SGTM.  With which cases did you test this?
>
> Adding Stefan, in case he has any comments.
>

Mainly with the original recipe and some variants of it.

I attach a slightly improved version of the patch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Try-to-find-where-dynamically-defined-functions-were.patch --]
[-- Type: text/x-diff; name=Try-to-find-where-dynamically-defined-functions-were.patch, Size: 6329 bytes --]

From 841ad680dbbfa3a4a95a3b09a7409fcd9ce6cdd7 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Fri, 6 Jan 2023 08:54:34 +0000
Subject: [PATCH] Try to find where dynamically defined functions were defined

* lisp/help-mode.el
(help-function-def--find-probable-definition-place): New function.
(help-function-def--button-function): Use it.  See bug#60568.
---
 lisp/help-mode.el | 111 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 110 insertions(+), 1 deletion(-)

diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index bf64d032b6..15fd23c1cd 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -252,6 +252,91 @@ 'help-customize-face
 		   (customize-face v))
   'help-echo (purecopy "mouse-2, RET: customize face"))
 
+(defun help-function-def--find-probable-definition-place (fun)
+  "Find the function in which FUN was likely defined.
+FUN is the symbol of a function.
+The current buffer must be visiting the file in which the
+function was defined (see `symbol-file').
+If the function in which FUN was likely defined is found, return
+a cons with its name and its beginning position.
+Otherwise, return nil."
+  (save-excursion
+    ;; Build a list of strings with the symbols (and strings) of FUN.
+    (let ((names
+           (mapcar (lambda (el)
+                     (concat
+                      "\\_<"
+                      (regexp-quote (format "%s" el))
+                      "\\_>"))
+                   (flatten-tree (symbol-function fun))))
+          results)
+      (delete-dups names)
+      (when names
+        ;; Build an alist with all functions in which each symbol is
+        ;; found.
+        (dolist (el names)
+          ;; Exclude symbols that are 3 characters or less.
+          (when (> (length el) 9)
+            (let (result)
+              (goto-char (point-min))
+              (while (re-search-forward el nil t)
+                (push (lisp-current-defun-name) result))
+              (delete-dups result)
+              (when result
+                (push (cons el result) results)))))
+        (when results
+          ;; First check if one of the symbols is found in a single
+          ;; function.
+          (let ((el (catch 'found
+                      (progn
+                      (mapcar
+                       (lambda (el)
+                         (if (= (length el) 2)
+                             (throw 'found (car el))))
+                       results)))))
+            (if (stringp el)
+                ;; If one such function was found, it's the function
+                ;; we are after.
+                (progn
+                  (goto-char (point-min))
+                  (re-search-forward el nil t)
+                  (beginning-of-defun)
+                  (cons (lisp-current-defun-name) (point)))
+              ;; Otherwise, find which function contains the most
+              ;; symbols in FUN.
+              (let (places)
+                (dolist (el results)
+                  (dolist (e (cdr el))
+                    (push e places)))
+                (let* ((sorted-places (sort places #'string<))
+                       (ptr sorted-places)
+                       (count 1)
+                       (max 0)
+                       result)
+                  (while ptr
+                    (if (string= (car ptr) (cadr ptr))
+                        (setq count (1+ count))
+                      (when (> count max)
+                        (setq max count)
+                        (setq result (car ptr)))
+                      (setq count 1))
+                    (setq ptr (cdr ptr)))
+                  (when result
+                    ;; Determine the beginning position of that
+                    ;; function.
+                    (goto-char (point-min))
+                    (when (catch 'found
+                            (while (re-search-forward
+                                    (concat "\\_<"
+                                            (regexp-quote result)
+                                            "\\_>"))
+                              (when (string= (lisp-current-defun-name)
+                                             result)
+                                (throw 'found t)))
+                            (throw 'found nil))
+                      (beginning-of-defun)
+                      (cons result (point)))))))))))))
+
 (defun help-function-def--button-function (fun &optional file type)
   (or file
       (setq file (find-lisp-object-file-name fun type)))
@@ -281,7 +366,31 @@ help-function-def--button-function
             (unless (= (point) position)
               (push-mark nil t))
             (goto-char position))
-        (message "Unable to find location in file")))))
+        (let ((probable-definition-place
+               (help-function-def--find-probable-definition-place fun)))
+          (when probable-definition-place
+            (goto-char (cdr probable-definition-place)))
+          (let ((help-buffer-under-preparation t))
+            (help-setup-xref (list #'help-function-def--button-function
+                                   fun file)
+		             (called-interactively-p 'interactive))
+	    (with-help-window (help-buffer)
+	      (insert (substitute-command-keys
+                       (format "Function %s could not be found in `%s'.\n\n"
+                              fun (file-name-nondirectory file))))
+              (setq help-mode--current-data (list :symbol fun
+                                                  :file file))
+	      (save-excursion
+	        (re-search-backward
+                 (substitute-command-keys "`\\([^`']+\\)'")
+                 nil t)
+	        (help-xref-button 1 'help-function-def fun file))
+              (when probable-definition-place
+                (insert (substitute-command-keys
+                         (format "It was probably defined by `%s'.\n\n"
+                                (car probable-definition-place)))))
+	      (insert "Function definition:\n\n")
+	      (insert (pp-to-string (symbol-function fun))))))))))
 
 (define-button-type 'help-function-def
   :supertype 'help-xref
-- 
2.39.0

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> One of the discussed features was displaying function source code right
> in *Help* buffers. This feature usefulness have been objected at that
> time, on the grounds that showing function code may be too long and
> cause large *Help* buffers.

FWIW, I find myself regularly jumping to `M-x ielm` to look at the
`symbol-function`, so I would actually appreciate a button in the *Help*
buffer to display the actual value in the `symbol-function` slot.
This would also bring `describe-function` a bit closer to
`describe-variable`, which I think is good.

> 1. emacs -Q
> 2. M-: (require 'ob-shell)
> 3. <F1> f org-babel-execute:sh <RET>
> 4. Click on the source code link in *Help* buffer
> 5. Observe point jumping to (point-min) with no obvious way to find the
>    function definition.

We have `definition-name` for that.
I.e. `org-babel-shell-initialize` should arguably do

    (put 'org-babel-execute:sh 'definition-name 'org-babel-shell-initialize)

so that step 4 above jumps to `org-babel-shell-initialize`.

The patch below does that, along with saving some kittens.


        Stefan


diff --git a/lisp/org/ob-shell.el b/lisp/org/ob-shell.el
index 4a60186cd5d..5f7373c3faa 100644
--- a/lisp/org/ob-shell.el
+++ b/lisp/org/ob-shell.el
@@ -74,20 +74,25 @@ org-babel-shell-initialize
 is modified outside the Customize interface."
   (interactive)
   (dolist (name org-babel-shell-names)
-    (eval `(defun ,(intern (concat "org-babel-execute:" name))
-	       (body params)
-	     ,(format "Execute a block of %s commands with Babel." name)
-	     (let ((shell-file-name ,name)
-                   (org-babel-prompt-command
-                    (or (alist-get ,name org-babel-shell-set-prompt-commands)
-                        (alist-get t org-babel-shell-set-prompt-commands))))
-	       (org-babel-execute:shell body params))))
-    (eval `(defalias ',(intern (concat "org-babel-variable-assignments:" name))
-	     'org-babel-variable-assignments:shell
-	     ,(format "Return list of %s statements assigning to the block's \
+    (let ((fname (intern (concat "org-babel-execute:" name))))
+      (defalias fname
+        (lambda (body params)
+	  (:documentation
+           (format "Execute a block of %s commands with Babel." name))
+	  (let ((shell-file-name name)
+                (org-babel-prompt-command
+                 (or (alist-get name org-babel-shell-set-prompt-commands)
+                     (alist-get t org-babel-shell-set-prompt-commands))))
+	    (org-babel-execute:shell body params))))
+      (put fname 'definition-name 'org-babel-shell-initialize))
+    (defalias (intern (concat "org-babel-variable-assignments:" name))
+      #'org-babel-variable-assignments:shell
+      (format "Return list of %s statements assigning to the block's \
 variables."
-		      name)))
-    (eval `(defvar ,(intern (concat "org-babel-default-header-args:" name)) '()))))
+	      name))
+    (funcall (if (fboundp 'defvar-1) #'defvar-1 #'set) ;Emacs-29
+             (intern (concat "org-babel-default-header-args:" name))
+             '())))
 
 (defcustom org-babel-shell-names
   '("sh" "bash" "zsh" "fish" "csh" "ash" "dash" "ksh" "mksh" "posh")

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> Not always, I can imagine functions defined with fset for example.

I'd consider that an error.
`defalias` on the other hand is indeed a normal and common case.

>>> What about the attached patch?  It seems to work well.
>> SGTM.  With which cases did you test this?
>> Adding Stefan, in case he has any comments.

SGTM as well.  I do have some comments, see below.

> +(defun help-function-def--find-probable-definition-place (fun)

Can we shorten the name using "guess" i.s.o "find-probable"?
Also, this should be in `find-func.el` rather than in `help-mode.el`, no?

> +  (save-excursion
> +    ;; Build a list of strings with the symbols (and strings) of FUN.
> +    (let ((names
> +           (mapcar (lambda (el)
> +                     (concat
> +                      "\\_<"
> +                      (regexp-quote (format "%s" el))

I think you want "%S" rather than "%s".

> +                      "\\_>"))
> +                   (flatten-tree (symbol-function fun))))

- We probably want to use `advice-cd*r` so as to flatten the actual
  function definition rather than the combination of its definition with
  the various pieces of advice that are currently applied.
- The above works for non-compiled functions but for byte-compiled
  functions we need an ad-hoc version of `flatten-tree` which extracts
  the constants from the constant vector of bytecode objects (also
  recursively since bytecode objects tend to contain other bytecode
  objects).
  For native-compiled functions, we may be able to get some kind of
  "constant vector" as well, but I don't think that's currently
  accessible from ELisp.  We should ask Andrea.

> +          ;; Exclude symbols that are 3 characters or less.
> +          (when (> (length el) 9)

If we filter them out when constructing `names`, we can use a comment
where the number matches the number used in the code :-)

> +                    ;; Determine the beginning position of that
> +                    ;; function.
> +                    (goto-char (point-min))
> +                    (when (catch 'found
> +                            (while (re-search-forward
> +                                    (concat "\\_<"
> +                                            (regexp-quote result)
> +                                            "\\_>"))
> +                              (when (string= (lisp-current-defun-name)
> +                                             result)
> +                                (throw 'found t)))
> +                            (throw 'found nil))
> +                      (beginning-of-defun)
> +                      (cons result (point)))))))))))))

Can't we use `find-function-noselect` here?  Or maybe just return the
function name and let the caller then use `find-function-noselect`.

> @@ -281,7 +366,31 @@ help-function-def--button-function
>              (unless (= (point) position)
>                (push-mark nil t))
>              (goto-char position))
> -        (message "Unable to find location in file")))))
> +        (let ((probable-definition-place
> +               (help-function-def--find-probable-definition-place fun)))
> +          (when probable-definition-place
> +            (goto-char (cdr probable-definition-place)))
> +          (let ((help-buffer-under-preparation t))
> +            (help-setup-xref (list #'help-function-def--button-function
> +                                   fun file)
> +		             (called-interactively-p 'interactive))
> +	    (with-help-window (help-buffer)
> +	      (insert (substitute-command-keys
> +                       (format "Function %s could not be found in `%s'.\n\n"
> +                              fun (file-name-nondirectory file))))
> +              (setq help-mode--current-data (list :symbol fun
> +                                                  :file file))
> +	      (save-excursion
> +	        (re-search-backward
> +                 (substitute-command-keys "`\\([^`']+\\)'")
> +                 nil t)
> +	        (help-xref-button 1 'help-function-def fun file))
> +              (when probable-definition-place
> +                (insert (substitute-command-keys
> +                         (format "It was probably defined by `%s'.\n\n"
> +                                (car probable-definition-place)))))
> +	      (insert "Function definition:\n\n")
> +	      (insert (pp-to-string (symbol-function fun))))))))))

Please move this to a separate function.

Also, I'm not completely convinced what you do with the *Help* buffer
here is the UI we'll want, really.  But I don't have a good idea for
what to replace it with (yet), so we can start with that and tweak
it later.


        Stefan

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Gregory Heytings]

>>>> What about the attached patch?  It seems to work well.
>>>
>>> SGTM.  With which cases did you test this?
>>> Adding Stefan, in case he has any comments.
>
> SGTM as well.
>

TBH, I'm not sure this is useful anymore, if the 'definition-name' slot is 
supposed to be used for such functions (which I didn't know).  WDYT?

>> +(defun help-function-def--find-probable-definition-place (fun)
>
> Can we shorten the name using "guess" i.s.o "find-probable"?
>
> Also, this should be in `find-func.el` rather than in `help-mode.el`, 
> no?
>

Yes and yes.

>> +                   (flatten-tree (symbol-function fun))))
>
> - We probably want to use `advice-cd*r` so as to flatten the actual 
> function definition rather than the combination of its definition with 
> the various pieces of advice that are currently applied.
>

You mean, (flatten-tree (advice--cd*r (symbol-function fun))), right?

>
> - The above works for non-compiled functions but for byte-compiled 
> functions we need an ad-hoc version of `flatten-tree` which extracts the 
> constants from the constant vector of bytecode objects (also recursively 
> since bytecode objects tend to contain other bytecode objects).
>
> For native-compiled functions, we may be able to get some kind of 
> "constant vector" as well, but I don't think that's currently accessible 
> from ELisp.  We should ask Andrea.
>

Are such dynamically defined functions not always non-compiled, by 
definition?  At least with the original recipe in this feature request, 
(symbol-file 'org-babel-execute:sh) returns "lisp/org/ob-shell.elc", so it 
has been created by a byte-compiled function.

>> +          ;; Exclude symbols that are 3 characters or less.
>> +          (when (> (length el) 9)
>
> If we filter them out when constructing `names`, we can use a comment 
> where the number matches the number used in the code :-)
>

Oh yes ;-)

>> +                    ;; Determine the beginning position of that
>> +                    ;; function.
>> +                    (goto-char (point-min))
>> +                    (when (catch 'found
>> +                            (while (re-search-forward
>> +                                    (concat "\\_<"
>> +                                            (regexp-quote result)
>> +                                            "\\_>"))
>> +                              (when (string= (lisp-current-defun-name)
>> +                                             result)
>> +                                (throw 'found t)))
>> +                            (throw 'found nil))
>> +                      (beginning-of-defun)
>> +                      (cons result (point)))))))))))))
>
> Can't we use `find-function-noselect` here?  Or maybe just return the 
> function name and let the caller then use `find-function-noselect`.
>

Indeed!

>> @@ -281,7 +366,31 @@ help-function-def--button-function
>>              (unless (= (point) position)
>>                (push-mark nil t))
>>              (goto-char position))
>> -        (message "Unable to find location in file")))))
>> +        (let ((probable-definition-place
>> +               (help-function-def--find-probable-definition-place fun)))
>> +          (when probable-definition-place
>> +            (goto-char (cdr probable-definition-place)))
>> +          (let ((help-buffer-under-preparation t))
>> +            (help-setup-xref (list #'help-function-def--button-function
>> +                                   fun file)
>> +		             (called-interactively-p 'interactive))
>> +	    (with-help-window (help-buffer)
>> +	      (insert (substitute-command-keys
>> +                       (format "Function %s could not be found in `%s'.\n\n"
>> +                              fun (file-name-nondirectory file))))
>> +              (setq help-mode--current-data (list :symbol fun
>> +                                                  :file file))
>> +	      (save-excursion
>> +	        (re-search-backward
>> +                 (substitute-command-keys "`\\([^`']+\\)'")
>> +                 nil t)
>> +	        (help-xref-button 1 'help-function-def fun file))
>> +              (when probable-definition-place
>> +                (insert (substitute-command-keys
>> +                         (format "It was probably defined by `%s'.\n\n"
>> +                                (car probable-definition-place)))))
>> +	      (insert "Function definition:\n\n")
>> +	      (insert (pp-to-string (symbol-function fun))))))))))
>
> Please move this to a separate function.
>

Sure.

>
> Also, I'm not completely convinced what you do with the *Help* buffer 
> here is the UI we'll want, really.  But I don't have a good idea for 
> what to replace it with (yet), so we can start with that and tweak it 
> later.
>

Okay.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>>>> What about the attached patch?  It seems to work well.
>>>> SGTM.  With which cases did you test this?
>>>> Adding Stefan, in case he has any comments.
>> SGTM as well.
> TBH, I'm not sure this is useful anymore, if the 'definition-name' slot is
> supposed to be used for such functions (which I didn't know).  WDYT?

If you look at the code of `find-func.el` you'll see that we already
rely on heuristics, so it seems like a natural complement.
But I agree that `definition-name` is usually preferable since it's more
reliable.  Still most people don't know `definition-name`, so it can
still be helpful.

>> - We probably want to use `advice-cd*r` so as to flatten the actual
>> function definition rather than the combination of its definition with the
>> various pieces of advice that are currently applied.
> You mean, (flatten-tree (advice--cd*r (symbol-function fun))), right?

Yes.

>> - The above works for non-compiled functions but for byte-compiled
>> functions we need an ad-hoc version of `flatten-tree` which extracts the
>> constants from the constant vector of bytecode objects (also recursively
>> since bytecode objects tend to contain other bytecode objects).
>>
>> For native-compiled functions, we may be able to get some kind of
>> "constant vector" as well, but I don't think that's currently accessible
>> from ELisp.  We should ask Andrea.
>
> Are such dynamically defined functions not always non-compiled, by
> definition?

No.  They can be defined by macros rather than functions in which case
they'll usually be compiled.  Or they may be closures defined locally
within another defun, in which case again they'll usually be compiled.
[ E.g. with the patch to `ob-shell.el` that I sent, the
`org-babel-execute:sh` is byte-compiled.  ]

> At least with the original recipe in this feature request,
> (symbol-file 'org-babel-execute:sh) returns "lisp/org/ob-shell.elc", so it
> has been created by a byte-compiled function.

The `ob-shell.el` code in master creates the functions at run time via
`eval`, so they're created while loading `ob-shell.elc` but
they're interpreted.  With my patch, they're just closures so their body
is compiled when `ob-shell.el` is compiled.


        Stefan

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 60568@debbugs.gnu.org,  Eli Zaretskii <eliz@gnu.org>,
>   yantar92@posteo.net,  mardani29@yahoo.es
> Date: Fri, 06 Jan 2023 19:36:04 -0500
> 
> >>>>> What about the attached patch?  It seems to work well.
> >>>> SGTM.  With which cases did you test this?
> >>>> Adding Stefan, in case he has any comments.
> >> SGTM as well.
> > TBH, I'm not sure this is useful anymore, if the 'definition-name' slot is
> > supposed to be used for such functions (which I didn't know).  WDYT?
> 
> If you look at the code of `find-func.el` you'll see that we already
> rely on heuristics, so it seems like a natural complement.
> But I agree that `definition-name` is usually preferable since it's more
> reliable.  Still most people don't know `definition-name`, so it can
> still be helpful.

Maybe people will know better if definition-name would be documented.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Gregory Heytings]

>> If you look at the code of `find-func.el` you'll see that we already 
>> rely on heuristics, so it seems like a natural complement. But I agree 
>> that `definition-name` is usually preferable since it's more reliable. 
>> Still most people don't know `definition-name`, so it can still be 
>> helpful.
>
> Maybe people will know better if definition-name would be documented.
>

I think the place where it should be documented is the suggested *Help* 
screen, with a sentence like "To ensure an accurate identification of the 
place where `frobnicate' was defined, the definition-name slot of the 
`frobnicate' symbol should be set ."

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Ihor Radchenko]

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

>> 1. emacs -Q
>> 2. M-: (require 'ob-shell)
>> 3. <F1> f org-babel-execute:sh <RET>
>> 4. Click on the source code link in *Help* buffer
>> 5. Observe point jumping to (point-min) with no obvious way to find the
>>    function definition.
>
> We have `definition-name` for that.
> I.e. `org-babel-shell-initialize` should arguably do
>
>     (put 'org-babel-execute:sh 'definition-name 'org-babel-shell-initialize)
>
> so that step 4 above jumps to `org-babel-shell-initialize`.
>
> The patch below does that, along with saving some kittens.

Thanks!

> +    (funcall (if (fboundp 'defvar-1) #'defvar-1 #'set) ;Emacs-29
> +             (intern (concat "org-babel-default-header-args:" name))
> +             '())))

Why '()?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> If you look at the code of `find-func.el` you'll see that we already
>> rely on heuristics, so it seems like a natural complement.
>> But I agree that `definition-name` is usually preferable since it's more
>> reliable.  Still most people don't know `definition-name`, so it can
>> still be helpful.
>
> Maybe people will know better if definition-name would be documented.

May it also be a declare form?
Or is there an exhaustive list of symbol properties used by Emacs?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Ihor Radchenko]

Gregory Heytings <gregory@heytings.org> writes:

>> Maybe people will know better if definition-name would be documented.
>>
>
> I think the place where it should be documented is the suggested *Help* 
> screen, with a sentence like "To ensure an accurate identification of the 
> place where `frobnicate' was defined, the definition-name slot of the 
> `frobnicate' symbol should be set ."

But is it of interest for _users_? I'd rather see it in Elisp manual.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, gregory@heytings.org,
>  60568@debbugs.gnu.org, mardani29@yahoo.es
> Date: Sat, 07 Jan 2023 13:14:56 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> If you look at the code of `find-func.el` you'll see that we already
> >> rely on heuristics, so it seems like a natural complement.
> >> But I agree that `definition-name` is usually preferable since it's more
> >> reliable.  Still most people don't know `definition-name`, so it can
> >> still be helpful.
> >
> > Maybe people will know better if definition-name would be documented.
> 
> May it also be a declare form?
> Or is there an exhaustive list of symbol properties used by Emacs?

See the node "Standard Properties" in the ELisp manual.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>,
>  60568@debbugs.gnu.org, mardani29@yahoo.es
> Date: Sat, 07 Jan 2023 13:38:22 +0000
> 
> Gregory Heytings <gregory@heytings.org> writes:
> 
> >> Maybe people will know better if definition-name would be documented.
> >>
> >
> > I think the place where it should be documented is the suggested *Help* 
> > screen, with a sentence like "To ensure an accurate identification of the 
> > place where `frobnicate' was defined, the definition-name slot of the 
> > `frobnicate' symbol should be set ."
> 
> But is it of interest for _users_? I'd rather see it in Elisp manual.

Yes, IMO it should be documented in the ELisp manual.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Gregory Heytings]

>>> Maybe people will know better if definition-name would be documented.
>>
>> I think the place where it should be documented is the suggested *Help* 
>> screen, with a sentence like "To ensure an accurate identification of 
>> the place where `frobnicate' was defined, the definition-name slot of 
>> the `frobnicate' symbol should be set."
>
> But is it of interest for _users_? I'd rather see it in Elisp manual.
>

It should be documented in the Elisp manual, indeed.  What I said was a 
bit unclear: I meant that a reminder should be displayed in the *Help* 
buffer.  With this, Elisp programmers who are not aware of that subtlety 
would become aware of it.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Ihor Radchenko]

Gregory Heytings <gregory@heytings.org> writes:

>>
>> But is it of interest for _users_? I'd rather see it in Elisp manual.
>>
>
> It should be documented in the Elisp manual, indeed.  What I said was a 
> bit unclear: I meant that a reminder should be displayed in the *Help* 
> buffer.  With this, Elisp programmers who are not aware of that subtlety 
> would become aware of it.

Then, may "13.4 Defining Functions" be a good place to link to this
subtlety?

The documentation for `defalias' is not really very clear, even now,
when I am aware about the issue herein:

     The proper place to use ‘defalias’ is where a specific function or
     macro name is being defined—especially where that name appears
     explicitly in the source file being loaded.  This is because
     ‘defalias’ records which file defined the function, just like
     ‘defun’ (*note Unloading::).

     By contrast, in programs that manipulate function definitions for
     other purposes, it is better to use ‘fset’, which does not keep
     such records.  *Note Function Cells::.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>,
>  60568@debbugs.gnu.org, mardani29@yahoo.es
> Date: Sat, 07 Jan 2023 15:07:33 +0000
> 
> Gregory Heytings <gregory@heytings.org> writes:
> 
> >>
> >> But is it of interest for _users_? I'd rather see it in Elisp manual.
> >>
> >
> > It should be documented in the Elisp manual, indeed.  What I said was a 
> > bit unclear: I meant that a reminder should be displayed in the *Help* 
> > buffer.  With this, Elisp programmers who are not aware of that subtlety 
> > would become aware of it.
> 
> Then, may "13.4 Defining Functions" be a good place to link to this
> subtlety?
> 
> The documentation for `defalias' is not really very clear, even now,
> when I am aware about the issue herein:
> 
>      The proper place to use ‘defalias’ is where a specific function or
>      macro name is being defined—especially where that name appears
>      explicitly in the source file being loaded.  This is because
>      ‘defalias’ records which file defined the function, just like
>      ‘defun’ (*note Unloading::).
> 
>      By contrast, in programs that manipulate function definitions for
>      other purposes, it is better to use ‘fset’, which does not keep
>      such records.  *Note Function Cells::.

FWIW, I don't see anything unclear here.  Moreover, what does defalias
have to do with the issue of links to code of generated functions?

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> Then, may "13.4 Defining Functions" be a good place to link to this
>> subtlety?
>> 
>> The documentation for `defalias' is not really very clear, even now,
>> when I am aware about the issue herein:
>> 
>>      The proper place to use ‘defalias’ is where a specific function or
>>      macro name is being defined—especially where that name appears
>>      explicitly in the source file being loaded.  This is because
>>      ‘defalias’ records which file defined the function, just like
>>      ‘defun’ (*note Unloading::).
>> 
>>      By contrast, in programs that manipulate function definitions for
>>      other purposes, it is better to use ‘fset’, which does not keep
>>      such records.  *Note Function Cells::.
>
> FWIW, I don't see anything unclear here.  Moreover, what does defalias
> have to do with the issue of links to code of generated functions?

The above says that `defalias' "records which file defined the
function". It appears to be related to what we are discussing here -
locating the function definition.

However, the present manual description does not talk what "records"
refers to. I presume that the records are to be used by *Help* system.
If so, it also makes sense to elaborate. Either here, or in other node
linked from here.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: gregory@heytings.org, monnier@iro.umontreal.ca, 60568@debbugs.gnu.org,
>  mardani29@yahoo.es
> Date: Sat, 07 Jan 2023 15:19:12 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Then, may "13.4 Defining Functions" be a good place to link to this
> >> subtlety?
> >> 
> >> The documentation for `defalias' is not really very clear, even now,
> >> when I am aware about the issue herein:
> >> 
> >>      The proper place to use ‘defalias’ is where a specific function or
> >>      macro name is being defined—especially where that name appears
> >>      explicitly in the source file being loaded.  This is because
> >>      ‘defalias’ records which file defined the function, just like
> >>      ‘defun’ (*note Unloading::).
> >> 
> >>      By contrast, in programs that manipulate function definitions for
> >>      other purposes, it is better to use ‘fset’, which does not keep
> >>      such records.  *Note Function Cells::.
> >
> > FWIW, I don't see anything unclear here.  Moreover, what does defalias
> > have to do with the issue of links to code of generated functions?
> 
> The above says that `defalias' "records which file defined the
> function". It appears to be related to what we are discussing here -
> locating the function definition.

Tangentially at best.

> However, the present manual description does not talk what "records"
> refers to. I presume that the records are to be used by *Help* system.
> If so, it also makes sense to elaborate. Either here, or in other node
> linked from here.

We must find a better place.

And the main problem is not where to document this (its natural place
is where all the standard properties are described), the main problem
is how to index it so that people could easily find it.  Also, where
else to mention that property in the manual, in related context.  We
don't seem to have anything about generating code...

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> The patch below does that, along with saving some kittens.
> Thanks!

As always, my patch is 100% untested.

>> +    (funcall (if (fboundp 'defvar-1) #'defvar-1 #'set) ;Emacs-29
>> +             (intern (concat "org-babel-default-header-args:" name))
>> +             '())))
>
> Why '()?

No idea.  I just tried to preserve the part of the code I didn't feel
like changing.


        Stefan

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Eli Zaretskii]

> Cc: gregory@heytings.org, mardani29@yahoo.es, monnier@iro.umontreal.ca,
>  60568@debbugs.gnu.org
> Date: Sat, 07 Jan 2023 17:23:01 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > However, the present manual description does not talk what "records"
> > refers to. I presume that the records are to be used by *Help* system.
> > If so, it also makes sense to elaborate. Either here, or in other node
> > linked from here.
> 
> We must find a better place.
> 
> And the main problem is not where to document this (its natural place
> is where all the standard properties are described), the main problem
> is how to index it so that people could easily find it.  Also, where
> else to mention that property in the manual, in related context.  We
> don't seem to have anything about generating code...

I believe I found a good place for that, please take a look at the
emacs-29 branch.

bug#60568: [FR] 30.0.50; Help buffers and function bodies for generated functions

[Ihor Radchenko]

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

>>> The patch below does that, along with saving some kittens.
>> Thanks!
>
> As always, my patch is 100% untested.
>
>>> +    (funcall (if (fboundp 'defvar-1) #'defvar-1 #'set) ;Emacs-29
>>> +             (intern (concat "org-babel-default-header-args:" name))
>>> +             '())))
>>
>> Why '()?
>
> No idea.  I just tried to preserve the part of the code I didn't feel
> like changing.

I went with "nil".

Applied, onto main. All the tests are passing.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=ecb62e2e3

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>