bug#22583: 25.1.50; find-function-search-for-symbol don't call find-function-after-hook

bug#22583: 25.1.50; find-function-search-for-symbol don't call find-function-after-hook

[Tino Calancha]

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


Hi emacs,

when calling find-function or find-variable
find-function-after-hook is run.
In the ther hand, find-function-search-for-symbol
don't run this hook.

For example,
I set find-function-after-hook to always visit source code
in a read-only buffer. This work for find-function/variable.

But sometimes, you call describe-function/describe-variable,
and, after reading the doc. string you decide to take a look
in the implementation. In that cases the buffer may not be
in read-only mode because find-function-after-hook is not called.


In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 2.24.29)
Repository revision: 90c647b97f0e62ca8bc2fc1d27f0170136277fb7

[-- Attachment #2: Type: text/plain, Size: 1207 bytes --]

diff --git a/find-func.el b/find-func.el
index 0575ce4..e05f75c 100644
--- a/find-func.el
+++ b/find-func.el
@@ -302,7 +302,10 @@ The search is done in the source for library LIBRARY."
   (while (and (symbolp symbol) (get symbol 'definition-name))
     (setq symbol (get symbol 'definition-name)))
   (if (string-match "\\`src/\\(.*\\.\\(c\\|m\\)\\)\\'" library)
-      (find-function-C-source symbol (match-string 1 library) type)
+      (let* ((buf-pos (find-function-C-source symbol (match-string 1 library) type))
+             (buf     (car buf-pos)))
+        (when buf (with-current-buffer buf
+                    (run-hooks 'find-function-after-hook))) buf-pos)
     (when (string-match "\\.el\\(c\\)\\'" library)
       (setq library (substring library 0 (match-beginning 1))))
     ;; Strip extension from .emacs.el to make sure symbol is searched in
@@ -340,6 +343,7 @@ The search is done in the source for library LIBRARY."
                                "\\_>")
                        nil t)))
 		(progn
+                  (run-hooks 'find-function-after-hook)
 		  (beginning-of-line)
 		  (cons (current-buffer) (point)))
 	      (cons (current-buffer) nil))))))))

bug#22583: 25.1.50; find-function-search-for-symbol don't call find-function-after-hook

[Dmitry Gutov]

On 02/07/2016 05:18 PM, Tino Calancha wrote:

> when calling find-function or find-variable
> find-function-after-hook is run.
> In the ther hand, find-function-search-for-symbol
> don't run this hook.

find-function-search-for-symbol is a querying function. It can be used 
by different consumers, e.g. xref-location-marker. I'm not sure the user 
would want find-function-after-hook to run in every such case.

Maybe help-variable-def's button definition's help-function should run 
the hook explicitly instead.

bug#22583: 25.1.50; find-function-search-for-symbol don't call find-function-after-hook

[Tino Calancha]

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


>find-function-search-for-symbol is a querying function. It can be used by different consumers, e.g. xref-location-marker.
>I'm not sure the user would want find-function-after-hook to run in every such case.
Right: the hook may be unnecessary called many times. Not good.

>Maybe help-variable-def's button definition's help-function should run the hook explicitly instead.
Thanks for the tip. This way looks more sensible.

[-- Attachment #2: Type: text/plain, Size: 830 bytes --]

diff --git a/help-mode.el b/help-mode.el
index 22e5386..9c512bd 100644
--- a/help-mode.el
+++ b/help-mode.el
@@ -202,6 +202,7 @@ The format is (FUNCTION ARGS...).")
 		   (let ((location
 			  (find-function-search-for-symbol fun type file)))
 		     (pop-to-buffer (car location))
+			 (run-hooks 'find-function-after-hook)
 		     (if (cdr location)
 			 (goto-char (cdr location))
 		       (message "Unable to find location in file"))))
@@ -231,6 +232,7 @@ The format is (FUNCTION ARGS...).")
 		     (setq file (help-C-file-name var 'var)))
 		   (let ((location (find-variable-noselect var file)))
 		     (pop-to-buffer (car location))
+		     (run-hooks 'find-function-after-hook)
 		     (if (cdr location)
 		       (goto-char (cdr location))
 		       (message "Unable to find location in file"))))

bug#22583: 25.1.50; find-library don't call find-function-after-hook

[Tino Calancha]

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


IMO, for consistency, find-function-after-hook should be
also run by find-library.
For instance, as mentioned above, i set this hook to always
open source code on read-only buffers.
If find-library don't run the hook, it may open a file in
writable mode.

[-- Attachment #2: Type: text/plain, Size: 718 bytes --]

diff --git a/lisp/emacs-lisp/find-func.el b/lisp/emacs-lisp/find-func.el
index 0575ce4..b04a9d2 100644
--- a/lisp/emacs-lisp/find-func.el
+++ b/lisp/emacs-lisp/find-func.el
@@ -283,7 +283,11 @@ LIBRARY should be a string (the name of the library)."
 			 "Library name: ")
 		       table nil nil nil nil def))))
   (let ((buf (find-file-noselect (find-library-name library))))
-    (condition-case nil (switch-to-buffer buf) (error (pop-to-buffer buf)))))
+    (condition-case nil
+        (prog1
+            (switch-to-buffer buf)
+          (run-hooks 'find-function-after-hook))
+      (error (pop-to-buffer buf)))))
 
 ;;;###autoload
 (defun find-function-search-for-symbol (symbol type library)

bug#22583: 25.1.50; find-library don't call find-function-after-hook

[Dmitry Gutov]

On 02/18/2016 02:52 AM, Tino Calancha wrote:
>
> IMO, for consistency, find-function-after-hook should be
> also run by find-library.
> For instance, as mentioned above, i set this hook to always
> open source code on read-only buffers.
> If find-library don't run the hook, it may open a file in
> writable mode.

Sounds good to me.

Tino, could you resend these as one patch, using 'git format-patch' as 
described in CONTRIBUTE, with an appropriate commit message (which means 
using ChangeLog-style description, as also described in CONTRIBUTE).

John, would you say it's okay for emacs-25?

bug#22583: 25.1.50; find-library don't call find-function-after-hook

[John Wiegley]

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

>>>>> Dmitry Gutov <dgutov@yandex.ru> writes:

> Tino, could you resend these as one patch, using 'git format-patch' as
> described in CONTRIBUTE, with an appropriate commit message (which means
> using ChangeLog-style description, as also described in CONTRIBUTE).

> John, would you say it's okay for emacs-25?

At this point, qualification for inclusion into emacs-25 is fairly strict (now
clarifying for the sake of future discussion):

  - It should *not* result in any changes to the NEWS file, APIs, or existing
    documentation. Not unless we missed something major.

  - If it fixes new behavior -- so, it hasn't been out in the wild yet -- then
    just fix it if you think it's not destabilizing.

  - If it corrects old behavior, sufficient testing is needed to ensure that
    this "right" behavior does not lead to surprising behavior after two
    pretests have already been out.

Be very cautious with changes at this point, since they won't have had the
benefit of those pretests. Especially if it's a breakage that has been broken
for a long time, I say it can stay broken until 25.2. But if we're smoothing
out new code before it hits the streets, there is still a little time for
that.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

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

bug#22583: 25.1.50; find-library don't call find-function-after-hook

[Dmitry Gutov]

On 02/22/2016 02:00 AM, John Wiegley wrote:

> At this point, qualification for inclusion into emacs-25 is fairly strict (now
> clarifying for the sake of future discussion):

Thanks, this is useful (especially the item 2), but could you also 
answer the question for this particular patch?

Like, I don't know if these changes should be reflected in the NEWS, or 
existing documentation.

bug#22583: 25.1.50; find-library don't call find-function-after-hook

[John Wiegley]

>>>>> Dmitry Gutov <dgutov@yandex.ru> writes:

> On 02/22/2016 02:00 AM, John Wiegley wrote:
>> At this point, qualification for inclusion into emacs-25 is fairly strict
>> (now clarifying for the sake of future discussion):

> Thanks, this is useful (especially the item 2), but could you also answer
> the question for this particular patch?

> Like, I don't know if these changes should be reflected in the NEWS, or
> existing documentation.

I'm not really sure about this particular change, not having followed the
whole thread.  Let me read it through today and get back to you.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#22583: 25.1.50; find-library don't call find-function-after-hook

[Tino Calancha]

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


> Tino, could you resend these as one patch, using 'git format-patch' as 
> described in CONTRIBUTE, with an appropriate commit message (which means 
> using ChangeLog-style description, as also described in CONTRIBUTE).
Attached the requested patch.

[-- Attachment #2: Type: text/plain, Size: 2149 bytes --]

From de28333e8797c5693876d8321c7c9f3255a13965 Mon Sep 17 00:00:00 2001
From: Tino Calancha <f92capac@gmail.com>
Date: Tue, 23 Feb 2016 01:52:00 +0900
Subject: [PATCH] ; Run find-function-after-hook after find a symbol

Run the hook when visiting the source code with find-library,
 describe-variable and describe-function (Bug#22583).
* lisp/emacs-lisp/find-func.el (find-library):
* lisp/help-mode.el (help-function-def, help-variable-def):
Run the hook inside the help-function of the buttons.
---
 lisp/emacs-lisp/find-func.el | 6 +++++-
 lisp/help-mode.el            | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/find-func.el b/lisp/emacs-lisp/find-func.el
index 0575ce4..b04a9d2 100644
--- a/lisp/emacs-lisp/find-func.el
+++ b/lisp/emacs-lisp/find-func.el
@@ -283,7 +283,11 @@ find-library
 			 "Library name: ")
 		       table nil nil nil nil def))))
   (let ((buf (find-file-noselect (find-library-name library))))
-    (condition-case nil (switch-to-buffer buf) (error (pop-to-buffer buf)))))
+    (condition-case nil
+        (prog1
+            (switch-to-buffer buf)
+          (run-hooks 'find-function-after-hook))
+      (error (pop-to-buffer buf)))))
 
 ;;;###autoload
 (defun find-function-search-for-symbol (symbol type library)
diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index e0f3351..f66fbb7 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -202,6 +202,7 @@ 'help-function-def
 		   (let ((location
 			  (find-function-search-for-symbol fun type file)))
 		     (pop-to-buffer (car location))
+			 (run-hooks 'find-function-after-hook)
 		     (if (cdr location)
 			 (goto-char (cdr location))
 		       (message "Unable to find location in file"))))
@@ -231,6 +232,7 @@ 'help-variable-def
 		     (setq file (help-C-file-name var 'var)))
 		   (let ((location (find-variable-noselect var file)))
 		     (pop-to-buffer (car location))
+		     (run-hooks 'find-function-after-hook)
 		     (if (cdr location)
 		       (goto-char (cdr location))
 		       (message "Unable to find location in file"))))
-- 
2.7.0

bug#22583: 25.1.50; find-library don't call find-function-after-hook

[Lars Magne Ingebrigtsen]

Tino Calancha <f92capac@gmail.com> writes:

>> Tino, could you resend these as one patch, using 'git format-patch'
>> as described in CONTRIBUTE, with an appropriate commit message
>> (which means using ChangeLog-style description, as also described in
>> CONTRIBUTE).
> Attached the requested patch.

Thanks; applied to the trunk.

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