bug#32029: PATCH: xref-find-definitions-at-mouse

bug#32029: PATCH: xref-find-definitions-at-mouse

[Tobias Gerdin]

Hello,

I find it convenient to be able to go to definitions using the mouse 
(especially when getting to know new code bases). I have the below 
function bound to C-mouse-1 like so:

(global-set-key [C-mouse-1] 'xref-find-definitions-at-mouse)
(global-set-key [C-down-mouse-1] nil)

I also find it convenient to be able to get back to where I was using 
only the mouse (enabling keyboard-free navigation):

(global-set-key [C-mouse-3] 'xref-pop-marker-stack)
(global-set-key [C-down-mouse-3] nil)

Finding suitable default keybindings (well, "mouse bindings") is beyond 
the scope of this patch, but what I have above is the same binding as 
other popular IDEs such as IntelliJ and VS Code.

Regards,

Tobias Gerdin

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 9a437b6f69..85a1bc6be4 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -873,6 +873,18 @@ With prefix argument, prompt for the identifier."
    (interactive (list (xref--read-identifier "Find references of: ")))
    (xref--find-xrefs identifier 'references identifier nil))

+;;;###autoload
+(defun xref-find-definitions-at-mouse (event)
+  "Find the definition of identifier around mouse click."
+  (interactive "e")
+  (let* ((backend (xref-find-backend))
+     (identifier (save-excursion
+              (mouse-set-point event)
+              (xref-backend-identifier-at-point backend))))
+    (if identifier
+    (xref--find-definitions identifier nil)
+      (user-error "No identifier here"))))
+
  (declare-function apropos-parse-pattern "apropos" (pattern))

  ;;;###autoload

bug#32029: PATCH: xref-find-definitions-at-mouse

[Dmitry Gutov]

On 7/2/18 2:18 AM, Tobias Gerdin wrote:

> +;;;###autoload
> +(defun xref-find-definitions-at-mouse (event)
> +  "Find the definition of identifier around mouse click."
> +  (interactive "e")
> +  (let* ((backend (xref-find-backend))
> +     (identifier (save-excursion
> +              (mouse-set-point event)
> +              (xref-backend-identifier-at-point backend))))
> +    (if identifier
> +    (xref--find-definitions identifier nil)
> +      (user-error "No identifier here"))))

You should call `mouse-set-point` before `xref-find-backend`, because 
the latter might conceivably depend on the value of point.

I think this can be written much shorter (call mouse-set-point, then 
interactively call xref-find-definitions), but I'm not sure how.

bug#32029: [PATCH] xref-find-definitions-at-mouse

[Tobias Gerdin]

Den 2018-07-03 kl. 15:15, skrev Dmitry Gutov:

> On 7/2/18 2:18 AM, Tobias Gerdin wrote:
>
>> +;;;###autoload
>> +(defun xref-find-definitions-at-mouse (event)
>> +  "Find the definition of identifier around mouse click."
>> +  (interactive "e")
>> +  (let* ((backend (xref-find-backend))
>> +     (identifier (save-excursion
>> +              (mouse-set-point event)
>> +              (xref-backend-identifier-at-point backend))))
>> +    (if identifier
>> +    (xref--find-definitions identifier nil)
>> +      (user-error "No identifier here"))))
>
> You should call `mouse-set-point` before `xref-find-backend`, because 
> the latter might conceivably depend on the value of point.
>
> I think this can be written much shorter (call mouse-set-point, then 
> interactively call xref-find-definitions), but I'm not sure how.
A new version below. The initial version was modeled after 
`ffap-at-point`. Make using of call-interactively would be neat, but 
since we want to avoid making the call to xref-find-definitions inside 
the body of save-excursion I do not see how either (unless one woud 
actually move the point which I find a bit aggressive). I am not an 
overly experienced Elisp programmer so if you happen to come up with a 
way to do this I'm all ears. If not the below would do the job until then.

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 9a437b6f69..befebbb426 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -873,6 +873,16 @@ With prefix argument, prompt for the identifier."
    (interactive (list (xref--read-identifier "Find references of: ")))
    (xref--find-xrefs identifier 'references identifier nil))

+;;;###autoload
+(defun xref-find-definitions-at-mouse (event)
+  "Find the definition of identifier around mouse click."
+  (interactive "e")
+  (if-let ((identifier (save-excursion
+             (mouse-set-point event)
+             (xref-backend-identifier-at-point (xref-find-backend)))))
+      (xref-find-definitions identifier)
+    (user-error "No identifier here")))
+
  (declare-function apropos-parse-pattern "apropos" (pattern))

  ;;;###autoload

bug#32029: [PATCH] xref-find-definitions-at-mouse

[Tobias Gerdin]

Den 2018-07-03 kl. 15:15, skrev Dmitry Gutov:
>> I think this can be written much shorter (call mouse-set-point, then 
>> interactively call xref-find-definitions), but I'm not sure how.
Actually, since the mark will be saved before the jump if one could 
arrange for saving the mark before the call to mouse-set-point and not 
saving it again before jumping the call to save-excursion would not be 
needed and xref-find-definitions could be called interactively. It 
appears that pushing the mark is set all the way down in 
`xref--show-xrefs` though, so unless setting this is parameterized 
somehow it does not look straigthforward to me. And not worth it in this 
case IMO.

bug#32029: [PATCH] xref-find-definitions-at-mouse

[Dmitry Gutov]

On 7/4/18 11:03 AM, Tobias Gerdin wrote:
> Den 2018-07-03 kl. 15:15, skrev Dmitry Gutov:
>>> I think this can be written much shorter (call mouse-set-point, then 
>>> interactively call xref-find-definitions), but I'm not sure how.
> Actually, since the mark will be saved before the jump if one could 
> arrange for saving the mark before the call to mouse-set-point and not 
> saving it again before jumping the call to save-excursion would not be 
> needed and xref-find-definitions could be called interactively.
Would that really be a desired behavior, though?

Regarding the patch, I'm fine with it, but I'd like someone else to 
confirm that the name of the new function makes sense.

bug#32029: [PATCH] xref-find-definitions-at-mouse

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 4 Jul 2018 15:24:25 +0300
> 
> Regarding the patch, I'm fine with it, but I'd like someone else to 
> confirm that the name of the new function makes sense.

Someone else here.  We have already several functions called
SOMETHING-at-mouse, so I think the proposed name does make sense.

Thanks.

bug#32029: [PATCH] xref-find-definitions-at-mouse

[Eli Zaretskii]

> From: Tobias Gerdin <tgerdin@gmail.com>
> Date: Tue, 3 Jul 2018 23:37:23 +0200
> 
> > You should call `mouse-set-point` before `xref-find-backend`, because 
> > the latter might conceivably depend on the value of point.
> >
> > I think this can be written much shorter (call mouse-set-point, then 
> > interactively call xref-find-definitions), but I'm not sure how.
> A new version below.

Dmitry, are you okay with having this on master?  Or is there
something else that should be done about this?

Thanks.

bug#32029: [PATCH] xref-find-definitions-at-mouse

[Dmitry Gutov]

On 7/6/18 11:55 AM, Eli Zaretskii wrote:

> Dmitry, are you okay with having this on master?  Or is there
> something else that should be done about this?

Yes.

A NEWS entry, I guess.

I mistakenly sent my last reply to emacs-devel instead of this bug thread.

bug#32029: [PATCH] xref-find-definitions-at-mouse

[Tobias Gerdin]

Den 2018-07-06 kl. 11:03, skrev Dmitry Gutov:

> On 7/6/18 11:55 AM, Eli Zaretskii wrote:
>
>> Dmitry, are you okay with having this on master?  Or is there
>> something else that should be done about this?
>
> Yes.
>
> A NEWS entry, I guess.
diff --git a/etc/NEWS b/etc/NEWS
index c92ee6e680..2bfe884987 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -165,6 +165,12 @@ from a remote host.
  This triggers to search the program on the remote host as indicated by
  'default-directory'.

++++
+** New function 'xref-find-definitions-at-mouse'.
+Allows jumping to a definition by clicking on the identifier. Needs to be
+bound to a mouse event. See the node "Mouse Buttons" in the Emacs manual
+for details.
+
  \f
  * Editing Changes in Emacs 27.1

bug#32029: [PATCH] xref-find-definitions-at-mouse

[Eli Zaretskii]

> Cc: 32029@debbugs.gnu.org
> From: Tobias Gerdin <tgerdin@gmail.com>
> Date: Fri, 6 Jul 2018 22:23:19 +0200
> 
> Den 2018-07-06 kl. 11:03, skrev Dmitry Gutov:
> 
> > On 7/6/18 11:55 AM, Eli Zaretskii wrote:
> >
> >> Dmitry, are you okay with having this on master?  Or is there
> >> something else that should be done about this?
> >
> > Yes.
> >
> > A NEWS entry, I guess.
> diff --git a/etc/NEWS b/etc/NEWS
> index c92ee6e680..2bfe884987 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -165,6 +165,12 @@ from a remote host.
>   This triggers to search the program on the remote host as indicated by
>   'default-directory'.
> 
> ++++
> +** New function 'xref-find-definitions-at-mouse'.
> +Allows jumping to a definition by clicking on the identifier. Needs to be
> +bound to a mouse event. See the node "Mouse Buttons" in the Emacs manual
> +for details.
> +

Thanks, I preferred to write the NEWS entry (and a suitable manual
change) myself, so as not to exhaust the amount of changes we can
accept from you without legal paperwork.

Would you like to start the copyright assignment paperwork at this
time, so that future contributions could be unlimited?

bug#32029: [PATCH] xref-find-definitions-at-mouse

[Eli Zaretskii]

> From: Tobias Gerdin <tgerdin@gmail.com>
> Date: Tue, 3 Jul 2018 23:37:23 +0200
> 
> A new version below.

Thanks, pushed to the master branch.