describe-key vs. widget red tape

describe-key vs. widget red tape

[jidanni]

What will happen when I hit RET or click this mouse button here, I
asked myself. Let's use trusty C-h k (describe-key).

Nope. There's an extra layer of indirection involved apparently,
and describe-key is not willing to cut through the red tape and give
the ultimate answer along with its usual answer.

    <down-mouse-1> at that spot runs the command widget-button-click
      which is an interactive compiled Lisp function in `wid-edit.el'.
    It is bound to <down-mouse-1>, <down-mouse-2>.
    (widget-button-click EVENT)

    Invoke the button that the mouse is pointing at.


    RET runs the command widget-button-press
      which is an interactive compiled Lisp function in `wid-edit.el'.
    It is bound to RET.
    (widget-button-press POS &optional EVENT)

    Invoke button at POS.

Nope. The only way to find out what will happen is to fasten one's
seat belts and hit or click and find out.

And even if there is a different incantation to find out, describe-key
should also cough up the ultimate answer too.

bug#139: describe-key vs. widget red tape

[Lars Ingebrigtsen]

jidanni@jidanni.org writes:

> What will happen when I hit RET or click this mouse button here, I
> asked myself. Let's use trusty C-h k (describe-key).
>
> Nope. There's an extra layer of indirection involved apparently,
> and describe-key is not willing to cut through the red tape and give
> the ultimate answer along with its usual answer.
>
>     <down-mouse-1> at that spot runs the command widget-button-click
>       which is an interactive compiled Lisp function in `wid-edit.el'.
>     It is bound to <down-mouse-1>, <down-mouse-2>.
>     (widget-button-click EVENT)

[...]

> Nope. The only way to find out what will happen is to fasten one's
> seat belts and hit or click and find out.
>
> And even if there is a different incantation to find out, describe-key
> should also cough up the ultimate answer too.

I don't think there's any way that could work in general.  Emacs knows
what down-mouse-1 calls, but it doesn't know about what that function in
turn does.

We could teach describe-key about certain common constructs, like
widgets and buttons, but that would be very ad hoc indeed, I think?

So I'm closing this bug report.  If somebody else feels that
describe-key should start introspection of widgets/buttons, feel free to
reopen.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#139: describe-key vs. widget red tape

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Mon, 30 Sep 2019 07:56:58 +0200
> Cc: 139@debbugs.gnu.org
> 
> >     <down-mouse-1> at that spot runs the command widget-button-click
> >       which is an interactive compiled Lisp function in `wid-edit.el'.
> >     It is bound to <down-mouse-1>, <down-mouse-2>.
> >     (widget-button-click EVENT)
> 
> [...]
> 
> > Nope. The only way to find out what will happen is to fasten one's
> > seat belts and hit or click and find out.
> >
> > And even if there is a different incantation to find out, describe-key
> > should also cough up the ultimate answer too.
> 
> I don't think there's any way that could work in general.  Emacs knows
> what down-mouse-1 calls, but it doesn't know about what that function in
> turn does.
> 
> We could teach describe-key about certain common constructs, like
> widgets and buttons, but that would be very ad hoc indeed, I think?

If the action of the button is not clear from the button's label, it
should have a help-echo string which describes that.

bug#139: describe-key vs. widget red tape

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> We could teach describe-key about certain common constructs, like
>> widgets and buttons, but that would be very ad hoc indeed, I think?
>
> If the action of the button is not clear from the button's label, it
> should have a help-echo string which describes that.

What the button does may be clear, but it can be somewhat convoluted to
determine precisely what function is going to end up being run.  (This
is for introspection/debugging purposes.) 

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#139: describe-key vs. widget red tape

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: jidanni@jidanni.org,  139@debbugs.gnu.org
> Date: Mon, 30 Sep 2019 09:40:07 +0200
> 
> > If the action of the button is not clear from the button's label, it
> > should have a help-echo string which describes that.
> 
> What the button does may be clear, but it can be somewhat convoluted to
> determine precisely what function is going to end up being run.  (This
> is for introspection/debugging purposes.) 

I'm not sure introspection/debugging was what the OP had in mind, but
we could perhaps bind something like M-mouse-3 on a button to produce
that information.  It wouldn't be easy, though, given how well the
widget library hides that.

bug#139: describe-key vs. widget red tape

[Mauro Aranda]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Cc: jidanni@jidanni.org,  139@debbugs.gnu.org
>> Date: Mon, 30 Sep 2019 09:40:07 +0200
>>
>> > If the action of the button is not clear from the button's label, it
>> > should have a help-echo string which describes that.
>>
>> What the button does may be clear, but it can be somewhat convoluted to
>> determine precisely what function is going to end up being run.  (This
>> is for introspection/debugging purposes.)
>
> I'm not sure introspection/debugging was what the OP had in mind, but
> we could perhaps bind something like M-mouse-3 on a button to produce
> that information.  It wouldn't be easy, though, given how well the
> widget library hides that.

I took a look.  Perhaps we could start with something like the attached?
The file defines a command for describing the actions associated to a
widget.  One common indirection is that a widget's action might be to
tell its parent to run its action, so I tried to solve that indirection.

To test it, load it, put point on a widget and then type:
M-x widget-describe-actions

If that's a good start, I'm willing to work on this feature request.

Best regards,
Mauro.

[-- Attachment #2: Type: text/html, Size: 1629 bytes --]

bug#139: describe-key vs. widget red tape

[Mauro Aranda]

[-- Attachment #1.1: Type: text/plain, Size: 16 bytes --]

Here's the file

[-- Attachment #1.2: Type: text/html, Size: 45 bytes --]

[-- Attachment #2: widget-describe-actions.el --]
[-- Type: text/x-emacs-lisp, Size: 2537 bytes --]

;;; widget-describe-actions.el --- Example of describing widget actions -*- lexical-binding:t -*-

(require 'wid-edit)
(require 'wid-browse)

(defun widget-describe-actions (&optional widget-or-pos)
  "Describe the actions associated to the widget at point.
Displays a buffer with a description about the actions, as well as a link to
browse all the properties of the widget.
This command resolves the indirection of widgets running the action of its
parents, so the real action executed can be known.

When called from Lisp, pass WIDGET-OR-POS as the widget to describe,
or a buffer position where a widget is present.  If WIDGET-OR-POS is nil,
the widget at point is the widget to describe."
  (interactive "d")
  (let ((widget (if (widgetp widget-or-pos)
		    widget-or-pos
		  (widget-at (or widget-or-pos (point)))))
	(inhibit-read-only t) ; For erasing the contents of the buffer.
	(map (make-sparse-keymap)) ; We will change the binding of `q'.
	action mouse-down-action)
    (when widget
      (setq action (widget-resolve-parent-action widget)
	    mouse-down-action (widget-get widget :mouse-down-action))
      (pop-to-buffer "*Widget Actions*" nil t)
      (erase-buffer)
      (widget-browse-mode)
      ;; In order to be more like a *Help* buffer, use quit-window instead
      ;; of bury-buffer.
      (set-keymap-parent map widget-browse-mode-map)
      (define-key map "q" 'quit-window)
      (use-local-map map)
      (widget-insert "This widget type is ")
      (widget-create 'widget-browse :format "%[%v%]\n%d"
		     :doc (get (car widget) 'widget-documentation)
		     widget)
      (widget-insert "\n")
      (when (symbolp action)
	(widget-insert (propertize "Action" 'face 'bold))
	(widget-insert "\nThe action of this widget is ")
	(widget-create 'function-link :value action)
	(widget-insert "\n\n"))
      (when (symbolp mouse-down-action)
	(widget-insert (propertize "Mouse-down-action" 'face 'bold))
	(widget-insert "\nThe mouse-down-action of this widget is ")
	(widget-create 'function-link :value mouse-down-action)
	(widget-insert "\n"))
      (widget-setup)
      (goto-char (point-min)))))

(defun widget-resolve-parent-action (widget)
  "If action of WIDGET is `widget-parent-action', find out what would that be."
  (let ((action (widget-get widget :action))
	(parent (widget-get widget :parent)))
    (while (and (eq action 'widget-parent-action)
		(setq parent (widget-get parent :parent)))
      (setq action (widget-get parent :action)))
    action))
      
(provide 'widget-describe-actions)

bug#139: describe-key vs. widget red tape

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> I took a look.  Perhaps we could start with something like the attached?
> The file defines a command for describing the actions associated to a
> widget.  One common indirection is that a widget's action might be to
> tell its parent to run its action, so I tried to solve that indirection.
>
> To test it, load it, put point on a widget and then type:
> M-x widget-describe-actions
>
> If that's a good start, I'm willing to work on this feature request.

Great!

I tried it on a random button, and I got:

---
This widget type is push-button
A pushable button.

Action
The action of this widget is [custom-reset]

Mouse-down-action
The mouse-down-action of this widget is [ignore]
---

Perhaps more traditional would be to make the links `custom-reset'
instead of [custom-reset]?  Otherwise looks great to me, and I'm
reopening this bug report.

Could this also be extended to button.el buttons?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#139: describe-key vs. widget red tape

[Mauro Aranda]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I tried it on a random button, and I got:
>
> ---
> This widget type is push-button
> A pushable button.
>
> Action
> The action of this widget is [custom-reset]
>
> Mouse-down-action
> The mouse-down-action of this widget is [ignore]
> ---
>
> Perhaps more traditional would be to make the links `custom-reset'
> instead of [custom-reset]?

That should be easy to change.  I'll do it.

> Could this also be extended to button.el buttons?

I think so.  I'll think how to put it all together and come back with a
patch.  (Will be busy the next few days, but perhaps by the weekend I'll
be able to send it)

Best regards,
Mauro.

[-- Attachment #2: Type: text/html, Size: 885 bytes --]

bug#139: describe-key vs. widget red tape

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

>> Could this also be extended to button.el buttons?
>
> I think so.  I'll think how to put it all together and come back with a
> patch.  (Will be busy the next few days, but perhaps by the weekend I'll
> be able to send it)

Great!

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#139: describe-key vs. widget red tape

[Mauro Aranda]

[-- Attachment #1.1: Type: text/plain, Size: 1235 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>>> Could this also be extended to button.el buttons?
>>
>> I think so.  I'll think how to put it all together and come back with a
>> patch.  (Will be busy the next few days, but perhaps by the weekend I'll
>> be able to send it)
>
> Great!

Find in the attached patch my idea to implement this.

I wrote a command, describe-actions, so the user does not have to care
if the element to describe is either a button, a widget, or whatever.
And then, the libraries that define this kind of elements should add
themselves to `describe-actions-functions'.

Then I implemented the functions for the button.el buttons and the
widgets.

There are some things I would like to ask:
- I would like to put an initial value of nil to
`describe-actions-functions', and let each library add the function to
this list.  Is it OK to use `with-eval-after-load' here? Is there
another way?

- I put an autoload cookie in `seq-find', in order to use it in
`describe-actions'.  Is it OK?

Please review the patch and send any suggestions for improvement.  If
accepted, I'll write the documentation needed for the NEWS file and the
manual.

Best regards,
Mauro.

[-- Attachment #1.2: Type: text/html, Size: 1557 bytes --]

[-- Attachment #2: 0001-New-command-describe-actions-Bug-139.patch --]
[-- Type: text/x-patch, Size: 9454 bytes --]

From 314ad932333f6c34143d7e388809003c7b424316 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Fri, 4 Oct 2019 13:58:11 -0300
Subject: [PATCH] New command describe-actions (Bug#139)

* lisp/help-fns.el (describe-actions-functions): New variable, to use
in describe-actions.
(describe-actions): New command, to describe the actions of an
element, like a button or widget.
* lisp/emacs-lisp/seq.el (seq-find): Add autoload cookie, so
describe-actions can use it.
* lisp/button.el (button-describe-actions): New command, to describe
the actions of a button.
(button--describe-actions-internal): Helper function for
button-describe-action.
* lisp/wid-edit.el (widget-describe-actions): New command, to describe
the actions of a widget.
(widget-resolve-parent-action): Helper function, to allow
widget-describe-actions show more useful information.
---
 lisp/button.el         | 58 +++++++++++++++++++++++++++++++++++++++++++++
 lisp/emacs-lisp/seq.el |  1 +
 lisp/help-fns.el       | 42 +++++++++++++++++++++++++++++++++
 lisp/wid-edit.el       | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 165 insertions(+)

diff --git a/lisp/button.el b/lisp/button.el
index 04e77ca..66bc12f 100644
--- a/lisp/button.el
+++ b/lisp/button.el
@@ -538,6 +538,64 @@ backward-button
   (interactive "p\nd\nd")
   (forward-button (- n) wrap display-message no-error))
 
+(defun button--describe-actions-internal (type action mouse-down-action)
+  "Describe a button's TYPE, ACTION and MOUSE-DOWN-ACTION in a *Help* buffer.
+This is a helper function for `button-describe-actions', in order to be possible
+to use `help-setup-xref'."
+  (with-help-window (help-buffer)
+    (with-current-buffer (help-buffer)
+      (insert "This button's type is ")
+      (princ type)
+      (insert "\n\n")
+      (when (functionp action)
+	(insert (propertize "Action" 'face 'bold))
+	(insert "\nThe action of this button is ")
+	(if (symbolp action)
+	    (progn
+	      (princ action)
+	      (insert ",\nwhich is ")
+	      (describe-function-1 action))
+	  (insert "\n")
+	  (princ action)))
+      (when (functionp mouse-down-action)
+	(insert (propertize "Mouse-down-action" 'face 'bold))
+	(insert "\nThe mouse-down-action of this button is ")
+	(if (symbolp mouse-down-action)
+	    (progn
+	      (princ mouse-down-action)
+	      (insert ",\nwhich is ")
+	      (describe-function-1 mouse-down-action))
+	  (insert "\n")
+	  (princ mouse-down-action))))))
+
+(defun button-describe-actions (&optional button-or-pos)
+  "Describe the actions associated to the button at point.
+Displays a *Help* buffer with a description of the actions.
+
+When called from Lisp, pass BUTTON-OR-POS as the button to describe, or a
+buffer position where a button is present.  If BUTTON-OR-POS is nil, the
+button at point is the button to describe."
+  (interactive "d")
+  (let ((button (cond ((numberp button-or-pos)
+		       (button-at button-or-pos))
+		      ((markerp button-or-pos)
+		       (with-current-buffer (marker-buffer button-or-pos)
+			 (button-at button-or-pos)))
+		      ((null button-or-pos)
+		       (button-at (point)))
+		      (t
+		       button-or-pos)))
+	action mouse-down-action type)
+    (when button
+      (setq type (button-type button)
+            action (button-get button 'action)
+	    mouse-down-action (button-get button 'mouse-down-action))
+      (help-setup-xref
+       (list #'button--describe-actions-internal type action mouse-down-action)
+       (called-interactively-p 'interactive))
+      (button--describe-actions-internal type action mouse-down-action)
+      t)))
+
 (provide 'button)
 
 ;;; button.el ends here
diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index 3413cd1..f001dce 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -334,6 +334,7 @@ seq-sort-by
           (throw 'seq--break result))))
     nil))
 
+;;;###autoload
 (cl-defgeneric seq-find (pred sequence &optional default)
   "Return the first element for which (PRED element) is non-nil in SEQUENCE.
 If no element is found, return DEFAULT.
diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index e9e2818d..e9d2139 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -1530,6 +1530,48 @@ describe-categories
 	(while (setq table (char-table-parent table))
 	  (insert "\nThe parent category table is:")
 	  (describe-vector table 'help-describe-category-set))))))
+\f
+;; Actions.
+
+(defvar describe-actions-functions '(button-describe-actions
+                                     widget-describe-actions)
+  "A list of functions for `describe-actions' to call.
+Each function should take one argument, a position in the buffer, and return
+non-nil if it described the actions of an element at that position.
+The argument passed might be nil, which indicates to describe the actions of
+the element at point.")
+
+;;;###autoload
+(defun describe-actions (&optional pos)
+  "Describe the actions associated to an element at a buffer position POS.
+Actions are functions that get executed when the user activates the element,
+by clicking on it, or pressing a key.  Typically, actions are associated to
+a button (e.g., links in a *Help* buffer) or a widget (e.g., buttons, links,
+editable fields, etc., of the customization buffers).
+
+Interactively, click on an element to describe its actions, or hit RET
+to describe the actions of the element at point.
+
+When called from Lisp, POS may be a buffer position, or nil, to describe the
+actions of the element at point.
+
+Traverses the list `describe-action-functions', until one of the functions
+returns non-nil."
+  (interactive
+   (list
+    (let ((key
+           (read-key
+	    "Click an element, or hit RET to describe the element at point")))
+      (cond ((eq key ?\C-m) nil)
+            ((and (mouse-event-p key)
+                  (eq (event-basic-type key) 'mouse-1)
+                  (equal (event-modifiers key) '(click)))
+             (posn-point (event-end key)))
+            ((eq key ?\C-g) (signal 'quit nil))
+            (t (user-error "You didn't specify an element"))))))
+  (unless (seq-find (lambda (fun) (when (fboundp fun) (funcall fun pos)))
+		    describe-actions-functions)
+    (message "No actions here")))
 
 \f
 ;;; Replacements for old lib-src/ programs.  Don't seem especially useful.
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 916d41a..f8f485a 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -586,6 +586,70 @@ widget-map-buttons
       (if (and widget (funcall function widget maparg))
 	  (setq overlays nil)))))
 
+(defun widget-describe-actions (&optional widget-or-pos)
+  "Describe the actions associated to the widget at point.
+Displays a buffer with a description of the actions, as well as a link to
+browse all the properties of the widget.
+This command resolves the indirection of widgets running the action of its
+parents, so the real action executed can be known.
+
+When called from Lisp, pass WIDGET-OR-POS as the widget to describe,
+or a buffer position where a widget is present.  If WIDGET-OR-POS is nil,
+the widget at point is the widget to describe."
+  (interactive "d")
+  (require 'wid-browse)
+  (let ((widget (if (widgetp widget-or-pos)
+		    widget-or-pos
+		  (widget-at (or widget-or-pos (point)))))
+	(inhibit-read-only t) ; For erasing the contents of the buffer.
+	action mouse-down-action)
+    (when widget
+      (setq action (widget-resolve-parent-action widget)
+	    mouse-down-action (widget-get widget :mouse-down-action))
+      (help-setup-xref (list #'widget-describe-actions widget)
+		       (called-interactively-p 'interactive))
+      (with-help-window (help-buffer)
+	(with-current-buffer (help-buffer)
+	  (widget-insert "This widget's type is ")
+	  (widget-create 'widget-browse :format "%[%v%]\n%d"
+			 :doc (get (car widget) 'widget-documentation)
+			 :help-echo "Browse this widget's properties"
+			 widget)
+	  (widget-insert "\n")
+	  (when (functionp action)
+	    (widget-insert (propertize "Action" 'face 'bold))
+	    (widget-insert "\nThe action of this widget is ")
+	    (if (symbolp action)
+		(widget-create 'function-link :value action
+                               :button-prefix ""
+                               :button-suffix ""
+			       :help-echo "Describe this function")
+	      (widget-insert "\n")
+	      (princ action))
+	    (widget-insert "\n\n"))
+	  (when (functionp mouse-down-action)
+	    (widget-insert (propertize "Mouse-down-action" 'face 'bold))
+	    (widget-insert "\nThe mouse-down-action of this widget is ")
+	    (if (symbolp mouse-down-action)
+		(widget-create 'function-link :value mouse-down-action
+                               :button-prefix ""
+                               :button-suffix ""
+			       :help-echo "Describe this function")
+	      (widget-insert "\n")
+	      (princ mouse-down-action))
+	    (widget-insert "\n"))
+	  (widget-setup)
+	  (goto-char (point-min)))))))
+
+(defun widget-resolve-parent-action (widget)
+  "If action of WIDGET is `widget-parent-action', find out what would that be."
+  (let ((action (widget-get widget :action))
+	(parent (widget-get widget :parent)))
+    (while (and (eq action 'widget-parent-action)
+		(setq parent (widget-get parent :parent)))
+      (setq action (widget-get parent :action)))
+    action))
+
 ;;; Images.
 
 (defcustom widget-image-directory (file-name-as-directory
-- 
2.7.4

bug#139: describe-key vs. widget red tape

[Basil L. Contovounesios]

Mauro Aranda <maurooaranda@gmail.com> writes:

> Find in the attached patch my idea to implement this.

Thank you for working on this!

> I wrote a command, describe-actions, so the user does not have to care
> if the element to describe is either a button, a widget, or whatever.

Isn't the name describe-actions too vague/ambiguous?

> And then, the libraries that define this kind of elements should add
> themselves to `describe-actions-functions'.

SGTM, but what if, in the future, we decide we want to describe more
properties of these elements?  Instead of specifically describing
"actions", why not "describe buttons", "describe widgets", or "describe
button-like elements" (i.e. both buttons and widgets) in general,
leaving the precise properties described open to future change?

> Then I implemented the functions for the button.el buttons and the
> widgets.
>
> There are some things I would like to ask:
> - I would like to put an initial value of nil to
> `describe-actions-functions', and let each library add the function to
> this list.  Is it OK to use `with-eval-after-load' here?

I think it's best to avoid it, especially in preloaded libraries like
button.el.

> Is there another way?

I think the way it is now is fine.  Alternatively, you could move the
button- and widget-specific code to help-fns.el, but I can't really tell
if either approach is superior.

> - I put an autoload cookie in `seq-find', in order to use it in
> `describe-actions'.  Is it OK?

I think so, but you could also (require 'seq) where it's needed or use
the already loaded cl-lib library instead.

[...]

> diff --git a/lisp/button.el b/lisp/button.el
> index 04e77ca..66bc12f 100644
> --- a/lisp/button.el
> +++ b/lisp/button.el
> @@ -538,6 +538,64 @@ backward-button
>    (interactive "p\nd\nd")
>    (forward-button (- n) wrap display-message no-error))
>  
> +(defun button--describe-actions-internal (type action mouse-down-action)

The internal nature of the function is already conveyed by the double
hyphen '--' in its name, so there is no need for the '-internal' suffix,
which is usually used in C definitions.

> +  "Describe a button's TYPE, ACTION and MOUSE-DOWN-ACTION in a *Help* buffer.
> +This is a helper function for `button-describe-actions', in order to be possible
> +to use `help-setup-xref'."
> +  (with-help-window (help-buffer)
> +    (with-current-buffer (help-buffer)
> +      (insert "This button's type is ")
> +      (princ type)
> +      (insert "\n\n")

Shouldn't this sentence end with a full stop?

Perhaps the type should also be `quoted' and passed through
format-message or similar?

> +      (when (functionp action)
> +	(insert (propertize "Action" 'face 'bold))

According to the top-level dir-locals-file in the Emacs repository,
Elisp should be indented with indent-tabs-mode disabled.

> +	(insert "\nThe action of this button is ")
> +	(if (symbolp action)
> +	    (progn
> +	      (princ action)

This function symbol should probably be `quoted' and passed through
format-message or similar.

> +	      (insert ",\nwhich is ")
> +	      (describe-function-1 action))
> +	  (insert "\n")

If 'action' is not a symbol, then the previous call to 'insert' will
leave a trailing space character before this newline.

> +	  (princ action)))
> +      (when (functionp mouse-down-action)

There should be a newline or two between action and mouse-down-action.

> +	(insert (propertize "Mouse-down-action" 'face 'bold))
> +	(insert "\nThe mouse-down-action of this button is ")
> +	(if (symbolp mouse-down-action)
> +	    (progn
> +	      (princ mouse-down-action)
> +	      (insert ",\nwhich is ")
> +	      (describe-function-1 mouse-down-action))
> +	  (insert "\n")
> +	  (princ mouse-down-action))))))

The logic is identical for action and mouse-down-action, and the
argument list of this function feels a bit too rigid.  Why not accept an
alist of useful properties as a single argument instead?  Then you could
write something like the following:

  (defun button--describe-actions (properties)
    "Describe a button's PROPERTIES (an alist) in a *Help* buffer.
  This is a helper function for `button-describe-actions', intended
  to be used with `help-setup-xref'."
    (with-help-window (help-buffer)
      (with-current-buffer (help-buffer)
        (insert (format-message "This button's type is `%s'."
                                (alist-get 'type properties)))
        (dolist (prop '(action mouse-down-action))
          (let ((name (symbol-name prop))
                (val  (alist-get prop properties)))
            (when (functionp val)
              (insert "\n\n"
                      (propertize (capitalize name) 'face 'bold)
                      "\nThe " name " of this button is")
              (if (symbolp val)
                  (progn
                    (insert (format-message " `%s',\nwhich is " val))
                    (describe-function-1 val))
                (insert "\n")
                (princ val))))))))

> +(defun button-describe-actions (&optional button-or-pos)
> +  "Describe the actions associated to the button at point.
                           ^^^^^^^^^^^^^
"associated with", here and in other places below.

> +Displays a *Help* buffer with a description of the actions.
> +
> +When called from Lisp, pass BUTTON-OR-POS as the button to describe, or a
> +buffer position where a button is present.  If BUTTON-OR-POS is nil, the
> +button at point is the button to describe."
> +  (interactive "d")
> +  (let ((button (cond ((numberp button-or-pos)
> +		       (button-at button-or-pos))
> +		      ((markerp button-or-pos)
> +		       (with-current-buffer (marker-buffer button-or-pos)
> +			 (button-at button-or-pos)))

These two clauses can be joined using integer-or-marker-p, since
button-at accepts either type of buffer position.

> +		      ((null button-or-pos)
> +		       (button-at (point)))
> +		      (t
> +		       button-or-pos)))
> +	action mouse-down-action type)
> +    (when button
> +      (setq type (button-type button)
> +            action (button-get button 'action)
> +	    mouse-down-action (button-get button 'mouse-down-action))
> +      (help-setup-xref
> +       (list #'button--describe-actions-internal type action mouse-down-action)
> +       (called-interactively-p 'interactive))
> +      (button--describe-actions-internal type action mouse-down-action)
> +      t)))

As suggested previously, why not pass all interesting properties to the
helper function as a single argument?  For example:

  (defun button-describe-actions (&optional button-or-pos)
    "Describe the actions associated with the button at point.
  Displays a *Help* buffer with a description of the actions.

  When called from Lisp, BUTTON-OR-POS can be a buffer position
  where a button (including overlay buttons) is present, a button
  object (e.g., an overlay), or nil, meaning the button at point."
    (interactive "d")
    (let* ((button (cond ((integer-or-marker-p button-or-pos)
                          (button-at button-or-pos))
                         ((null button-or-pos)
                          (button-at (point)))
                         (t
                          button-or-pos)))
           (props (and button
                       (mapcar (lambda (prop)
                                 (cons prop (button-get button prop)))
                               '(type action mouse-down-action)))))
      (when props
        (help-setup-xref (list #'button--describe-actions props)
                         (called-interactively-p 'interactive))
        (button--describe-actions props)
        t)))

>  (provide 'button)
>  
>  ;;; button.el ends here

[...]

> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
> index e9e2818d..e9d2139 100644
> --- a/lisp/help-fns.el
> +++ b/lisp/help-fns.el
> @@ -1530,6 +1530,48 @@ describe-categories
>  	(while (setq table (char-table-parent table))
>  	  (insert "\nThe parent category table is:")
>  	  (describe-vector table 'help-describe-category-set))))))
> +\f
> +;; Actions.
> +
> +(defvar describe-actions-functions '(button-describe-actions
> +                                     widget-describe-actions)
> +  "A list of functions for `describe-actions' to call.
> +Each function should take one argument, a position in the buffer, and return
> +non-nil if it described the actions of an element at that position.
> +The argument passed might be nil, which indicates to describe the actions of
> +the element at point.")

Why should these hook functions accept a nil argument?
Why not always call them with a buffer position as argument?

> +;;;###autoload
> +(defun describe-actions (&optional pos)
> +  "Describe the actions associated to an element at a buffer position POS.
> +Actions are functions that get executed when the user activates the element,
> +by clicking on it, or pressing a key.  Typically, actions are associated to
> +a button (e.g., links in a *Help* buffer) or a widget (e.g., buttons, links,
> +editable fields, etc., of the customization buffers).
> +
> +Interactively, click on an element to describe its actions, or hit RET
> +to describe the actions of the element at point.
> +
> +When called from Lisp, POS may be a buffer position, or nil, to describe the
> +actions of the element at point.
> +
> +Traverses the list `describe-action-functions', until one of the functions
                                     ^^
Elsewhere you say "actions" rather than "action".

Instead of "traverses the list" I would say more specifically "runs the
hook" or "calls each function in turn".

> +returns non-nil."
> +  (interactive
> +   (list
> +    (let ((key
> +           (read-key
> +	    "Click an element, or hit RET to describe the element at point")))
> +      (cond ((eq key ?\C-m) nil)
> +            ((and (mouse-event-p key)
> +                  (eq (event-basic-type key) 'mouse-1)
> +                  (equal (event-modifiers key) '(click)))
> +             (posn-point (event-end key)))

What if the click is in a different window?

> +            ((eq key ?\C-g) (signal 'quit nil))
> +            (t (user-error "You didn't specify an element"))))))

I wonder if we can reuse help--read-key-sequence or similar here?

> +  (unless (seq-find (lambda (fun) (when (fboundp fun) (funcall fun pos)))
> +		    describe-actions-functions)
> +    (message "No actions here")))
>  
>  \f
>  ;;; Replacements for old lib-src/ programs.  Don't seem especially useful.
> diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
> index 916d41a..f8f485a 100644
> --- a/lisp/wid-edit.el
> +++ b/lisp/wid-edit.el
> @@ -586,6 +586,70 @@ widget-map-buttons
>        (if (and widget (funcall function widget maparg))
>  	  (setq overlays nil)))))
>  
> +(defun widget-describe-actions (&optional widget-or-pos)
> +  "Describe the actions associated to the widget at point.
> +Displays a buffer with a description of the actions, as well as a link to
> +browse all the properties of the widget.
> +This command resolves the indirection of widgets running the action of its
> +parents, so the real action executed can be known.
> +
> +When called from Lisp, pass WIDGET-OR-POS as the widget to describe,
> +or a buffer position where a widget is present.  If WIDGET-OR-POS is nil,
> +the widget at point is the widget to describe."
> +  (interactive "d")
> +  (require 'wid-browse)
> +  (let ((widget (if (widgetp widget-or-pos)
> +		    widget-or-pos
> +		  (widget-at (or widget-or-pos (point)))))

widget-at accepts nil, so you can just write (widget-at widget-or-pos).

[...]

> +(defun widget-resolve-parent-action (widget)

Should this function have a name with a double hyphen '--'?

> +  "If action of WIDGET is `widget-parent-action', find out what would that be."
> +  (let ((action (widget-get widget :action))
> +	(parent (widget-get widget :parent)))
> +    (while (and (eq action 'widget-parent-action)
> +		(setq parent (widget-get parent :parent)))
> +      (setq action (widget-get parent :action)))
> +    action))

I think there's a bug here.  Assuming WIDGET's :action is
'widget-parent-action', we get the following:

0. ACTION is set to widget-parent-action
1. PARENT is set to the parent of WIDGET
2. (eq action ...) is true
3. PARENT is set to its parent

Instead of looking at WIDGET's parent, we are now looking at its
grandparent.  Shouldn't we do the following instead?

  (defun widget--resolve-parent-action (widget)
    "Resolve the real action of WIDGET up its inheritance chain.
  Follow the :parents of WIDGET until its :action is no longer
  `widget-parent-action', and return its value."
    (let ((action (widget-get widget :action)))
      (while (eq action #'widget-parent-action)
        (setq widget (widget-get widget :parent))
        (setq action (widget-get widget :action)))
      action))

Thanks,

-- 
Basil

bug#139: describe-key vs. widget red tape

[Lars Ingebrigtsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

>> Find in the attached patch my idea to implement this.
>
> Thank you for working on this!

Yes, it looks really useful.

>> There are some things I would like to ask:
>> - I would like to put an initial value of nil to
>> `describe-actions-functions', and let each library add the function to
>> this list.  Is it OK to use `with-eval-after-load' here?
>
> I think it's best to avoid it, especially in preloaded libraries like
> button.el.

Yes, I think you can just have the libraries push stuff directly to this
variable (and the variable would define be in a library that's preloaded
in Emacs).

>> - I put an autoload cookie in `seq-find', in order to use it in
>> `describe-actions'.  Is it OK?
>
> I think so, but you could also (require 'seq) where it's needed or use
> the already loaded cl-lib library instead.

Putting an autoload cookie there is fine, I think.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#139: describe-key vs. widget red tape

[Mauro Aranda]

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

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> Find in the attached patch my idea to implement this.
>
> Thank you for working on this!

Thanks for the review!

>> I wrote a command, describe-actions, so the user does not have to care
>> if the element to describe is either a button, a widget, or whatever.
>
> Isn't the name describe-actions too vague/ambiguous?

Perhaps.  I didn't feel it was obvious, so I made some effort in
describing it in the doc string.  But maybe there's a better name out
there.

>> And then, the libraries that define this kind of elements should add
>> themselves to `describe-actions-functions'.
>
> SGTM, but what if, in the future, we decide we want to describe more
> properties of these elements?  Instead of specifically describing
> "actions", why not "describe buttons", "describe widgets", or "describe
> button-like elements" (i.e. both buttons and widgets) in general,
> leaving the precise properties described open to future change?

I like this idea! I think I was a little short-sighted focusing only in
the actions, because there is already a command for describing a
widget.
But, as with describe-actions, I can't find a good name for it.  I think
describe-buttons wouldn't make justice to the widgets that are not
buttons (e.g., editable fields), and describe widgets makes it sound
like it doesn't include other libraries...

>> Is there another way?
>
> I think the way it is now is fine.  Alternatively, you could move the
> button- and widget-specific code to help-fns.el, but I can't really tell
> if either approach is superior.

OK, thanks.

>> diff --git a/lisp/button.el b/lisp/button.el
>> index 04e77ca..66bc12f 100644
>> --- a/lisp/button.el
>> +++ b/lisp/button.el
>> @@ -538,6 +538,64 @@ backward-button
>>    (interactive "p\nd\nd")
>>    (forward-button (- n) wrap display-message no-error))
>>
>> +(defun button--describe-actions-internal (type action mouse-down-action)
>
> The internal nature of the function is already conveyed by the double
> hyphen '--' in its name, so there is no need for the '-internal' suffix,
> which is usually used in C definitions.

Didn't know about the '-internal' suffix, thanks.  I'll change it.

>> +  "Describe a button's TYPE, ACTION and MOUSE-DOWN-ACTION in a *Help*
buffer.
>> +This is a helper function for `button-describe-actions', in order
>> to be possible
>> +to use `help-setup-xref'."
>> +  (with-help-window (help-buffer)
>> +    (with-current-buffer (help-buffer)
>> +      (insert "This button's type is ")
>> +      (princ type)
>> +      (insert "\n\n")
>
> Shouldn't this sentence end with a full stop?

Yes, I'll fix it.

> Perhaps the type should also be `quoted' and passed through
> format-message or similar?

Right, looks much better.  Thank you.

>> +      (when (functionp action)
>> + (insert (propertize "Action" 'face 'bold))
>
> According to the top-level dir-locals-file in the Emacs repository,
> Elisp should be indented with indent-tabs-mode disabled.

What happened was that I developed the initial draft out of tree.  I'll
be more careful next time.

>> + (insert "\nThe action of this button is ")
>> + (if (symbolp action)
>> +    (progn
>> +      (princ action)
>
> This function symbol should probably be `quoted' and passed through
> format-message or similar.

I'm not sure: other help functions, like describe-function or
describe-key don't do that.  Maybe they should too?

>> +      (insert ",\nwhich is ")
>> +      (describe-function-1 action))
>> +  (insert "\n")
>
> If 'action' is not a symbol, then the previous call to 'insert' will
> leave a trailing space character before this newline.

Right, I'll fix it.

>> +  (princ action)))
>> +      (when (functionp mouse-down-action)
>
> There should be a newline or two between action and mouse-down-action.

Yes, I forgot to add those newlines.  I'll fix it.

>> + (insert (propertize "Mouse-down-action" 'face 'bold))
>> + (insert "\nThe mouse-down-action of this button is ")
>> + (if (symbolp mouse-down-action)
>> +    (progn
>> +      (princ mouse-down-action)
>> +      (insert ",\nwhich is ")
>> +      (describe-function-1 mouse-down-action))
>> +  (insert "\n")
>> +  (princ mouse-down-action))))))
>
> The logic is identical for action and mouse-down-action, and the
> argument list of this function feels a bit too rigid.  Why not accept an
> alist of useful properties as a single argument instead?  Then you could
> write something like the following:
>
>   (defun button--describe-actions (properties)
>     "Describe a button's PROPERTIES (an alist) in a *Help* buffer.
>   This is a helper function for `button-describe-actions', intended
>   to be used with `help-setup-xref'."
>     (with-help-window (help-buffer)
>       (with-current-buffer (help-buffer)
>         (insert (format-message "This button's type is `%s'."
>                                 (alist-get 'type properties)))
>         (dolist (prop '(action mouse-down-action))
>           (let ((name (symbol-name prop))
>                 (val  (alist-get prop properties)))
>             (when (functionp val)
>               (insert "\n\n"
>                       (propertize (capitalize name) 'face 'bold)
>                       "\nThe " name " of this button is")
>               (if (symbolp val)
>                   (progn
>                     (insert (format-message " `%s',\nwhich is " val))
>                     (describe-function-1 val))
>                 (insert "\n")
>                 (princ val))))))))

Totally! I'll adapt it to something like that.

>> +(defun button-describe-actions (&optional button-or-pos)
>> +  "Describe the actions associated to the button at point.
>                            ^^^^^^^^^^^^^
> "associated with", here and in other places below.

Thanks, I'll fix those.

>> +Displays a *Help* buffer with a description of the actions.
>> +
>> +When called from Lisp, pass BUTTON-OR-POS as the button to describe, or
a
>> +buffer position where a button is present.  If BUTTON-OR-POS is nil, the
>> +button at point is the button to describe."
>> +  (interactive "d")
>> +  (let ((button (cond ((numberp button-or-pos)
>> +       (button-at button-or-pos))
>> +      ((markerp button-or-pos)
>> +       (with-current-buffer (marker-buffer button-or-pos)
>> + (button-at button-or-pos)))
>
> These two clauses can be joined using integer-or-marker-p, since
> button-at accepts either type of buffer position.

I know that, but I put the two checks because I had found a problem with
the `forward' and `back' buttons of the *Help* buffer.  However, it
looks like I can trigger the problem (or a similar one, I'm not sure)
even with the way I wrote it, so I'll have to investigate it further.

>> +      ((null button-or-pos)
>> +       (button-at (point)))
>> +      (t
>> +       button-or-pos)))
>> + action mouse-down-action type)
>> +    (when button
>> +      (setq type (button-type button)
>> +            action (button-get button 'action)
>> +    mouse-down-action (button-get button 'mouse-down-action))
>> +      (help-setup-xref
>> + (list #'button--describe-actions-internal type action
>> mouse-down-action)
>> +       (called-interactively-p 'interactive))
>> +      (button--describe-actions-internal type action mouse-down-action)
>> +      t)))
>
> As suggested previously, why not pass all interesting properties to the
> helper function as a single argument?  For example:
>
>   (defun button-describe-actions (&optional button-or-pos)
>     "Describe the actions associated with the button at point.
>   Displays a *Help* buffer with a description of the actions.
>
>   When called from Lisp, BUTTON-OR-POS can be a buffer position
>   where a button (including overlay buttons) is present, a button
>   object (e.g., an overlay), or nil, meaning the button at point."
>     (interactive "d")
>     (let* ((button (cond ((integer-or-marker-p button-or-pos)
>                           (button-at button-or-pos))
>                          ((null button-or-pos)
>                           (button-at (point)))
>                          (t
>                           button-or-pos)))
>            (props (and button
>                        (mapcar (lambda (prop)
>                                  (cons prop (button-get button prop)))
>                                '(type action mouse-down-action)))))
>       (when props
>         (help-setup-xref (list #'button--describe-actions props)
>                          (called-interactively-p 'interactive))
>         (button--describe-actions props)
>         t)))

Yes, it is much better, thank you.

>> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
>> index e9e2818d..e9d2139 100644
>> --- a/lisp/help-fns.el
>> +++ b/lisp/help-fns.el
>> @@ -1530,6 +1530,48 @@ describe-categories
>>   (while (setq table (char-table-parent table))
>>    (insert "\nThe parent category table is:")
>>    (describe-vector table 'help-describe-category-set))))))
>> +
>> +;; Actions.
>> +
>> +(defvar describe-actions-functions '(button-describe-actions
>> +                                     widget-describe-actions)
>> +  "A list of functions for `describe-actions' to call.
>> +Each function should take one argument, a position in the buffer, and
return
>> +non-nil if it described the actions of an element at that position.
>> +The argument passed might be nil, which indicates to describe the
actions of
>> +the element at point.")
>
> Why should these hook functions accept a nil argument?
> Why not always call them with a buffer position as argument?

Keep in mind that I started with `widget-describe-actions', and because
I developed that as a command, I thought it was useful to accept a nil
argument defaulting to point.  I like it better.

But yes, there is no problem in passing them always a buffer position.

>> +;;;###autoload
>> +(defun describe-actions (&optional pos)
>> +  "Describe the actions associated to an element at a buffer position
POS.
>> +Actions are functions that get executed when the user activates the
element,
>> +by clicking on it, or pressing a key.  Typically, actions are
associated to
>> +a button (e.g., links in a *Help* buffer) or a widget (e.g., buttons,
links,
>> +editable fields, etc., of the customization buffers).
>> +
>> +Interactively, click on an element to describe its actions, or hit RET
>> +to describe the actions of the element at point.
>> +
>> +When called from Lisp, POS may be a buffer position, or nil, to
describe the
>> +actions of the element at point.
>> +
>> +Traverses the list `describe-action-functions', until one of the
functions
>                                      ^^
> Elsewhere you say "actions" rather than "action".

Typo.  I'll fix it.

> Instead of "traverses the list" I would say more specifically "runs the
> hook" or "calls each function in turn".

That's better, yes.  I'll change it.

>> +returns non-nil."
>> +  (interactive
>> +   (list
>> +    (let ((key
>> +           (read-key
>> +    "Click an element, or hit RET to describe the element at point")))
>> +      (cond ((eq key ?\C-m) nil)
>> +            ((and (mouse-event-p key)
>> +                  (eq (event-basic-type key) 'mouse-1)
>> +                  (equal (event-modifiers key) '(click)))
>> +             (posn-point (event-end key)))
>
> What if the click is in a different window?
>

I missed that, silly me.  I'm not sure what would be better, but I would
like to describe the clicked element (if any), even if it's not in the
selected window.

>> +            ((eq key ?\C-g) (signal 'quit nil))
>> +            (t (user-error "You didn't specify an element"))))))
>
> I wonder if we can reuse help--read-key-sequence or similar here?

I don't think so:  It has a prompt that would not be useful here, and
for the expected input, it seems a little overkill.  But, I might be
short-sighted here too...

>> +When called from Lisp, pass WIDGET-OR-POS as the widget to describe,
>> +or a buffer position where a widget is present.  If WIDGET-OR-POS is
nil,
>> +the widget at point is the widget to describe."
>> +  (interactive "d")
>> +  (require 'wid-browse)
>> +  (let ((widget (if (widgetp widget-or-pos)
>> +    widget-or-pos
>> +  (widget-at (or widget-or-pos (point)))))
>
> widget-at accepts nil, so you can just write (widget-at widget-or-pos).

Yes, don't know why I missed that.  I'll fix it.

> [...]
>
>> +(defun widget-resolve-parent-action (widget)
>
> Should this function have a name with a double hyphen '--'?

Yes, I'll do it.

>> + "If action of WIDGET is `widget-parent-action', find out what
>> would that be."
>> +  (let ((action (widget-get widget :action))
>> + (parent (widget-get widget :parent)))
>> +    (while (and (eq action 'widget-parent-action)
>> + (setq parent (widget-get parent :parent)))
>> +      (setq action (widget-get parent :action)))
>> +    action))
>
> I think there's a bug here.  Assuming WIDGET's :action is
> 'widget-parent-action', we get the following:
>
> 0. ACTION is set to widget-parent-action
> 1. PARENT is set to the parent of WIDGET
> 2. (eq action ...) is true
> 3. PARENT is set to its parent
>
> Instead of looking at WIDGET's parent, we are now looking at its
> grandparent.  Shouldn't we do the following instead?
>
>   (defun widget--resolve-parent-action (widget)
>     "Resolve the real action of WIDGET up its inheritance chain.
>   Follow the :parents of WIDGET until its :action is no longer
>   `widget-parent-action', and return its value."
>     (let ((action (widget-get widget :action)))
>       (while (eq action #'widget-parent-action)
>         (setq widget (widget-get widget :parent))
>         (setq action (widget-get widget :action)))
>       action))

Yes, I had something like that in my draft, and for some reason I
changed it and ended up with a buggy version.  Thanks for spotting
that, I'll revert it to my original version but keep the doc string you
suggested, because it is much better.

Thank you again for your review, it was very helpful.  I hope I'll find
time in the next days to keep working on it.

[-- Attachment #2: Type: text/html, Size: 17945 bytes --]

bug#139: describe-key vs. widget red tape

[Mauro Aranda]

[-- Attachment #1.1: Type: text/plain, Size: 540 bytes --]

I finally found some time to keep working on this.

I reworked the patch with the useful suggestion from Basil.  I think I
addressed all the points raised, and that the patch looks much better
now.  I went with `describe-widget' for the name of the command, but
have no preference over the name, so if someone suggests something
better, I'm OK with it.

If the approach of putting the variable in a preloaded library, like
Lars suggested, is better, feel free to either adapt the code
or tell me and I'll do it.

WDYT?

Best regards,
Mauro

[-- Attachment #1.2: Type: text/html, Size: 647 bytes --]

[-- Attachment #2: 0001-New-command-describe-widget-Bug-139.patch --]
[-- Type: text/x-patch, Size: 8388 bytes --]

From eb7f8a6172761826ff3e89314927ee1a931c1efa Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Fri, 11 Oct 2019 10:00:01 -0300
Subject: [PATCH] New command describe-widget (Bug#139)

Thanks to Basil Contovounesios for providing enhancements.

* lisp/help-fns.el (describe-widget-functions): New variable, used by
describe-widget.
(describe-widget): New command, to display information about a widget.
* lisp/button.el (button-describe): New command, for describing a button.
(button--describe): Helper function for button-describe.
* lisp/wid-edit.el (widget-describe): New command, for describing a
widget.
(widget--resolve-parent-action): Helper function, to allow
widget-describe to display more useful information.
---
 lisp/button.el   | 45 ++++++++++++++++++++++++++++++++++++++++++++
 lisp/help-fns.el | 44 +++++++++++++++++++++++++++++++++++++++++++
 lisp/wid-edit.el | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 146 insertions(+)

diff --git a/lisp/button.el b/lisp/button.el
index 04e77ca..bc212e5 100644
--- a/lisp/button.el
+++ b/lisp/button.el
@@ -538,6 +538,51 @@ backward-button
   (interactive "p\nd\nd")
   (forward-button (- n) wrap display-message no-error))
 
+(defun button--describe (properties)
+  "Describe a button's PROPERTIES (an alist) in a *Help* buffer.
+This is a helper function for `button-describe', in order to be possible to
+use `help-setup-xref'.
+
+Each element of PROPERTIES should be of the form (PROPERTY . VALUE)."
+  (help-setup-xref (list #'button--describe properties)
+                   (called-interactively-p 'interactive))
+  (with-help-window (help-buffer)
+    (with-current-buffer (help-buffer)
+      (insert (format-message "This button's type is `%s'."
+                              (alist-get 'type properties)))
+      (dolist (prop '(action mouse-action))
+        (let ((name (symbol-name prop))
+              (val (alist-get prop properties)))
+          (when (functionp val)
+            (insert "\n\n"
+                    (propertize (capitalize name) 'face 'bold)
+                    "\nThe " name " of this button is")
+            (if (symbolp val)
+                (progn
+                  (insert (format-message " `%s',\nwhich is " val))
+                  (describe-function-1 val))
+              (insert "\n")
+              (princ val))))))))
+
+(defun button-describe (&optional button-or-pos)
+  "Display a buffer with information about the button at point.
+
+When called from Lisp, pass BUTTON-OR-POS as the button to describe, or a
+buffer position where a button is present.  If BUTTON-OR-POS is nil, the
+button at point is the button to describe."
+  (interactive "d")
+  (let* ((button (cond ((integer-or-marker-p button-or-pos)
+                        (button-at button-or-pos))
+                       ((null button-or-pos) (button-at (point)))
+                       ((overlayp button-or-pos) button-or-pos)))
+         (props (and button
+                     (mapcar (lambda (prop)
+                               (cons prop (button-get button prop)))
+                             '(type action mouse-action)))))
+    (when props
+      (button--describe props)
+      t)))
+
 (provide 'button)
 
 ;;; button.el ends here
diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 06b15a3..997b076 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -1533,6 +1533,50 @@ describe-categories
 	  (describe-vector table 'help-describe-category-set))))))
 
 \f
+;; Widgets.
+
+(defvar describe-widget-functions
+  '(button-describe widget-describe)
+  "A list of functions for `describe-widget' to call.
+Each function should take one argument, a buffer position, and return
+non-nil if it described a widget at that position.")
+
+;;;###autoload
+(defun describe-widget (&optional pos)
+  "Display a buffer with information about a widget.
+You can use this command to describe buttons (e.g., the links in a *Help*
+buffer), editable fields of the customization buffers, etc.
+
+Interactively, click on a widget to describe it, or hit RET to describe the
+widget at point.
+
+When called from Lisp, POS may be a buffer position or a mouse position list.
+
+Calls each function of the list `describe-widget-functions' in turn, until
+one of them returns non-nil."
+  (interactive
+   (list
+    (let ((key
+           (read-key
+            "Click on a widget, or hit RET to describe the widget at point")))
+      (cond ((eq key ?\C-m) (point))
+            ((and (mouse-event-p key)
+                  (eq (event-basic-type key) 'mouse-1)
+                  (equal (event-modifiers key) '(click)))
+             (event-end key))
+            ((eq key ?\C-g) (signal 'quit nil))
+            (t (user-error "You didn't specify a widget"))))))
+  (let (buf)
+    ;; Allow describing a widget in a different window.
+    (when (posnp pos)
+      (setq buf (window-buffer (posn-window pos))
+            pos (posn-point pos)))
+    (with-current-buffer (or buf (current-buffer))
+      (unless (cl-some (lambda (fun) (when (fboundp fun) (funcall fun pos)))
+                       describe-widget-functions)
+        (message "No widget found at that position")))))
+\f
+
 ;;; Replacements for old lib-src/ programs.  Don't seem especially useful.
 
 ;; Replaces lib-src/digest-doc.c.
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 4d1a609..5e904cb 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -590,6 +590,63 @@ widget-map-buttons
       (if (and widget (funcall function widget maparg))
 	  (setq overlays nil)))))
 
+(defun widget-describe (&optional widget-or-pos)
+  "Describe the widget at point.
+Displays a buffer with information about the widget (e.g., its actions) as well
+as a link to browse all the properties of the widget.
+
+This command resolves the indirection of widgets running the action of its
+parents, so the real action executed can be known.
+
+When called from Lisp, pass WIDGET-OR-POS as the widget to describe,
+or a buffer position where a widget is present.  If WIDGET-OR-POS is nil,
+the widget at point is the widget to describe."
+  (interactive "d")
+  (require 'wid-browse) ; The widget-browse widget.
+  (let ((widget (if (widgetp widget-or-pos)
+                    widget-or-pos
+                  (widget-at widget-or-pos)))
+        props)
+    (when widget
+      (help-setup-xref (list #'widget-describe widget)
+                       (called-interactively-p 'interactive))
+      (setq props (list (cons 'action (widget--resolve-parent-action widget))
+                        (cons 'mouse-down-action
+                              (widget-get widget :mouse-down-action))))
+      (with-help-window (help-buffer)
+        (with-current-buffer (help-buffer)
+          (widget-insert "This widget's type is ")
+          (widget-create 'widget-browse :format "%[%v%]\n%d"
+                         :doc (get (car widget) 'widget-documentation)
+                         :help-echo "Browse this widget's properties"
+                         widget)
+          (dolist (action '(action mouse-down-action))
+            (let ((name (symbol-name action))
+                  (val (alist-get action props)))
+              (when (functionp val)
+                (widget-insert "\n\n" (propertize (capitalize name) 'face 'bold)
+                               "'\nThe " name " of this widget is")
+                (if (symbolp val)
+                    (progn (widget-insert " ")
+                           (widget-create 'function-link :value val
+                                          :button-prefix "" :button-suffix ""
+                                          :help-echo "Describe this function"))
+                  (widget-insert "\n")
+                  (princ val)))))))
+      (widget-setup)
+      t)))
+
+(defun widget--resolve-parent-action (widget)
+  "Resolve the real action of WIDGET up its inheritance chain.
+Follow the WIDGET's parents, until its :action is no longer
+`widget-parent-action', and return its value."
+  (let ((action (widget-get widget :action))
+        (parent (widget-get widget :parent)))
+    (while (eq action 'widget-parent-action)
+      (setq parent (widget-get parent :parent)
+            action (widget-get parent :action)))
+    action))
+
 ;;; Images.
 
 (defcustom widget-image-directory (file-name-as-directory
-- 
2.7.4

bug#139: describe-key vs. widget red tape

[Drew Adams]

> I went with `describe-widget' for the name of the command, but have no preference over the name, so if someone suggests something better, I'm OK with it.

Coming late to this party.  Apologies if
this question/comment is irrelevant or has
already been addressed.

IIUC, `C-h k' would still not offer any help
for a button click.  If I misunderstand about
that please let me know, and skip the rest of
this message.

---

It may be fine/appropriate to _also_ have
a separate help command for this (button
actions), e.g., so you can bind it to a
separate key.

But can/should we not try to also integrate
this help with `C-h k'? 

`C-h k' is for mouse-button actions as well
as keyboard keys.  From a user point of
view I'd expect `C-h k' to tell me about
any mouse or keyboard action I use.

Not having `C-h k' help with button actions
has long been a noticeable lacuna.

We already let `C-h k' tell you about both
down and up events for a click.  Why should
a user not expect that s?he can follow
`C-h k' with a click on a button and get the
expected help about what that button does
when you click it?

bug#139: describe-key vs. widget red tape

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> I finally found some time to keep working on this.
>
> I reworked the patch with the useful suggestion from Basil.  I think I
> addressed all the points raised, and that the patch looks much better
> now.  I went with `describe-widget' for the name of the command, but
> have no preference over the name, so if someone suggests something
> better, I'm OK with it.

I think this looks great.  It just needs a NEWS entry and a mention in
the Emacs (or Elisp?) manual (somewhere close to where describe-function
etc is documented).

And as Drew says, we should consider hooking this up to `C-h k' when on
a button/widget, perhaps.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#139: describe-key vs. widget red tape

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> I finally found some time to keep working on this.
>>
>> I reworked the patch with the useful suggestion from Basil.  I think I
>> addressed all the points raised, and that the patch looks much better
>> now.  I went with `describe-widget' for the name of the command, but
>> have no preference over the name, so if someone suggests something
>> better, I'm OK with it.
>
> I think this looks great.  It just needs a NEWS entry and a mention in
> the Emacs (or Elisp?) manual (somewhere close to where describe-function
> etc is documented).

I've now applied this patch (and I added a news entry), and I'll be
pushing it to Emacs 28 as soon as I've added some documentation.

> And as Drew says, we should consider hooking this up to `C-h k' when on
> a button/widget, perhaps.

Ok, yeah, that should probably be added, too.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#139: describe-key vs. widget red tape

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I've now applied this patch (and I added a news entry), and I'll be
> pushing it to Emacs 28 as soon as I've added some documentation.

Done.

>> And as Drew says, we should consider hooking this up to `C-h k' when on
>> a button/widget, perhaps.
>
> Ok, yeah, that should probably be added, too.

And looking at that a bit more, it just didn't seem natural to add that
in any non-hacky fashion.  Instead I had the button-push (and widget)
command doc strings mention these commands.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#139: Still can't figure out what the command I used is called

[積丹尼 Dan Jacobson]

10 years later. gnus-version "Gnus v5.13" emacs-version "27.1"

If some click / press can do some action,
then surely "C-h k runs the command describe-key"
can, somewhere in its output, tell me what that does.

I mean say poking around, clicking things left and right, I found a neat
command. But what is it? It is neat, but has no name I can dig up.

We do C-h k RET, but of course "the secretary won't let us see what the
command finally points to":

   RET runs the command gnus-article-press-button (found in
   gnus-mime-button-map), which is an interactive compiled Lisp function
   in ‘gnus-art.el’.

   It is bound to RET, <menu-bar> <MIME Part> <Toggle Display>.

   (gnus-article-press-button)

   Check text at point for a callback function.
   If the text at point has a ‘gnus-callback’ property,
   call it with the value of the ‘gnus-data’ text property.

Same with
  <down-mouse-1> at that spot runs the command mouse-drag-region (found
   in global-map), which is an interactive compiled Lisp function in
  ‘mouse.el’...

What they finally (dynamically) point to is still 'top secret' in
emacs-version "27.1".

bug#139: Still can't figure out what the command I used is called

[Thien-Thi Nguyen]

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

() 積丹尼 Dan Jacobson <jidanni@jidanni.org>
() Sat, 31 Oct 2020 09:32:23 +0800

   What they finally (dynamically) point to is still 'top
   secret' in emacs-version "27.1".

I globally bind F11 to ‘describe-text-properties’.
That sometimes helps when things are too mysterious.

-- 
Thien-Thi Nguyen -----------------------------------------------
 (defun responsep (query)               ; (2020) Software Libero
   (pcase (context query)               ;       = Dissenso Etico
     (`(technical ,ml) (correctp ml))
     ...))                              748E A0E8 1CB8 A748 9BFA
--------------------------------------- 6CE4 6703 2224 4C80 7502

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 219 bytes --]

bug#139: Still can't figure out what the command I used is called

[積丹尼 Dan Jacobson]

>>>>> "TN" == Thien-Thi Nguyen <ttn@gnuvola.org> writes:
TN> I globally bind F11 to ‘describe-text-properties’.
TN> That sometimes helps when things are too mysterious.

Indeed that is great. (Except hard to remember to query the text,
instead of the key, to find out what the key does!)

bug#139: describe-key vs. widget red tape

[Lars Ingebrigtsen]

積丹尼 Dan Jacobson <jidanni@jidanni.org> writes:

> What they finally (dynamically) point to is still 'top secret' in
> emacs-version "27.1".

This was fixed in Emacs 28, as previously stated.  Closing again.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no