bug#20664: 25.0.50; Customize: Link option or face name to the source definition

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Drew Adams]

Feature request.  In Customize, for options and faces.  Put a link on
the option/face name, so that following the link takes you to the source
code (defcustom or defface).

In GNU Emacs 25.0.50.1 (i686-pc-mingw32)
 of 2014-10-20 on LEG570
Bzr revision: 118168 rgm@gnu.org-20141020195941-icp42t8ttcnud09g
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --enable-checking=yes,glyphs CPPFLAGS=-DGLYPH_DEBUG=1'

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Oleh Krehel]

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

Drew Adams <drew.adams@oracle.com> writes:

> Feature request.  In Customize, for options and faces.  Put a link on
> the option/face name, so that following the link takes you to the source
> code (defcustom or defface).

Please check the patch. 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-to-follow-to-face-definition-in-Customize.patch --]
[-- Type: text/x-diff, Size: 1949 bytes --]

From bea177aff8de195d428106003fab8b8498e7aae8 Mon Sep 17 00:00:00 2001
From: Oleh Krehel <ohwoeowho@gmail.com>
Date: Wed, 27 May 2015 15:06:42 +0200
Subject: [PATCH] Allow to follow to face definition in Customize

* lisp/cus-edit.el (custom-face-value-create): Make the face name into
  a button that calls `find-face-definition'.

(Bug#20664)
---
 lisp/cus-edit.el | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index 1d9a9d6..246f08e 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -3483,19 +3483,22 @@ the present value is saved to its :shown-value property instead."
 	  (widget-put widget :buttons buttons))
 
       ;; Draw an ordinary `custom-face' widget
-      (let ((opoint (point)))
-	;; Visibility indicator.
-	(push (widget-create-child-and-convert
-	       widget 'custom-visibility
-	       :help-echo "Hide or show this face."
-	       :on "Hide" :off "Show"
-	       :on-glyph "down" :off-glyph "right"
-	       :action 'custom-toggle-hide-face
-	       (not hiddenp))
-	      buttons)
-	;; Face name (tag).
-	(insert " " tag)
-	(widget-specify-sample widget opoint (point)))
+      (let (opoint)
+        ;; Visibility indicator.
+        (push (widget-create-child-and-convert
+               widget 'custom-visibility
+               :help-echo "Hide or show this face."
+               :on "Hide" :off "Show"
+               :on-glyph "down" :off-glyph "right"
+               :action 'custom-toggle-hide-face
+               (not hiddenp))
+              buttons)
+        (setq opoint (point))
+        ;; Face name (tag).
+        (insert " " tag)
+        (make-button opoint (point)
+                     'follow-link t
+                     'action (lambda (&rest _x) (find-face-definition symbol))))
       (insert
        (cond ((eq custom-buffer-style 'face) " ")
 	     ((string-match-p "face\\'" tag)   ":")
-- 
1.8.4


[-- Attachment #3: Type: text/plain, Size: 635 bytes --]


With this patch, "RET" works as expected, while "<mouse-1>" is giving an
error for a reason I don't understand.  In order to make it work for the
mouse, I have to do something contorted instead of a plain
`make-button':

(define-key (overlay-get
             (make-button opoint (point)
                          'follow-link t
                          'action (lambda (&rest _x) (find-face-definition symbol)))
             'keymap) [down-mouse-1]
  (lambda ()
    (interactive)
    (let ((button (button-at (point))))
      (when button
        (button-activate button)
        t))))

Maybe someone can suggest a better way.

Oleh

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[martin rudalics]

 > With this patch, "RET" works as expected, while "<mouse-1>" is giving an
 > error for a reason I don't understand.

Bug#20398?

martin

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Drew Adams]

> Please check the patch.

I get this: 

widget-button-click: Wrong type argument: consp, #<overlay from 259 to 279 in *Customize Face: Show Paren Mismatch*>

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Oleh Krehel]

martin rudalics <rudalics@gmx.at> writes:

>> With this patch, "RET" works as expected, while "<mouse-1>" is giving an
>> error for a reason I don't understand.
>
> Bug#20398?

No, this is likely caused by the fact that `Custom-mode' binds
"<down-mouse-1>" to the incompatible (with buttons) `widget-button-click'.

And even with 'follow-link t, "<down-mouse-1>" supersedes "<mouse-1>",
which is actually bound to `push-button'.

I think if `Custom-mode' didn't bind "<down-mouse-1>", it would
translate to "<mouse-1>" and properly call `push-button'.

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Oleh Krehel]

Drew Adams <drew.adams@oracle.com> writes:

>> Please check the patch.
>
> I get this: 
>
> widget-button-click: Wrong type argument: consp, #<overlay from 259 to 279 in *Customize Face: Show Paren Mismatch*>

Yes, I get the same thing when clicking "<mouse-1>". It works fine for
"RET" and "<mouse-2>" though. With the additional code from above, it
also should work for "<mouse-1>", but I wonder if there's a better way.

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Drew Adams]

> >> With this patch, "RET" works as expected, while "<mouse-1>" is
> >> giving an error for a reason I don't understand.
> > Bug#20398?
> 
> No, this is likely caused by the fact that `Custom-mode' binds
> "<down-mouse-1>" to the incompatible (with buttons) `widget-button-click'.
> And even with 'follow-link t, "<down-mouse-1>" supersedes "<mouse-1>",
> which is actually bound to `push-button'.
> 
> I think if `Custom-mode' didn't bind "<down-mouse-1>", it would
> translate to "<mouse-1>" and properly call `push-button'.

FWIW: With `emacs -Q', setting `mouse-1-click-follows-link' = nil,
`mouse-2' gives the same error message, plus an additional error saying
that `symbol' is an bound variable.

widget-button-click: Wrong type argument: consp, #<overlay from 259 to 272 in *Customize Face: Dired Header*>
Symbol's value as variable is void: symbol

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Oleh Krehel]

Drew Adams <drew.adams@oracle.com> writes:

> FWIW: With `emacs -Q', setting `mouse-1-click-follows-link' = nil,
> `mouse-2' gives the same error message, plus an additional error saying
> that `symbol' is an bound variable.
>
> widget-button-click: Wrong type argument: consp, #<overlay from 259 to 272 in *Customize Face: Dired Header*>
> Symbol's value as variable is void: symbol

Strange, cus-edit.el does have lexical-binding, it should work.

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Drew Adams]

> > FWIW: With `emacs -Q', setting `mouse-1-click-follows-link' = nil,
> > `mouse-2' gives the same error message, plus an additional error
> > saying that `symbol' is an bound variable.
> >
> > widget-button-click: Wrong type argument: consp, #<overlay from
> > 259 to 272 in *Customize Face: Dired Header*>
> > Symbol's value as variable is void: symbol
> 
> Strange, cus-edit.el does have lexical-binding, it should work.

My bad.  Sorry for the noise.

Adding a -*- lexical-binding:t -*- declaration to the patch-test
file I used makes it work - the link is followed OK, and there is
no void-var error.  (But I do still get the wrong-type-arg error.)

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Lars Ingebrigtsen]

Oleh Krehel <ohwoeowho@gmail.com> writes:

> * lisp/cus-edit.el (custom-face-value-create): Make the face name into
>   a button that calls `find-face-definition'.

[...]

> +        (setq opoint (point))
> +        ;; Face name (tag).
> +        (insert " " tag)
> +        (make-button opoint (point)
> +                     'follow-link t
> +                     'action (lambda (&rest _x) (find-face-definition symbol))))

Makes sense to me.  I tweaked it a bit before applying, though -- used
insert-text-button for convenience.

> With this patch, "RET" works as expected, while "<mouse-1>" is giving an
> error for a reason I don't understand.  In order to make it work for the
> mouse, I have to do something contorted instead of a plain
> `make-button':

I think that's just a bug in Customize -- I think I've now fixed that,
but it involved some refactoring, so hopefully I didn't break anything.
But a standard Customize buffer seems to have the same number of things
that work before/after the rework, so I'm hopeful.  :-)

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

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Mauro Aranda]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Oleh Krehel <ohwoeowho@gmail.com> writes:
>
>> * lisp/cus-edit.el (custom-face-value-create): Make the face name into
>>   a button that calls `find-face-definition'.
>
> [...]
>
>> +        (setq opoint (point))
>> +        ;; Face name (tag).
>> +        (insert " " tag)
>> +        (make-button opoint (point)
>> +                     'follow-link t
>> + 'action (lambda (&rest _x) (find-face-definition symbol))))
>
> Makes sense to me.  I tweaked it a bit before applying, though -- used
> insert-text-button for convenience.
>

Why a button.el button, and not a link widget? Using a button.el button
makes it look like TAB is inconsistent, because it skips the button, so
you can't get to the button by just pressing TAB.

And given there are links to which you can get to by pressing TAB, not
being able to reach this one makes it feel like something is wrong.

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

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Drew Adams]

>> Makes sense to me.  I tweaked it a bit before applying, though -- used
>> insert-text-button for convenience.
>
> Why a button.el button, and not a link widget? Using a
> button.el button makes it look like TAB is inconsistent,
> because it skips the button, so you can't get to the
> button by just pressing TAB.
>
> And given there are links to which you can get to by
> pressing TAB, not being able to reach this one makes
> it feel like something is wrong.

[Caveat: I haven't tried the updated code.]

Yes, it's important that the fix fit well with Customize,
both from a UI/user point of view and from a coding
point of view.

Please also do make sure that `mouse-1-click-follows-link'
is effective, i.e., that it has its expected effect when
either nil or non-nil.  (I have it set to nil, BTW.)

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Mauro Aranda]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I think that's just a bug in Customize -- I think I've now fixed that,
> but it involved some refactoring, so hopefully I didn't break anything.
> But a standard Customize buffer seems to have the same number of things
> that work before/after the rework, so I'm hopeful.  :-)

After the change in wid-edit.el, clicking in the Value Menu button
doesn't display a menu.

I attach a patch to fix it.

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

[-- Attachment #2: 0001-Fix-recent-change-in-wid-edit.patch --]
[-- Type: text/x-patch, Size: 1055 bytes --]

From 5f92d06af343c3ac28f1fb1be72eb695d38fe771 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Thu, 17 Sep 2020 18:31:57 -0300
Subject: [PATCH] Fix recent change in wid-edit

* lisp/wid-edit.el (widget-button--check-and-call-button): Record the
ending position of event, because we might need it when
the :mouse-down-action function returns non-nil.  (Bug#20664)
---
 lisp/wid-edit.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 8be489bf08..5ac52777f8 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -978,7 +978,8 @@ widget-button--check-and-call-button
 If nothing was called, return non-nil."
   (let* ((oevent event)
          (mouse-1 (memq (event-basic-type event) '(mouse-1 down-mouse-1)))
-         newpoint pos)
+         (pos (widget-event-point event))
+         newpoint)
     (catch 'button-press-cancelled
       ;; Mouse click on a widget button.  Do the following
       ;; in a save-excursion so that the click on the button
-- 
2.28.0

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> After the change in wid-edit.el, clicking in the Value Menu button
> doesn't display a menu.
>
> I attach a patch to fix it.

Thanks; applied to the trunk.

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

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> Why a button.el button, and not a link widget? Using a button.el button
> makes it look like TAB is inconsistent, because it skips the button, so
> you can't get to the button by just pressing TAB.

I just hate the widget code...  soooo much!

But I've now rewritten it to use a widget.

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

bug#20664: 25.0.50; Customize: Link option or face name to the source definition

[Mauro Aranda]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> Why a button.el button, and not a link widget? Using a button.el button
>> makes it look like TAB is inconsistent, because it skips the button, so
>> you can't get to the button by just pressing TAB.
>
> I just hate the widget code...  soooo much!
>
> But I've now rewritten it to use a widget.

Thanks!

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