Patch proposal: display symbol source code in help buffers

Patch proposal: display symbol source code in help buffers

[Arthur Miller]

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

Here is my second proposal to bring built-in help on par with external
offerings.

This one is little bit more than just three lines as the previous one was, so I
would really appreciate if I could get some thorough review and testing. It
works fine for me, but Emacs source code is big; so there is always possibility
that something is not displaying correctly. I had thoughts of writing automated
test, a scraper sort of like autloads collects help to collect docs from files
and than call new help routines and compare results, but that would be far more
involved than actually hacking help-mode :).

What I am worried about is how does it work with compressed sources? How do I
test it and what would I need to do to get it to work?

Do I need to manually decompress tar-ed sources or is it already taken care? I
believe, and I hope, that it is, but I am really not at home with how that part
of Emacs work. I appreciate advice and help there, also if someone can test.

Another thing: can I assume that all DEFVAR_LISP statements in .c files end with
a ');$'? I am using this in a regexp to fetch the entire source for those
defvars.

I have tried to be as least intrusive as I could, so everything is in
help-mode.

There is also a defcustom defaulting to nil to enable/disable this, and I think
it should always stay at nil since this can be cpu intensive. I think it
actually works quite well, I see no hickups, but I do run Emacs from surce
directory, and I use it as 'source-directory'. By the way, this patch expects
sources to be found in what this variable shows.

If this would be acceptable, I can write some NEWS and manual, unless someone
more english language savvy would like to help with that one.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Display-source-code-in-help-mode-buffers.patch --]
[-- Type: text/x-patch, Size: 5749 bytes --]

From 58eac63f759146cf7a601e1ae974dd373c837957 Mon Sep 17 00:00:00 2001
From: Arthur Miller <arthur.miller@live.com>
Date: Sun, 19 Sep 2021 21:28:01 +0200
Subject: [PATCH] Display source code in 'help-mode' buffers

* lisp/help-mode.el ('help-mode-inline-source'): New option.
('help--fetch-c-src'): New function.
('help--fetch-lisp-src'): New function.
('help--symbol-source'): New function.
('help-make-xrefs): Check for 'help-mode-inline-source' and
call 'help--symbol-source' to perform insertion when possible.
---
 lisp/help-mode.el | 110 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index 551cf7e1a3..935c54a6eb 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -149,6 +149,15 @@ help-mode-hook
   "Hook run by `help-mode'."
   :type 'hook
   :group 'help)
+
+(defcustom help-mode-inline-source nil
+  "Display inlined source code for SYMBOL in `help-mode' buffer.
+
+When enabled the source code of a symbol will be displayed inlined in
+the help buffer, if the source code for the symbol is available."
+  :type 'boolean
+  :group 'help
+  :version "28.1")
 \f
 ;; Button types used by help
 
@@ -503,6 +512,91 @@ describe-symbol-backends
     and a frame), inserts the description of that symbol in the current buffer
     and returns that text as well.")
 
+(defun help--fetch-c-src (sym type file)
+  "Find C source code for a Lisp symbol in a `file'.
+
+sym is the symbol to find.
+type is the type as obtained by 'describe-*' functions.
+file is the source file to search in."
+  (let (src pos)
+    (setq file (expand-file-name file source-directory))
+    (when (file-readable-p file)
+      (with-temp-buffer
+        (insert-file-contents-literally file)
+        (delay-mode-hooks (funcall 'c-mode))
+        (goto-char (point-min))
+        (unless type
+          ;; Either or both an alias and its target might be advised.
+          (setq sym (find-function-advised-original
+		            (indirect-function
+		             (find-function-advised-original sym)))))
+        (when (re-search-forward
+	       (if type
+		   (concat "DEFVAR[A-Z_]*[ \t\n]*([ \t\n]*\""
+			   (regexp-quote (symbol-name sym))
+			   "\"")
+	         (concat "DEFUN[ \t\n]*([ \t\n]*\""
+		         (regexp-quote (subr-name (advice--cd*r sym)))
+		         "\""))
+	       nil t)
+          (if type ;; defvar here
+              (progn
+                (goto-char (line-beginning-position))
+                (skip-chars-forward "[:blank:]")
+                (setq pos (point))
+                (re-search-forward ");$" nil t)
+                (narrow-to-region pos (point)))
+            (narrow-to-defun))
+          (if (fboundp 'font-lock-ensure)
+               (font-lock-ensure)
+             (with-no-warnings (font-lock-fontify-buffer)))
+          (setq src (buffer-string)))))
+    src))
+
+(defun help--fetch-lisp-src (sym type file)
+  "Find emacs-lisp source code for a Lisp symbol in a `file'.
+
+sym is the symbol to find.
+type is the type as obtained by 'describe-*' functions.
+file is the source file to search in."
+  (let (src pos sxp)
+    (when file
+      (setq file (or file (find-lisp-object-file-name sym type))))
+    (with-temp-buffer
+      (insert-file-contents file)
+      (delay-mode-hooks (funcall 'emacs-lisp-mode))
+      (require 'find-func)
+      (setq pos (cdr (find-function-search-for-symbol sym type file)))
+      (when pos
+        (goto-char pos)
+        (forward-sexp)
+        (setq sxp (buffer-substring-no-properties pos (point)))
+        (when sxp
+          (erase-buffer)
+          (insert sxp)
+          (if (fboundp 'font-lock-ensure)
+              (font-lock-ensure)
+            (with-no-warnings (font-lock-fontify-buffer)))
+          (setq src (buffer-string)))))
+    src))
+
+(defun help--symbol-source ()
+  "Fnd and return string to be inserted in help-mode buffer for the
+source code of the symbol.
+
+Used internally for `help-make-refs'."
+  (let* ((file (plist-get help-mode--current-data :file))
+        (type (plist-get help-mode--current-data :type))
+        (sym (plist-get help-mode--current-data :symbol))
+        src)
+    (when (eq file 'C-source)
+      (setq file
+            (help-C-file-name (indirect-function sym) 'fun)))
+    (setq src (if (string-suffix-p ".c" file)
+                  (help--fetch-c-src sym type file)
+                (help--fetch-lisp-src sym type file)))
+  (if src src "Source code not available.")))
+
 ;;;###autoload
 (defun help-make-xrefs (&optional buffer)
   "Parse and hyperlink documentation cross-references in the given BUFFER.
@@ -651,6 +745,22 @@ help-make-xrefs
           (while (and (not (bobp)) (bolp))
             (delete-char -1))
           (insert "\n")
+          ;; get source string if needed and available
+          (when help-mode-inline-source
+            (insert "\nSource Code: \n")
+            ;; describe-symbol does not produce 'current-data' plist
+            (unless help-mode--current-data
+              (save-excursion
+                (goto-char (point-min))
+                (when (re-search-forward "\\.\\(el\\|c\\)" nil t)
+                  (goto-char (- (point) 2))
+                  (let ((props (get-text-property (point) 'help-args)))
+                    (when props
+                      (setq help-mode--current-data
+                            (list :symbol (nth 0 props)
+                                  :file (nth 1 props))))))))
+              (insert (help--symbol-source))
+            (insert "\n"))
           (when (or help-xref-stack help-xref-forward-stack)
             (insert "\n"))
           ;; Make a back-reference in this buffer if appropriate.
-- 
2.33.0

Re: Patch proposal: display symbol source code in help buffers

[Lars Ingebrigtsen]

Arthur Miller <arthur.miller@live.com> writes:

> Here is my second proposal to bring built-in help on par with external
> offerings.

I haven't tried the patch, but this is for showing the source code of
the function/variable in question in the *Help* buffer?  I'm not quite
sure I understand the utility of that -- if you want to look at the
source, you can just hit the `s' key.  And then you get all the proper
language-related Emacs stuff in that buffer -- looking at sources is
usually nicer in the correct major mode than in other buffers.

So I don't understand who'd want this.

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

Re: Patch proposal: display symbol source code in help buffers

[Eli Zaretskii]

> From: Arthur Miller <arthur.miller@live.com>
> Date: Sun, 19 Sep 2021 21:50:35 +0200
> 
> Here is my second proposal to bring built-in help on par with external
> offerings.

Thanks.  Like Lars, I wonder why this is needed, given that we already
have a way to show the sources by a command invoked from the *Help*
buffer.  Can you tell how this feature makes a difference wrt what we
have already?

And a minor nit:

> * lisp/help-mode.el ('help-mode-inline-source'): New option.
> ('help--fetch-c-src'): New function.
> ('help--fetch-lisp-src'): New function.
> ('help--symbol-source'): New function.
> ('help-make-xrefs): Check for 'help-mode-inline-source' and
> call 'help--symbol-source' to perform insertion when possible.

We don't quote symbols inside the parentheses, only in the
descriptions that follow the colon, as you did in the last entry.

Thanks.

Re: Patch proposal: display symbol source code in help buffers

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I haven't tried the patch, but this is for showing the source code of
> the function/variable in question in the *Help* buffer?  I'm not quite
> sure I understand the utility of that -- if you want to look at the
> source, you can just hit the `s' key.  And then you get all the proper
> language-related Emacs stuff in that buffer -- looking at sources is
> usually nicer in the correct major mode than in other buffers.
>
> So I don't understand who'd want this.

I believe this is a feature in helpful.el:

https://github.com/Wilfred/helpful/blob/master/screenshots/helpful_source.png

FWIW, I personally also ask myself why/how this is useful.  Is it
because some users don't know that you can get to the source code?

That said, I don't see any reason not to have this as an optional
feature, if some users find that it works well for them.

Re: Patch proposal: display symbol source code in help buffers

[Lars Ingebrigtsen]

Stefan Kangas <stefankangas@gmail.com> writes:

> FWIW, I personally also ask myself why/how this is useful.  Is it
> because some users don't know that you can get to the source code?
>
> That said, I don't see any reason not to have this as an optional
> feature, if some users find that it works well for them.

The user would have to know about the user option to turn it on...  On
the other hand, as we've discussed in the "profile" issue, we might have
a profile that switches all stuff like this on, which makes the issue of
discovery moot.

I'm not against the feature per se, but I'm just puzzled that somebody
would want this feature.  :-)

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

Re: Patch proposal: display symbol source code in help buffers

[Gregor Zattler]

Hi Eli,
* Eli Zaretskii <eliz@gnu.org> [2021-09-20; 08:59]:
>> Here is my second proposal to bring built-in help on par with external
>> offerings.
>
> Thanks.  Like Lars, I wonder why this is needed, given that we already
> have a way to show the sources by a command invoked from the *Help*
> buffer.

I for instance did not know that hitting the `s' key would
show me the source.

> Can you tell how this feature makes a difference wrt what
> we have already?

First one does not have to know about the `s' keys function
in this case, then one does not have to hit the `s' key, its
simply there and easily ignored if not necessary.

Lastly hitting the `s' key opens another buffer in another
window (at least for me) and (also at least for me), at
surprising locations in my frame.  I have a UHD monitor and,
at least the way I configured it, emacs opens up to 4
windows, the help buffer and the source buffer not
necessarily adjunct.  All 4 windows can carry 92 lines, so
there is plenty of room below the text in the help buffer to
show more info.

helpful, the package, made life insofar easier for me.

Ciao; Gregor

Re: Patch proposal: display symbol source code in help buffers

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sun, 19 Sep 2021 23:09:51 -0700
> Cc: emacs-devel@gnu.org
> 
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
> > So I don't understand who'd want this.
> 
> I believe this is a feature in helpful.el:
> 
> https://github.com/Wilfred/helpful/blob/master/screenshots/helpful_source.png
> 
> FWIW, I personally also ask myself why/how this is useful.  Is it
> because some users don't know that you can get to the source code?
> 
> That said, I don't see any reason not to have this as an optional
> feature, if some users find that it works well for them.

So this is only for those who'd like to see the source by default,
instead of explicitly requesting that?

Re: Patch proposal: display symbol source code in help buffers

[Eli Zaretskii]

> From: Gregor Zattler <telegraph@gmx.net>
> Date: Mon, 20 Sep 2021 08:37:56 +0200
> 
> Lastly hitting the `s' key opens another buffer in another
> window (at least for me) and (also at least for me), at
> surprising locations in my frame.  I have a UHD monitor and,
> at least the way I configured it, emacs opens up to 4
> windows, the help buffer and the source buffer not
> necessarily adjunct.  All 4 windows can carry 92 lines, so
> there is plenty of room below the text in the help buffer to
> show more info.

But that window has the wrong mode for reading the source.  So it
might be a good "selling point", but once you are past "the drums",
using the display for seriously studying the source code is way less
convenient and efficient than having the source pop in its own window.
Right?

Re: Patch proposal: display symbol source code in help buffers

[Ihor Radchenko]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I'm not against the feature per se, but I'm just puzzled that somebody
> would want this feature.  :-)

Consider scenario when you are editing some-file.el and narrowed the
buffer to a defun A. Then, you need to examine source code of another
function B also defined in some-file.el.

Opening help buffer for B is easy - <F1> f <RET> with point at the B's
call. However, without source code being shown in the help buffer, if I
try to hit s, the narrowing is removed and the point is moved to a
function definition. Not convenient.

Of course, one can create indirect buffer for some-file.el, widen, and
jump to B's definition. However, I personally find help buffer with
source code more comfortable.

Best,
Ihor

Re: Patch proposal: display symbol source code in help buffers

[Stefan Kangas]

Ihor Radchenko <yantar92@gmail.com> writes:

> Consider scenario when you are editing some-file.el and narrowed the
> buffer to a defun A. Then, you need to examine source code of another
> function B also defined in some-file.el.
>
> Opening help buffer for B is easy - <F1> f <RET> with point at the B's
> call. However, without source code being shown in the help buffer, if I
> try to hit s, the narrowing is removed and the point is moved to a
> function definition. Not convenient.

I'm not arguing against the new feature here, but is the above a bug?

It seems to me that `help-view-source' should respect the users
narrowing, for example by querying to open a new indirect buffer or
something in that case.

Re: Patch proposal: display symbol source code in help buffers

[martin rudalics]

 > Lastly hitting the `s' key opens another buffer in another
 > window (at least for me) and (also at least for me), at
 > surprising locations in my frame.  I have a UHD monitor and,
 > at least the way I configured it, emacs opens up to 4
 > windows, the help buffer and the source buffer not
 > necessarily adjunct.  All 4 windows can carry 92 lines, so
 > there is plenty of room below the text in the help buffer to
 > show more info.

If you fit the window showing *Help* to its buffer, there is no room
left.  Where to pop up the location of the source and how to get rid of
it is a question we currently discuss in Bug#9054 and Bug#36767.  If and
when we reach a consensus there, we can add an option to immediately
display the source in the *Help* buffer or a window right below it (IIRC
a C source wouldn't be always available though).

martin

Re: Patch proposal: display symbol source code in help buffers

[martin rudalics]

 > It seems to me that `help-view-source' should respect the users
 > narrowing, for example by querying to open a new indirect buffer or
 > something in that case.

We now have 'widen-automatically' for that.

martin

Re: Patch proposal: display symbol source code in help buffers

[Ihor Radchenko]

Stefan Kangas <stefankangas@gmail.com> writes:

>> Opening help buffer for B is easy - <F1> f <RET> with point at the B's
>> call. However, without source code being shown in the help buffer, if I
>> try to hit s, the narrowing is removed and the point is moved to a
>> function definition. Not convenient.
>
> I'm not arguing against the new feature here, but is the above a bug?

My comment was to resolve your puzzled state about usefulness of the new
feature ;)

> It seems to me that `help-view-source' should respect the users
> narrowing, for example by querying to open a new indirect buffer or
> something in that case.

That would make sense. Yet, even without narrowing, I personally do not
like that the point moves from the defun I am working on to other
function definition.

Also, a subtle difference between showing the source code in help buffer
and jumping to the defun is how much text is shown. In the help buffer,
source code is effectively "narrowed" to defun - no surrounding code is
shown.

Best,
Ihor

Re: Patch proposal: display symbol source code in help buffers

[Dmitry Gutov]

On 20.09.2021 09:37, Gregor Zattler wrote:
>>> Here is my second proposal to bring built-in help on par with external
>>> offerings.
>> Thanks.  Like Lars, I wonder why this is needed, given that we already
>> have a way to show the sources by a command invoked from the*Help*
>> buffer.
> I for instance did not know that hitting the `s' key would
> show me the source.
> 

Help buffers also have a visible button (at the end of the first line) 
which takes you to the source.

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

Stefan Kangas <stefankangas@gmail.com> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> I haven't tried the patch, but this is for showing the source code of
>> the function/variable in question in the *Help* buffer?

Yes, that is a feature of Helpful. We had some discussion last week, which I
started by asking if we can get Helpful in i Emacs. I thought to have Helpful as
"advanced" help enabled in Emacs as-is, but some peopel expressed that would be
better to refactor it. Also, when looking at Helfpul sources it would some
refactoring to reomve dependency on some external libraries. Since Helfpul does
not use Emacs help-mode buffer, and nothing of built-in help-mode infrastructure
but implements everythin on it's own, I am of opinion that it takes less effort
to implement comparable features from scratch. Helpful author(s) have also not
said anything, either for or against, I cc:d willfred when I posted on the mail
list. 

>>                                                          I'm not quite
>> sure I understand the utility of that

To me it has been useful when I was learning, it still is; it is kind of nice to
be able to see the code together with the docs. It also makes, at least me, more
inclined to look at the sources when it is thrown at my face directly to see
what is going on, than if I have to take extra action to look at the sources.

>> sure I understand the utility of that -- if you want to look at the
>> source, you can just hit the `s' key.  And then you get all the proper
>> language-related Emacs stuff in that buffer -- looking at sources is
>> usually nicer in the correct major mode than in other buffers.

Yes, I agree that it is easy to hit 's' key. Often I would like to see functions
called in a function, and it that case I would anyway open the source file. But
there are cases when it is good enough to just glimpse through the source and
confirm what is going on. Most of the times I don't want to look at the source
at all, but if it is already there, I am actually more inclined to look at the
source than if it wasn't there.

Another feature compared to Helfpul (or anti-feature, depends whom you ask), is
that built-in help does not focus the help buffer, while Helpful shifts
focus. Many time I just want to glimpse on the source to confirm if my
understanding is correct, and I will maybe scroll other buffer (I like
two-buffer setup), so I prefer my cursor not to jump over to help buffer every
time as it does with helpful.

>> So I don't understand who'd want this.

People who are used to Helpful; people not used to Elisp at all, 

> I believe this is a feature in helpful.el:
>
> https://github.com/Wilfred/helpful/blob/master/screenshots/helpful_source.png
>
> FWIW, I personally also ask myself why/how this is useful.  Is it
> because some users don't know that you can get to the source code?
>
> That said, I don't see any reason not to have this as an optional
> feature, if some users find that it works well for them.

Of course, this can be/is cpu intensive so it should always be an opt-in
feature. I have put in a defcustom to enable/disable this, and it is nil by
default. I think that my implementation is slightly more effective than one in
Helpful, but that would be just very slightly, and probably does not matter when
it comes to compressed sources which have to be decompressed to be shown.

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

Stefan Kangas <stefankangas@gmail.com> writes:

> Ihor Radchenko <yantar92@gmail.com> writes:
>
>> Consider scenario when you are editing some-file.el and narrowed the
>> buffer to a defun A. Then, you need to examine source code of another
>> function B also defined in some-file.el.
>>
>> Opening help buffer for B is easy - <F1> f <RET> with point at the B's
>> call. However, without source code being shown in the help buffer, if I
>> try to hit s, the narrowing is removed and the point is moved to a
>> function definition. Not convenient.
>
> I'm not arguing against the new feature here, but is the above a bug?
>
> It seems to me that `help-view-source' should respect the users
> narrowing, for example by querying to open a new indirect buffer or
> something in that case.

Is that a bug in my patch or in the original help functionality?

I am not working with files buffers if they are open. I insert file from the
disk, at least I hope that is the way how insrt-file-content works, into a
temporary buffer, and I work solely in that temporary buffer, when I am done I
copy buffer-string into the help buffer.

Otherwise if it does screw up user's narrowing, than it is a bug, at least
not-intended behaviour. I would than appreciate a tip what is the cause so I can
fix it.

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arthur Miller <arthur.miller@live.com>
>> Date: Sun, 19 Sep 2021 21:50:35 +0200
>> 
>> Here is my second proposal to bring built-in help on par with external
>> offerings.
>
> Thanks.  Like Lars, I wonder why this is needed, given that we already
> have a way to show the sources by a command invoked from the *Help*
> buffer.  Can you tell how this feature makes a difference wrt what we
> have already?

I just answered to I think Lars or Stefan's answer to Lars, where I elaborate a
bit more why is it useful, if you can peek there, it would be nice so I dont'
need to re-write everything. Short summary:

- people new to Emacs are maybe not so inclined to look at the code, having code
  auto displayed makes at least me more inclined to actually look at the sources
  so, I think it has some educational purpose
- I don't always want to deep dive into the source, sometimes I just glimpse
  over to confirm my (mis)understanding :).
- no extra action needed to take to see the source (switch to buffer, hit 's)
- sometimes the source is just a wrapper, so I will need to perform a search in
  the source file, here I can actually hit C-h f on the symbol in help buffer
  and opened that function directly
- people are used to Helpful
- it is on a defcustom, nil by default, so having it as an opt-in does not cost
  much 

> And a minor nit:
>
>> * lisp/help-mode.el ('help-mode-inline-source'): New option.
>> ('help--fetch-c-src'): New function.
>> ('help--fetch-lisp-src'): New function.
>> ('help--symbol-source'): New function.
>> ('help-make-xrefs): Check for 'help-mode-inline-source' and
>> call 'help--symbol-source' to perform insertion when possible.
>
> We don't quote symbols inside the parentheses, only in the
> descriptions that follow the colon, as you did in the last entry.

Aha, yes yes, thanks, I'll remember that for the next time.

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 20.09.2021 09:37, Gregor Zattler wrote:
>>>> Here is my second proposal to bring built-in help on par with external
>>>> offerings.
>>> Thanks.  Like Lars, I wonder why this is needed, given that we already
>>> have a way to show the sources by a command invoked from the*Help*
>>> buffer.
>> I for instance did not know that hitting the `s' key would
>> show me the source.
>> 
>
> Help buffers also have a visible button (at the end of the first line) which
> takes you to the source.

Yes, but with all the consekvence as Gregor wrote (new buffer in new location,
unless buffer is already open).

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gregor Zattler <telegraph@gmx.net>
>> Date: Mon, 20 Sep 2021 08:37:56 +0200
>> 
>> Lastly hitting the `s' key opens another buffer in another
>> window (at least for me) and (also at least for me), at
>> surprising locations in my frame.  I have a UHD monitor and,
>> at least the way I configured it, emacs opens up to 4
>> windows, the help buffer and the source buffer not
>> necessarily adjunct.  All 4 windows can carry 92 lines, so
>> there is plenty of room below the text in the help buffer to
>> show more info.
>
> But that window has the wrong mode for reading the source.  So it
> might be a good "selling point", but once you are past "the drums",
> using the display for seriously studying the source code is way less
> convenient and efficient than having the source pop in its own window.
> Right?

Yes and no. If you really wish to study the implementation, than you will open
the source file of course, maybe do some changes, eval your chanes and see the
effect, run some in debugger, and use more advanced tools. But there we speak
about serious study of a feture. Sometimes it is enough to just take a peek at
something. Even while working in some source file, I might prefer to show help
for a function from some other file and not to jump to that other file. It is
like a little source code browser, where one can browse between code for
symbols.

Also, not everybody knows how built-in buffer works, as Gregor mentioned, and if
one just wishes to take a quick look on the source, not switching the buffer
from working buffer is a nice feature. I think I wrote that one as a point in
the other answer to you.

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Arthur Miller <arthur.miller@live.com> writes:
>
>> Here is my second proposal to bring built-in help on par with external
>> offerings.
>
> I haven't tried the patch, but this is for showing the source code of
> the function/variable in question in the *Help* buffer?  I'm not quite
> sure I understand the utility of that -- if you want to look at the
> source, you can just hit the `s' key.  And then you get all the proper
> language-related Emacs stuff in that buffer -- looking at sources is
> usually nicer in the correct major mode than in other buffers.
>
> So I don't understand who'd want this.

Sorry Lars, I was reading mails from top-to bottom, so I see this mail after I
have already answered to Stefan K. (and you indirectly) and Eli. Is it ok if you
take a peek in those two mails, since I have elaborated quite in those
two? It feels a bit phony now to repeat both of those answers again :).

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

Ihor Radchenko <yantar92@gmail.com> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> I'm not against the feature per se, but I'm just puzzled that somebody
>> would want this feature.  :-)
>
> Consider scenario when you are editing some-file.el and narrowed the
> buffer to a defun A. Then, you need to examine source code of another
> function B also defined in some-file.el.
>
> Opening help buffer for B is easy - <F1> f <RET> with point at the B's
> call. However, without source code being shown in the help buffer, if I
> try to hit s, the narrowing is removed and the point is moved to a
> function definition. Not convenient.
>
> Of course, one can create indirect buffer for some-file.el, widen, and
> jump to B's definition. However, I personally find help buffer with
> source code more comfortable.
>
> Best,
> Ihor

Oh, I saw just part of your mail answered by Stefan K (I think) and completely
missunderstood :). Should read entire thread before I answer anything.

I just wrote to Eli in other answer; sometimes when you work with a source file,
it is convenient to just display a source of a function from other file, without
switching to other file.

Sometimes it is convenient to first read the docs, and than to maybe see the
source, and if it is not enough, than to open the source file.

It is all about the workflow, and on per-case basis.

And yes, this one does not screw narrowing at all; at least I have not intended to.

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Kangas <stefankangas@gmail.com>
>> Date: Sun, 19 Sep 2021 23:09:51 -0700
>> Cc: emacs-devel@gnu.org
>> 
>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>> 
>> > So I don't understand who'd want this.
>> 
>> I believe this is a feature in helpful.el:
>> 
>> https://github.com/Wilfred/helpful/blob/master/screenshots/helpful_source.png
>> 
>> FWIW, I personally also ask myself why/how this is useful.  Is it
>> because some users don't know that you can get to the source code?
>> 
>> That said, I don't see any reason not to have this as an optional
>> feature, if some users find that it works well for them.
>
> So this is only for those who'd like to see the source by default,
> instead of explicitly requesting that?

Yes of course! A strictly opt-in feature. Defcustom is for that purpose.

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

martin rudalics <rudalics@gmx.at> writes:

>> Lastly hitting the `s' key opens another buffer in another
>> window (at least for me) and (also at least for me), at
>> surprising locations in my frame.  I have a UHD monitor and,
>> at least the way I configured it, emacs opens up to 4
>> windows, the help buffer and the source buffer not
>> necessarily adjunct.  All 4 windows can carry 92 lines, so
>> there is plenty of room below the text in the help buffer to
>> show more info.
>
> If you fit the window showing *Help* to its buffer, there is no room
> left.

It would be nice to make it fit to N lines in height, and C columns in width,
i.e. to make it fit to certain size.

>     .  Where to pop up the location of the source and how to get rid of
> it is a question we currently discuss in Bug#9054 and Bug#36767.

There are so many discussions here so I wasn't following those. I really don't
know what you have discussed there. I come to this from my own needs.

>                                                                   If and
> when we reach a consensus there, we can add an option to immediately
> display the source in the *Help* buffer or a window right below it (IIRC
> a C source wouldn't be always available though).

This shouldn't be by any mean in collision what you are saying here. What you
describe, as I understand it, is to display source on-demand, in help buffer,
which would be very nice too. What I have done is to display by global
switch. It would be still useful to have on-demand display when global switch is
off, and it would be trivial to re-build the patch to include that. I can
rebuild and send new one. I have found a bug and I need to rebuild-it anyway. I
can also refactor it and add an interactive function that can be boudn to a key
to display only sources in help buffer. And if global switch is on, and sources
are requested, it can function as a "narrow" button, i.e. fit only symbol source
to the help buffer. Would that be interesting?

Honestly, I dislike to display both help and entire source, because there is a
duplication of the doc string. I was always disturbed by this duplication in
helpful too. So I have experimented to display only the body of the function,
but I don't think that looked very nice either. There is a bug repport I did
about 'narrow-to-defun', and the help-mode.el I posted there, shows C sources
narrowed only to it's function body. 

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

Ihor Radchenko <yantar92@gmail.com> writes:

> Stefan Kangas <stefankangas@gmail.com> writes:
>
>>> Opening help buffer for B is easy - <F1> f <RET> with point at the B's
>>> call. However, without source code being shown in the help buffer, if I
>>> try to hit s, the narrowing is removed and the point is moved to a
>>> function definition. Not convenient.
>>
>> I'm not arguing against the new feature here, but is the above a bug?
>
> My comment was to resolve your puzzled state about usefulness of the new
> feature ;)
Yes. Thank you very much :). I realized that later. It seems like I was reading
thread upside-down. Just me being goofy after nights of baby feeding and 3 hours
cycle living. 

>> It seems to me that `help-view-source' should respect the users
>> narrowing, for example by querying to open a new indirect buffer or
>> something in that case.
>
> That would make sense. Yet, even without narrowing, I personally do not
> like that the point moves from the defun I am working on to other
> function definition.
>
> Also, a subtle difference between showing the source code in help buffer
> and jumping to the defun is how much text is shown. In the help buffer,
> source code is effectively "narrowed" to defun - no surrounding code is
> shown.
>
> Best,
> Ihor

Re: Patch proposal: display symbol source code in help buffers

[Lars Ingebrigtsen]

Ihor Radchenko <yantar92@gmail.com> writes:

> That would make sense. Yet, even without narrowing, I personally do not
> like that the point moves from the defun I am working on to other
> function definition.

Yes, that is pretty annoying.  Perhaps
`help-function-def--button-function' should push-mark before going to
the position?  Then `C-x C-x' would bring you back to where you were...

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

Re: Patch proposal: display symbol source code in help buffers

[Lars Ingebrigtsen]

Arthur Miller <arthur.miller@live.com> writes:

> Another feature compared to Helfpul (or anti-feature, depends whom you
> ask), is that built-in help does not focus the help buffer, while
> Helpful shifts focus.

(This is the `help-window-select' user option in Emacs, by the way.)

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

Re: Patch proposal: display symbol source code in help buffers

[Ihor Radchenko]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Ihor Radchenko <yantar92@gmail.com> writes:
>
>> That would make sense. Yet, even without narrowing, I personally do not
>> like that the point moves from the defun I am working on to other
>> function definition.
>
> Yes, that is pretty annoying.  Perhaps
> `help-function-def--button-function' should push-mark before going to
> the position?  Then `C-x C-x' would bring you back to where you were...

I like this idea. Maybe we can simply change the default value of
find-function-after-hook like in the attached patch?

Best,
Ihor


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Save-position-in-mark-ring-before-jumping-to-definit.patch --]
[-- Type: text/x-diff, Size: 1605 bytes --]

From 31f841b1e3921f7b2039656ed8f409b3b715741f Mon Sep 17 00:00:00 2001
Message-Id: <31f841b1e3921f7b2039656ed8f409b3b715741f.1632207397.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Tue, 21 Sep 2021 14:50:52 +0800
Subject: [PATCH] Save position in mark ring before jumping to definition

* lisp/emacs-lisp/find-func.el (find-function-after-hook): `push-mark'
is added to default value of the hook.

Following up:
https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01561.html
---
 etc/NEWS                     | 3 +++
 lisp/emacs-lisp/find-func.el | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 2bdcb6434b..2132e8babc 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -648,7 +648,10 @@ clicking the "X" icon in the tool bar.
 
 ---
 *** 'g' ('revert-buffer') in 'help-mode' no longer requires confirmation.
+*** Jumping to function/variable source now saves mark before moving point
+Jumping to source from "*Help*" buffer moves the point when the source buffer is already open.  Now, the old point is pushed to mark ring.
 
+---
 ** File Locks
 
 +++
diff --git a/lisp/emacs-lisp/find-func.el b/lisp/emacs-lisp/find-func.el
index 303039d653..b984fb3870 100644
--- a/lisp/emacs-lisp/find-func.el
+++ b/lisp/emacs-lisp/find-func.el
@@ -165,7 +165,7 @@ find-function-recenter-line
   :group 'find-function
   :version "20.3")
 
-(defcustom find-function-after-hook nil
+(defcustom find-function-after-hook '(push-mark)
   "Hook run after finding symbol definition.
 
 See the functions `find-function' and `find-variable'."
-- 
2.32.0

Re: Patch proposal: display symbol source code in help buffers

[Stefan Kangas]

Ihor Radchenko <yantar92@gmail.com> writes:

> I like this idea. Maybe we can simply change the default value of
> find-function-after-hook like in the attached patch?

We generally prefer not to add hooks like this if we can avoid it.  We
can usually just change the code instead.

For example, in this case maybe we could just call `push-mark' at the
end of `help-function-def--button-function'.

Re: Patch proposal: display symbol source code in help buffers

[Stefan Kangas]

Arthur Miller <arthur.miller@live.com> writes:

> Is that a bug in my patch or in the original help functionality?

IIUC, it is in the original help functionality.

Re: Patch proposal: display symbol source code in help buffers

[martin rudalics]

 >> If you fit the window showing *Help* to its buffer, there is no room
 >> left.
 >
 > It would be nice to make it fit to N lines in height, and C columns in width,
 > i.e. to make it fit to certain size.

Who would specify that "certain size"?

 >>      .  Where to pop up the location of the source and how to get rid of
 >> it is a question we currently discuss in Bug#9054 and Bug#36767.
 >
 > There are so many discussions here so I wasn't following those. I really don't
 > know what you have discussed there. I come to this from my own needs.

Similar issues like how to navigate to the source of a function or
variable from the current point of the user's interest.

 > This shouldn't be by any mean in collision what you are saying here. What you
 > describe, as I understand it, is to display source on-demand, in help buffer,
 > which would be very nice too. What I have done is to display by global
 > switch. It would be still useful to have on-demand display when global switch is
 > off, and it would be trivial to re-build the patch to include that. I can
 > rebuild and send new one. I have found a bug and I need to rebuild-it anyway. I
 > can also refactor it and add an interactive function that can be boudn to a key
 > to display only sources in help buffer. And if global switch is on, and sources
 > are requested, it can function as a "narrow" button, i.e. fit only symbol source
 > to the help buffer. Would that be interesting?

What we should establish first is what users want.  Eli and Lars have
explained their workflow in the threads I cited including how and why
they want to go to the source in the first place, where to go from there
and how to return to the initial state.  Few others have so far.

martin

Re: Patch proposal: display symbol source code in help buffers

[martin rudalics]

 > Yes, that is pretty annoying.  Perhaps
 > `help-function-def--button-function' should push-mark before going to
 > the position?  Then `C-x C-x' would bring you back to where you were...

Let's add this to a list of things TODO for *Help* buffers.

martin

Re: Patch proposal: display symbol source code in help buffers

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Tue, 21 Sep 2021 00:41:22 -0700
> Cc: Arthur Miller <arthur.miller@live.com>, emacs-devel@gnu.org
> 
> Ihor Radchenko <yantar92@gmail.com> writes:
> 
> > I like this idea. Maybe we can simply change the default value of
> > find-function-after-hook like in the attached patch?
> 
> We generally prefer not to add hooks like this if we can avoid it.  We
> can usually just change the code instead.
> 
> For example, in this case maybe we could just call `push-mark' at the
> end of `help-function-def--button-function'.

Indeed, especially since the proposed change to find-func.el would
affect all users of that package.

Re: Patch proposal: display symbol source code in help buffers

[Ihor Radchenko]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> Ihor Radchenko <yantar92@gmail.com> writes:
>> 
>> > I like this idea. Maybe we can simply change the default value of
>> > find-function-after-hook like in the attached patch?
>> 
>> We generally prefer not to add hooks like this if we can avoid it.  We
>> can usually just change the code instead.
>> 
>> For example, in this case maybe we could just call `push-mark' at the
>> end of `help-function-def--button-function'.
>
> Indeed, especially since the proposed change to find-func.el would
> affect all users of that package.

Agree. I updated the patch and also added support for
widen-automatically variable. See below.

Best,
Ihor


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Save-position-in-mark-ring-before-jumping-to-definit.patch --]
[-- Type: text/x-diff, Size: 1555 bytes --]

From e94ae3711796679e330637921b980f970b9d7025 Mon Sep 17 00:00:00 2001
Message-Id: <e94ae3711796679e330637921b980f970b9d7025.1632215537.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Tue, 21 Sep 2021 17:00:50 +0800
Subject: [PATCH 1/2] Save position in mark ring before jumping to definition

* lisp/help-mode.el (help-function-def--button-function): Current
point is saved in the mark ring before jumping to definition.

Following up:
https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01561.html
---
 etc/NEWS          | 3 +++
 lisp/help-mode.el | 1 +
 2 files changed, 4 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index 2bdcb6434b..2132e8babc 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -648,7 +648,10 @@ clicking the "X" icon in the tool bar.
 
 ---
 *** 'g' ('revert-buffer') in 'help-mode' no longer requires confirmation.
+*** Jumping to function/variable source now saves mark before moving point
+Jumping to source from "*Help*" buffer moves the point when the source buffer is already open.  Now, the old point is pushed to mark ring.
 
+---
 ** File Locks
 
 +++
diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index 551cf7e1a3..57655db337 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -268,6 +268,7 @@ help-function-def--button-function
             (when (or (< position (point-min))
                       (> position (point-max)))
               (widen))
+            (push-mark nil t)
             (goto-char position))
         (message "Unable to find location in file")))))
 
-- 
2.32.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Honor-widen-automatically-in-help-function-def-butto.patch --]
[-- Type: text/x-diff, Size: 1573 bytes --]

From 109bd7192b5b672508b9fa1d53c10a5a8aec3846 Mon Sep 17 00:00:00 2001
Message-Id: <109bd7192b5b672508b9fa1d53c10a5a8aec3846.1632215537.git.yantar92@gmail.com>
In-Reply-To: <e94ae3711796679e330637921b980f970b9d7025.1632215537.git.yantar92@gmail.com>
References: <e94ae3711796679e330637921b980f970b9d7025.1632215537.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Tue, 21 Sep 2021 17:10:10 +0800
Subject: [PATCH 2/2] Honor widen-automatically in
 help-function-def--button-function

* lisp/help-mode.el (help-function-def--button-function): Ask user to
widen the buffer when `widen-automatically' is set to nil.
---
 lisp/help-mode.el | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index 57655db337..5d566ca083 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -263,11 +263,14 @@ help-function-def--button-function
       (pop-to-buffer (car location))
       (run-hooks 'find-function-after-hook)
       (if position
-          (progn
+          (catch :exit
             ;; Widen the buffer if necessary to go to this position.
             (when (or (< position (point-min))
                       (> position (point-max)))
-              (widen))
+              (if (or widen-automatically
+                      (yes-or-no-p "The location is outside narrowing. Widen? "))
+                  (widen)
+                (throw :exit nil)))
             (push-mark nil t)
             (goto-char position))
         (message "Unable to find location in file")))))
-- 
2.32.0

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

martin rudalics <rudalics@gmx.at> writes:

>>> If you fit the window showing *Help* to its buffer, there is no room
>>> left.
>>
>> It would be nice to make it fit to N lines in height, and C columns in width,
>> i.e. to make it fit to certain size.
>
> Who would specify that "certain size"?
User, of course, who else? We put some default size and a defcuctom for user to
change, as always?

>>>      .  Where to pop up the location of the source and how to get rid of
>>> it is a question we currently discuss in Bug#9054 and Bug#36767.
>>
>> There are so many discussions here so I wasn't following those. I really don't
>> know what you have discussed there. I come to this from my own needs.
>
> Similar issues like how to navigate to the source of a function or
> variable from the current point of the user's interest.

I have tested yesterday, and it works well as it is in help buffer. It is like a
mini browser. I am sure there are possibly 1001 different ways to do things; but
somewhere we have to make a choice and do something. If the people wish
something different from how it works, we can always build on it later. As I
understand a lot of people use Helpful, and the buil-in help works even better
considering cross linking. I can click in built-in help and get new functions
and vars opened in the help buffer already; I can't do that in Helpful. Let's
have a face lift for built in help for 28. I am sure we could debate for years
and always someone will like it differently. Let's have something that works.

>> This shouldn't be by any mean in collision what you are saying here. What you
>> describe, as I understand it, is to display source on-demand, in help buffer,
>> which would be very nice too. What I have done is to display by global
>> switch. It would be still useful to have on-demand display when global switch is
>> off, and it would be trivial to re-build the patch to include that. I can
>> rebuild and send new one. I have found a bug and I need to rebuild-it anyway. I
>> can also refactor it and add an interactive function that can be boudn to a key
>> to display only sources in help buffer. And if global switch is on, and sources
>> are requested, it can function as a "narrow" button, i.e. fit only symbol source
>> to the help buffer. Would that be interesting?
>
> What we should establish first is what users want.  Eli and Lars have
> explained their workflow in the threads I cited including how and why
> they want to go to the source in the first place, where to go from there
> and how to return to the initial state.  Few others have so far.

I  haven't read the other thread, but I have used help and why I think it is
good, and some other have as well.

Can you try what I send in yesterday, with soucre view toggle? I think it works
quite nice (I did a misstake and went 1 row too far when deleting, but that's a
trivial to fix).

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Arthur Miller <arthur.miller@live.com> writes:
>
>> Another feature compared to Helfpul (or anti-feature, depends whom you
>> ask), is that built-in help does not focus the help buffer, while
>> Helpful shifts focus.
>
> (This is the `help-window-select' user option in Emacs, by the way.)

Ah, didn't know about that one :). Should have looked more. I prefer to not
select the help window automatically, because I don't like to shift focus from
what I work with.

Re: Patch proposal: display symbol source code in help buffers

[Lars Ingebrigtsen]

Ihor Radchenko <yantar92@gmail.com> writes:

> Agree. I updated the patch and also added support for
> widen-automatically variable. See below.

[...]

> diff --git a/lisp/help-mode.el b/lisp/help-mode.el
> index 551cf7e1a3..57655db337 100644
> --- a/lisp/help-mode.el
> +++ b/lisp/help-mode.el
> @@ -268,6 +268,7 @@ help-function-def--button-function
>              (when (or (< position (point-min))
>                        (> position (point-max)))
>                (widen))
> +            (push-mark nil t)

I was thinking something slightly more subtle.  That is, if we've just
opened the file (i.e., it has not been displayed before), we should not
push the mark.  And if point didn't move, we shouldn't push the mark
either, I think.

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

Re: Patch proposal: display symbol source code in help buffers

[martin rudalics]

 >>> It would be nice to make it fit to N lines in height, and C columns in width,
 >>> i.e. to make it fit to certain size.
 >>
 >> Who would specify that "certain size"?
 > User, of course, who else? We put some default size and a defcuctom for user to
 > change, as always?

Do you mean "one size fits all"?  I'm afraid I don't understand what you
intend here.

 > Let's
 > have a face lift for built in help for 28. I am sure we could debate for years
 > and always someone will like it differently. Let's have something that works.

That something will take its time to be written.  Certainly not before
Emacs 28 comes out.

 > I  haven't read the other thread, but I have used help and why I think it is
 > good, and some other have as well.

There are two threads and I think you should read them if you plan to
change anything in this area.

 > Can you try what I send in yesterday, with soucre view toggle? I think it works
 > quite nice (I did a misstake and went 1 row too far when deleting, but that's a
 > trivial to fix).

Compiling it tells me

../../lisp/help-mode.el:530:22: Warning: the function
     ‘find-function-advised-original’ is not known to be defined.

so it should require 'find-func'.  A few remarks:

+(defcustom help-mode-inline-source nil
+  "Display inlined source code for SYMBOL in `help-mode' buffer.

What is SYMBOL here?

+  "Find C source code for a Lisp symbol in a `file'.
+
+sym is the symbol to find.
+type is the type as obtained by 'describe-*' functions.
+file is the source file to search in."

Arguments should be uppercase and not hyphenated.

+             (with-no-warnings (font-lock-fontify-buffer)))

Do you know how long it takes to fontify xdisp.c?  With a non-optimized
build, C-h v for 'scroll-minibuffer-conservatively' takes more than 10
seconds here.

Other than that I can only say that this is _one_ possible option for
showing the source of the code.  BTW, we should decide on a suitable
interface for 'help-toggle-source-view' probably using the button for
showing the code.

martin

Re: Patch proposal: display symbol source code in help buffers

[Juri Linkov]

>>> If you fit the window showing *Help* to its buffer, there is no room
>>> left.
>>
>> It would be nice to make it fit to N lines in height, and C columns in width,
>> i.e. to make it fit to certain size.
>
> Who would specify that "certain size"?

In bug#45688 I proposed on displaying the Help buffer to call a hook
like temp-buffer-window-setup-hook or temp-buffer-window-show-hook
with the default value shrink-window-if-larger-than-buffer or
fit-window-to-buffer.  Then users can customize it and remove
resizing that will make it more useful for anyone who needs
to reuse the Help window to visit source code.

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

martin rudalics <rudalics@gmx.at> writes:

>>>> It would be nice to make it fit to N lines in height, and C columns in width,
>>>> i.e. to make it fit to certain size.
>>>
>>> Who would specify that "certain size"?
>> User, of course, who else? We put some default size and a defcuctom for user to
>> change, as always?
>
> Do you mean "one size fits all"?
Of course not :).

I said "user", because I meant an user should have the final word. I just said in
short words that there should be a defcustom variable for users to customize the
default value. But also every value that is in use, has to have some default
value to start with, no? I didn't proposed a hadcoded literal, I proposed a
defcustom variable, so users can set this their liking.

>> Let's
>> have a face lift for built in help for 28. I am sure we could debate for years
>> and always someone will like it differently. Let's have something that works.
>
> That something will take its time to be written.  Certainly not before
> Emacs 28 comes out.
>
>> I  haven't read the other thread, but I have used help and why I think it is
>> good, and some other have as well.
>
> There are two threads and I think you should read them if you plan to
> change anything in this area.
>
>> Can you try what I send in yesterday, with soucre view toggle? I think it works
>> quite nice (I did a misstake and went 1 row too far when deleting, but that's a
>> trivial to fix).
>
> Compiling it tells me
>
> ../../lisp/help-mode.el:530:22: Warning: the function
>     ‘find-function-advised-original’ is not known to be defined.
>
> so it should require 'find-func'.  A few remarks:
Thank you for spotting that one! I used to have it in fetch-c-src, but than I
removed it, and than refactored that code from 'find-function-C-source' but
forgott that it might require find-func.

> +(defcustom help-mode-inline-source nil
> +  "Display inlined source code for SYMBOL in `help-mode' buffer.
>
> What is SYMBOL here?

Current symbol in the help buffer. There is always a 'current symbol' in the
help buffer, as I understand the implementation, the one that help was asked
about. I can reword the docs. I have tried to be short as possible so it fits in
one line. I see also now that it shoudl at least be 'the SYMBOL', but I can
probably just skip "for SYMBOL" completely.

> +  "Find C source code for a Lisp symbol in a `file'.
> +
> +sym is the symbol to find.
> +type is the type as obtained by 'describe-*' functions.
> +file is the source file to search in."
>
> Arguments should be uppercase and not hyphenated.

Ok, thank you, I wasn't thinking about that one. I'll fix that of course.

> +             (with-no-warnings (font-lock-fontify-buffer)))
>
> Do you know how long it takes to fontify xdisp.c?  With a non-optimized
> build, C-h v for 'scroll-minibuffer-conservatively' takes more than 10
> seconds here.
I would't be surprised. I try to do as little work as possible: only function
oor defvar in question is fontified, so xdisp.c will never get fontified all at
once in this context. If that would still be problematic for low-level CPUs I
can of course do this optionally as well, with another defcustom.

I am really concerned with efficiency; I always am. I really like Helpful, and I
am not the only one, it is a popular library. What is problem is that it
does not use any built-in functionality but provides everything from scratch. It
has complete implementation of a help buffer on its own. So it is a dubble
functionality when I import it. I like it's implementation too, it is simpler
than built-in help infrastructure. So I hoped to get it into Emacs, and let old
help die out :). It won't happen so the next best thing is to re-implement Helpful
in terms of Emacs built-in features, i.e. bring Helpful features (not the source
code) to built-in *Help* buffers. At least some of them.

> Other than that I can only say that this is _one_ possible option for
> showing the source of the code.  BTW, we should decide on a suitable
> interface for 'help-toggle-source-view' probably using the button for
> showing the code.

Yes, I had my thoughts to propertize the "Source Code:" string in help buffer to
act similar as other link (or buttons as they are called in Emacs), but I just
haven't got to that yet. Also until there is any decision to use
'help-toggle-source-view' as an API, it can wait for a bit; but generally yes,
it should be another help-xref button.

I have also explicitly coded it so it can be used from any buffer, so user does
not need to switch to a help buffer to toggle the source view on/off. I see it
as a feature that help does not switch my cursor to help buffer by default :).

Thank you for your input Martin, that was really helpful. Some pun unavaoided
here :).

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

Juri Linkov <juri@linkov.net> writes:

>>>> If you fit the window showing *Help* to its buffer, there is no room
>>>> left.
>>>
>>> It would be nice to make it fit to N lines in height, and C columns in width,
>>> i.e. to make it fit to certain size.
>>
>> Who would specify that "certain size"?
>
> In bug#45688 I proposed on displaying the Help buffer to call a hook
> like temp-buffer-window-setup-hook or temp-buffer-window-show-hook
> with the default value shrink-window-if-larger-than-buffer or
> fit-window-to-buffer.  Then users can customize it and remove
> resizing that will make it more useful for anyone who needs
> to reuse the Help window to visit source code.

That sounds like useful approach. Something like:

(defcustom help-buffer-default-height 25
  "The default height of a help-buffer window."
  :type 'fixnum
  :group 'help
  :version "28.1") <-- :-)

Re: Patch proposal: display symbol source code in help buffers

[martin rudalics]

 > That sounds like useful approach. Something like:
 >
 > (defcustom help-buffer-default-height 25
 >    "The default height of a help-buffer window."
 >    :type 'fixnum
 >    :group 'help
 >    :version "28.1") <-- :-)

Help-buffer windows are as a rule displayed via 'display-buffer' and are
easily recognizable because their buffer is always called *Help*.  So
you can simply add a 'window-height' action alist entry for them which
is considerably more versatile than what you propose above.

martin

Re: Patch proposal: display symbol source code in help buffers

[Juri Linkov]

>> That sounds like useful approach. Something like:
>>
>> (defcustom help-buffer-default-height 25
>>    "The default height of a help-buffer window."
>>    :type 'fixnum
>>    :group 'help
>>    :version "28.1") <-- :-)
>
> Help-buffer windows are as a rule displayed via 'display-buffer' and are
> easily recognizable because their buffer is always called *Help*.  So
> you can simply add a 'window-height' action alist entry for them which
> is considerably more versatile than what you propose above.

Actually, I meant that when temp-buffer-resize-mode is enabled,
how would it be possible to resize everything except the Help buffer?

Re: Patch proposal: display symbol source code in help buffers

[martin rudalics]

 > Actually, I meant that when temp-buffer-resize-mode is enabled,
 > how would it be possible to resize everything except the Help buffer?

We can't.  We'd have to provide a white- or blacklist for that purpose.

martin

Re: Patch proposal: display symbol source code in help buffers

[Arthur Miller]

martin rudalics <rudalics@gmx.at> writes:

>> That sounds like useful approach. Something like:
>>
>> (defcustom help-buffer-default-height 25
>>    "The default height of a help-buffer window."
>>    :type 'fixnum
>>    :group 'help
>>    :version "28.1") <-- :-)
>
> Help-buffer windows are as a rule displayed via 'display-buffer' and are
> easily recognizable because their buffer is always called *Help*.  So
> you can simply add a 'window-height' action alist entry for them which
> is considerably more versatile than what you propose above.

Yes, I know; but think of new users comming from other editors and applications,
having no experience with Lisp and never heard of conses and alists.

I use display-buffer-alist to achieve something with *Help* buffer (and some
others) myself. I posted that snippet in the other thread I don't know if all
that stuff  existed back in time when original poster posted his bug repport
(2011), but at least nowadays, the bug should be marked as resolved, since it is
possible to tell Emacs to reuse Help buffer and window, instead of opening new
ones.

However, I don't think my solution to that problem, same as you suggest here,
is very convenient to new users. People are used to see some option they can
toggle on/off, some slider they can pull left right, some value they can pull
from a list or enter into some box. I think it is more helpful to see some
variable with clear and descriptive name, I think new users would appreciate
that. But that is just my opinion.

Anyway, I have read both threads you have refered too. One of them, the
Florians, (Bug#9054), is completely unrelated to what this patch does. Well it
is about help buffer, but there you are concerned with wath happens *before* user
enters the help buffer and in which way user will come to that buffer.

>        Where to pop up the location of the source and how to get rid of
> it is a question we currently discuss in Bug#9054

And that is exactly what I say above; this patch is completely not concerned
with where and how you will pop that window and source code file. I am concerned
with what happens in a help buffer, after user requested help, *not* how user come
to that point. I don't even pop a source file; just code for a function or var.

I wish to make help buffer more usable, because I wisth to go away from Helpful
and I see no reason why we should wait for something unrelated you guys had 10
years to get consensus on but didn't :). Sorry, that one was hard not to pick on
:).

I know I sound biased, but the suggested patch works above my personal
expectations. Suddenly the help buffer acts like a code browser. I really
suggest you to try and click a bit around. It wasn't my intention, since I never
do that in Helpful. I even don't know if it is possible there, but due to
back/forward functionality in built-in help buffer, it works really nice.

There is one bug I know off: it can't display code for *some* aliases, for
example 'find' which is alias for cl-find. Some other aliases works
fine. Besides that, I am very happy with it.

>                                                                   If and
> when we reach a consensus there, we can add an option to immediately
> display the source in the *Help* buffer or a window right below it

I would like to cite Eli here from the other thread, and say "life is too
short." :)

I am sure you guys will come up with something wonderful and amazing, but untill
you reach concensus, I'll go and work on some other options, because the
proposed patch is my proposal. It came up completely unrelated and independent
of that discussion, but it is touching the things you are discussing there. If
the code is bad you can improve on it and rework it, but as I understand my
proposal seems not be accepted, since I see you guys discuss options in the other
thread (Bug#36767), which I have already implemented and demoed. Nobody but
Tassilo has commented on, so I guess nobody has neither cared to try it nor see
the demo.

Anyway, thanks for your input in the other comment, it was helpful.

Re: Patch proposal: display symbol source code in help buffers

[martin rudalics]

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

 > We can't.  We'd have to provide a white- or blacklist for that purpose.

Actually we have such a blacklist already - it's 'display-buffer-alist'.
Please have a look at the attached diffs.

martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: resize-temp-buffer-window.diff --]
[-- Type: text/x-patch; name="resize-temp-buffer-window.diff", Size: 3113 bytes --]

diff --git a/lisp/help.el b/lisp/help.el
index 29ae340481..b67b5a4237 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -1589,6 +1589,16 @@ temp-buffer-resize-mode
       (add-hook 'temp-buffer-show-hook 'resize-temp-buffer-window 'append)
     (remove-hook 'temp-buffer-show-hook 'resize-temp-buffer-window)))

+(defun resize-temp-buffer-window-inhibit (buffer &optional horizontal)
+  "Return non-nil if windows on BUFFER should not be resized automatically.
+`resize-temp-buffer-window' calls this function to check whether
+`display-buffer-alist' contains a setting that inhibits
+auto-resizing of any window showing BUFFER."
+  (assq
+   (if horizontal 'window-width 'window-height)
+   (display-buffer-assq-regexp
+    (buffer-name buffer) display-buffer-alist nil)))
+
 (defun resize-temp-buffer-window (&optional window)
   "Resize WINDOW to fit its contents.
 WINDOW must be a live window and defaults to the selected one.
@@ -1607,26 +1617,33 @@ resize-temp-buffer-window
 This function may call `preserve-window-size' to preserve the
 size of WINDOW."
   (setq window (window-normalize-window window t))
-  (let ((height (if (functionp temp-buffer-max-height)
+  (let* ((buffer (window-buffer window))
+         (height (if (functionp temp-buffer-max-height)
+		     (with-selected-window window
+		       (funcall temp-buffer-max-height buffer))
+		   temp-buffer-max-height))
+	 (width (if (functionp temp-buffer-max-width)
 		    (with-selected-window window
-		      (funcall temp-buffer-max-height (window-buffer)))
-		  temp-buffer-max-height))
-	(width (if (functionp temp-buffer-max-width)
-		   (with-selected-window window
-		     (funcall temp-buffer-max-width (window-buffer)))
-		 temp-buffer-max-width))
-	(quit-cadr (cadr (window-parameter window 'quit-restore))))
+		      (funcall temp-buffer-max-width buffer))
+		  temp-buffer-max-width))
+	 (quit-cadr (cadr (window-parameter window 'quit-restore))))
     ;; Resize WINDOW iff it was made by `display-buffer'.
     (when (or (and (eq quit-cadr 'window)
 		   (or (and (window-combined-p window)
 			    (not (eq fit-window-to-buffer-horizontally
 				     'only))
-			    (pos-visible-in-window-p (point-min) window))
+			    (pos-visible-in-window-p
+                             (with-current-buffer buffer (point-min))
+                             window)
+                            (not (resize-temp-buffer-window-inhibit buffer)))
 		       (and (window-combined-p window t)
-			    fit-window-to-buffer-horizontally)))
+			    fit-window-to-buffer-horizontally
+                            (not (resize-temp-buffer-window-inhibit buffer t)))))
 	      (and (eq quit-cadr 'frame)
                    fit-frame-to-buffer
-                   (eq window (frame-root-window window))))
+                   (eq window (frame-root-window window))
+                   (not (resize-temp-buffer-window-inhibit buffer))
+                   (not (resize-temp-buffer-window-inhibit buffer t))))
 	(fit-window-to-buffer window height nil width nil t))))

 ;;; Help windows.

Re: Patch proposal: display symbol source code in help buffers

[martin rudalics]

 >> Help-buffer windows are as a rule displayed via 'display-buffer' and are
 >> easily recognizable because their buffer is always called *Help*.  So
 >> you can simply add a 'window-height' action alist entry for them which
 >> is considerably more versatile than what you propose above.
 >
 > Yes, I know; but think of new users comming from other editors and applications,
 > having no experience with Lisp and never heard of conses and alists.

Such users likely do not run 'temp-buffer-resize-mode' either.

 > I use display-buffer-alist to achieve something with *Help* buffer (and some
 > others) myself. I posted that snippet in the other thread I don't know if all
 > that stuff  existed back in time when original poster posted his bug repport
 > (2011), but at least nowadays, the bug should be marked as resolved, since it is
 > possible to tell Emacs to reuse Help buffer and window, instead of opening new
 > ones.

I got lost.  Which bug report is that?

 > However, I don't think my solution to that problem, same as you suggest here,
 > is very convenient to new users. People are used to see some option they can
 > toggle on/off, some slider they can pull left right, some value they can pull
 > from a list or enter into some box. I think it is more helpful to see some
 > variable with clear and descriptive name, I think new users would appreciate
 > that. But that is just my opinion.

The solution I suggested is for users who know well what they want.  New
users should be content with the default and, after experimenting with
it for some time, 'temp-buffer-resize-mode'.  BTW, most Emacs developers
use the default, I presume.

 > Anyway, I have read both threads you have refered too. One of them, the
 > Florians, (Bug#9054), is completely unrelated to what this patch does. Well it
 > is about help buffer, but there you are concerned with wath happens *before* user
 > enters the help buffer and in which way user will come to that buffer.
 >
 >>         Where to pop up the location of the source and how to get rid of
 >> it is a question we currently discuss in Bug#9054
 >
 > And that is exactly what I say above; this patch is completely not concerned
 > with where and how you will pop that window and source code file. I am concerned
 > with what happens in a help buffer, after user requested help, *not* how user come
 > to that point. I don't even pop a source file; just code for a function or var.

Sorry but did you really read bug report 9054?  That report says

   Two windows. Run `describe-function' on a function: The help buffer pops
   up in the other window. `other-window' and click the link to display the
   source file: my original buffer is gone. Very annoying.

   What I would like: Either a function that displays the source for the
   function at point in the other window or for the *Help* buffer to open
   the source file in the same window (i.e. the window which displays the
   *Help* buffer).

so it clearly talks about what happens *after* the user entered the help
buffer and not as you claim above "there you are concerned with wath
happens *before* user enters the help buffer and in which way user will
come to that buffer".

 > I wish to make help buffer more usable, because I wisth to go away from Helpful
 > and I see no reason why we should wait for something unrelated you guys had 10
 > years to get consensus on but didn't :). Sorry, that one was hard not to pick on
 > :).
 >
 > I know I sound biased, but the suggested patch works above my personal
 > expectations. Suddenly the help buffer acts like a code browser.

I can't speak for others but IMHO a code browser should operate only on
the code itself and not on a copy of it.  I wouldn't even want to clone
a buffer or use an indirect buffer for that purpose.  For me it's an
essential aspect of free software that browsing and hacking code go hand
in hand.

 > I really
 > suggest you to try and click a bit around. It wasn't my intention, since I never
 > do that in Helpful. I even don't know if it is possible there, but due to
 > back/forward functionality in built-in help buffer, it works really nice.

Sorry but I won't be of much help in this regard.

martin

Re: Patch proposal: display symbol source code in help buffers

[Juri Linkov]

>> We can't.  We'd have to provide a white- or blacklist for that purpose.
>
> Actually we have such a blacklist already - it's 'display-buffer-alist'.
> Please have a look at the attached diffs.

I guess this feature has effect when display-buffer-alist is customized
to contain (window-height . nil)?  Then I confirm it works, thanks!
Now finally it's possible to disable resizing for a limited set of buffers.

Re: Patch proposal: display symbol source code in help buffers

[martin rudalics]

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

 > I guess this feature has effect when display-buffer-alist is customized
 > to contain (window-height . nil)?  Then I confirm it works, thanks!

It's inherently flawed because it works only for 'display-buffer-alist'
- I couldn't justify that when documenting it.  I attached a patch that
addresses that (and fixes some other unrelated bugs).

martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: resize-temp-buffer-window.diff --]
[-- Type: text/x-patch; name="resize-temp-buffer-window.diff", Size: 14181 bytes --]

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 510efaf271..a5abfdb4c0 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -1334,6 +1334,11 @@ Temporary Displays
 (@pxref{Resizing Windows}) for resizing.  You can specify a different
 function by customizing the options @code{temp-buffer-max-height} and
 @code{temp-buffer-max-width} below.
+
+The effect of this option can be overridden by providing a suitable
+@code{window-height}, @code{window-width} or @code{window-size} action
+alist entry for @code{display-buffer} (@pxref{Buffer Display Action
+Alists}).
 @end defopt

 @defopt temp-buffer-max-height
diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 679744884a..63af4c5fd9 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -2924,9 +2924,9 @@ Buffer Display Action Alists
 selected, if the window chosen by @code{display-buffer} is displayed
 there.  Primarily affected by this are
 @code{display-buffer-use-some-frame} and
-@code{display-buffer-reuse-window}.
-@code{display-buffer-pop-up-frame} should be affected as well, but
-there is no guarantee that the window manager will comply.
+@code{display-buffer-reuse-window}.  Ideally,
+@code{display-buffer-pop-up-frame} should be affected as well, but there
+is no guarantee that the window manager will comply.

 @vindex window-parameters@r{, a buffer display action alist entry}
 @item window-parameters
@@ -2972,8 +2972,8 @@ Buffer Display Action Alists
 If the value specifies a function, that function is called with one
 argument---the chosen window.  The function is supposed to adjust the
 height of the window; its return value is ignored.  Suitable functions
-are @code{shrink-window-if-larger-than-buffer} and
-@code{fit-window-to-buffer}, see @ref{Resizing Windows}.
+are @code{fit-window-to-buffer} and
+@code{shrink-window-if-larger-than-buffer}, see @ref{Resizing Windows}.
 @end itemize

 By convention, the height of the chosen window is adjusted only if the
@@ -3007,11 +3007,31 @@ Buffer Display Action Alists
 width of the window; its return value is ignored.
 @end itemize

-By convention, the width of the chosen window is adjusted only if the
-window is part of a horizontal combination (@pxref{Windows and
-Frames}) to avoid changing the width of other, unrelated windows.
-Also, this entry should be processed under only certain conditions
-which are specified right below this list.
+@vindex window-size@r{, a buffer display action alist entry}
+@item window-size
+This entry is a combination of the two preceding ones and can be used to
+adjust the chosen window's height and width.  Since windows can be
+resized in one direction only without affecting other windows,
+@code{window-size} is effective only to set up the size of a window
+appearing alone on a frame.  The value can be one of the following:
+
+@itemize @bullet
+@item
+@code{nil} means to leave the size of the chosen window alone.
+
+@item
+A cons cell of two integers specifies the desired total width and height
+of the chosen window in lines and columns.  It's effect is to adjust the
+size of the frame accordingly.
+
+@item
+If the value specifies a function, that function is called with one
+argument---the chosen window.  The function is supposed to adjust the
+size of the window's frame; its return value is ignored.
+@end itemize
+
+This entry should be processed under only certain conditions which are
+specified right below this list.

 @vindex dedicated@r{, a buffer display action alist entry}
 @item dedicated
@@ -3112,6 +3132,14 @@ Buffer Display Action Alists
 buffer and never was used to show another buffer until it was reused
 by the current invocation of @code{display-buffer}.

+If no @code{window-height}, @code{window-width} or @code{window-size}
+entry was specified, the window may still be resized automatically when
+the buffer is temporary and @code{temp-buffer-resize-mode} has been
+enabled, @ref{Temporary Displays}.  In that case, the @sc{cdr} of a
+@code{window-height}, @code{window-height} or @code{window-size} entry
+can be used to inhibit or override the default behavior of
+@code{temp-buffer-resize-mode} for specific buffers or invocations of
+@code{display-buffer}.

 @node Choosing Window Options
 @subsection Additional Options for Displaying Buffers
diff --git a/lisp/help.el b/lisp/help.el
index 8f77167040..a5cdc9b786 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -1589,10 +1589,16 @@ temp-buffer-resize-mode
       (add-hook 'temp-buffer-show-hook 'resize-temp-buffer-window 'append)
     (remove-hook 'temp-buffer-show-hook 'resize-temp-buffer-window)))

+(defvar resize-temp-buffer-window-inhibit nil
+  "Non-nil means `resize-temp-buffer-window' should not resize.")
+
 (defun resize-temp-buffer-window (&optional window)
   "Resize WINDOW to fit its contents.
 WINDOW must be a live window and defaults to the selected one.
-Do not resize if WINDOW was not created by `display-buffer'.
+Do not resize if WINDOW was not created by `display-buffer'.  Do
+not resize either if a `window-height', `window-width' or
+`window-size' entry in `display-buffer-alist' prescribes some
+alternative resizing for WINDOW's buffer.

 If WINDOW is part of a vertical combination, restrain its new
 size by `temp-buffer-max-height' and do not resize if its minimum
@@ -1607,27 +1613,33 @@ resize-temp-buffer-window
 This function may call `preserve-window-size' to preserve the
 size of WINDOW."
   (setq window (window-normalize-window window t))
-  (let ((height (if (functionp temp-buffer-max-height)
+  (let* ((buffer (window-buffer window))
+         (height (if (functionp temp-buffer-max-height)
+		     (with-selected-window window
+		       (funcall temp-buffer-max-height buffer))
+		   temp-buffer-max-height))
+	 (width (if (functionp temp-buffer-max-width)
 		    (with-selected-window window
-		      (funcall temp-buffer-max-height (window-buffer)))
-		  temp-buffer-max-height))
-	(width (if (functionp temp-buffer-max-width)
-		   (with-selected-window window
-		     (funcall temp-buffer-max-width (window-buffer)))
-		 temp-buffer-max-width))
-	(quit-cadr (cadr (window-parameter window 'quit-restore))))
-    ;; Resize WINDOW iff it was made by `display-buffer'.
+		      (funcall temp-buffer-max-width buffer))
+		  temp-buffer-max-width))
+	 (quit-cadr (cadr (window-parameter window 'quit-restore))))
+    ;; Resize WINDOW only if it was made by `display-buffer'.
     (when (or (and (eq quit-cadr 'window)
 		   (or (and (window-combined-p window)
 			    (not (eq fit-window-to-buffer-horizontally
 				     'only))
-			    (pos-visible-in-window-p (point-min) window))
+			    (pos-visible-in-window-p
+                             (with-current-buffer buffer (point-min))
+                             window)
+                            (not resize-temp-buffer-window-inhibit))
 		       (and (window-combined-p window t)
-			    fit-window-to-buffer-horizontally)))
+			    fit-window-to-buffer-horizontally
+                            (not resize-temp-buffer-window-inhibit))))
 	      (and (eq quit-cadr 'frame)
                    fit-frame-to-buffer
-                   (eq window (frame-root-window window))))
-	(fit-window-to-buffer window height nil width nil t))))
+                   (eq window (frame-root-window window))
+                   (not resize-temp-buffer-window-inhibit)))
+      (fit-window-to-buffer window height nil width nil t))))

 ;;; Help windows.
 (defcustom help-window-select nil
diff --git a/lisp/window.el b/lisp/window.el
index b240b16f24..0eb26b0644 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -108,11 +108,15 @@ temp-buffer-window-setup
       ;; Return the buffer.
       buffer)))

+;; Defined in help.el.
+(defvar resize-temp-buffer-window-inhibit)
+
 (defun temp-buffer-window-show (buffer &optional action)
   "Show temporary buffer BUFFER in a window.
 Return the window showing BUFFER.  Pass ACTION as action argument
 to `display-buffer'."
-  (let (window frame)
+  (let ((resize-temp-buffer-window-inhibit nil)
+        window)
     (with-current-buffer buffer
       (set-buffer-modified-p nil)
       (setq buffer-read-only t)
@@ -130,9 +134,9 @@ temp-buffer-window-show
 		       t
 		     window-combination-limit)))
 	      (setq window (display-buffer buffer action)))
-	(setq frame (window-frame window))
-	(unless (eq frame (selected-frame))
-	  (raise-frame frame))
+        ;; We used to raise the window's frame here.  Do not do that
+        ;; since it would override an `inhibit-switch-frame' entry
+        ;; specified for the action alist used by `display-buffer'.
 	(setq minibuffer-scroll-window window)
 	(set-window-hscroll window 0)
 	(with-selected-window window
@@ -7245,11 +7249,14 @@ window--display-buffer
             (inhibit-modification-hooks t))
         (funcall (cdr (assq 'body-function alist)) window)))

-    (let ((quit-restore (window-parameter window 'quit-restore))
-	  (height (cdr (assq 'window-height alist)))
-	  (width (cdr (assq 'window-width alist)))
-	  (size (cdr (assq 'window-size alist)))
-	  (preserve-size (cdr (assq 'preserve-size alist))))
+    (let* ((quit-restore (window-parameter window 'quit-restore))
+	   (window-height (assq 'window-height alist))
+           (height (cdr window-height))
+	   (window-width (assq 'window-width alist))
+           (width (cdr window-width))
+           (window-size (assq 'window-size alist))
+           (size (cdr window-size))
+	   (preserve-size (cdr (assq 'preserve-size alist))))
       (cond
        ((or (eq type 'frame)
 	    (and (eq (car quit-restore) 'same)
@@ -7266,14 +7273,18 @@ window--display-buffer
 		(height (cdr size))
 		(frame (window-frame window)))
 	    (when (and (numberp width) (numberp height))
-	      (set-frame-height
-	       frame (+ (frame-height frame)
-			(- height (window-total-height window))))
-	      (set-frame-width
-	       frame (+ (frame-width frame)
-			(- width (window-total-width window)))))))
+              ;; Modifying the parameters of a newly created frame might
+              ;; not work everywhere, but then `temp-buffer-resize-mode'
+              ;; will certainly fail in a similar fashion.
+	      (modify-frame-parameters
+	       frame `((height . ,(+ (frame-height frame)
+			             (- height (window-total-height window))))
+                       (width . ,(+ (frame-width frame)
+			            (- width (window-total-width window))))))))
+          (setq resize-temp-buffer-window-inhibit t))
 	 ((functionp size)
-	  (ignore-errors (funcall size window)))))
+	  (ignore-errors (funcall size window))
+          (setq resize-temp-buffer-window-inhibit t))))
        ((or (eq type 'window)
 	    (and (eq (car quit-restore) 'same)
 		 (eq (nth 1 quit-restore) 'window)))
@@ -7293,9 +7304,11 @@ window--display-buffer
 		 (delta (- new-height (window-total-height window))))
 	    (when (and (window--resizable-p window delta nil 'safe)
 		       (window-combined-p window))
-	      (window-resize window delta nil 'safe))))
+	      (window-resize window delta nil 'safe)))
+          (setq resize-temp-buffer-window-inhibit 'vertical))
 	 ((functionp height)
-	  (ignore-errors (funcall height window))))
+	  (ignore-errors (funcall height window))
+          (setq resize-temp-buffer-window-inhibit 'vertical)))
 	;; Adjust width of window if asked for.
 	(cond
 	 ((not width))
@@ -7309,9 +7322,11 @@ window--display-buffer
 		 (delta (- new-width (window-total-width window))))
 	    (when (and (window--resizable-p window delta t 'safe)
 		       (window-combined-p window t))
-	      (window-resize window delta t 'safe))))
+	      (window-resize window delta t 'safe)))
+          (setq resize-temp-buffer-window-inhibit 'horizontal))
 	 ((functionp width)
-	  (ignore-errors (funcall width window))))
+	  (ignore-errors (funcall width window))
+          (setq resize-temp-buffer-window-inhibit 'horizontal)))
 	;; Preserve window size if asked for.
 	(when (consp preserve-size)
 	  (window-preserve-size window t (car preserve-size))
@@ -7556,9 +7571,11 @@ display-buffer
 Action alist entries are:
  `inhibit-same-window' -- A non-nil value prevents the same
     window from being used for display.
- `inhibit-switch-frame' -- A non-nil value prevents any frame
-    used for showing the buffer from being raised or selected.
- `reusable-frames' -- The value specifies the set of frames to
+`inhibit-switch-frame' -- A non-nil value prevents any frame used
+    for showing the buffer from being raised or selected.  Note
+    that a window manager may still raise a new frame and give it
+    focus, effectively overriding the value specified here.
+`reusable-frames' -- The value specifies the set of frames to
     search for a window that already displays the buffer.
     Possible values are nil (the selected frame), t (any live
     frame), visible (any visible frame), 0 (any visible or
@@ -7581,7 +7598,16 @@ display-buffer
     window) or a function to be called with one argument - the
     chosen window.  The function is supposed to adjust the width
     of the window; its return value is ignored.
- `preserve-size' -- The value should be either (t . nil) to
+
+ `window-size' -- This entry is only useful for windows appearing
+    alone on their frame and specifies the desired size of that
+    window either as a cons of integers (the total width and
+    height of the window on that frame), or a function to be
+    called with one argument - the chosen window.  The function
+    is supposed to adjust the size of the frame; its return value
+    is ignored.
+
+`preserve-size' -- The value should be either (t . nil) to
     preserve the width of the chosen window, (nil . t) to
     preserve its height or (t . t) to preserve its height and
     width in future changes of the window configuration.

Re: Patch proposal: display symbol source code in help buffers

[Ihor Radchenko]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I was thinking something slightly more subtle.  That is, if we've just
> opened the file (i.e., it has not been displayed before), we should not
> push the mark.  And if point didn't move, we shouldn't push the mark
> either, I think.

I agree that there is not much reason to push mark when point does not
move.

I slightly disagree about the case of opening new file.  I personally
find it useful when the first element added to mark ring is BOB.  When
cycling across mark ring, BOB is a nice visual indication that I already
looked across the whole ring, especially if I am searching for mark that
is actually not in the mark ring (though I thought otherwise).

On the other hand, it is rather a matter of personal preference.  Not
pushing BOB to mark ring might be a better default.  Even if it not, we
would probably need to change other places across the code to add BOB
consistently.  That's a whole other discussion.

See the updated patch.  Hopefully I got the news entry for Emacs 29.1
right.

Best,
Ihor


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Save-position-in-mark-ring-before-jumping-to-definit.patch --]
[-- Type: text/x-diff, Size: 1763 bytes --]

From e8d71f7e9fe111965f81b8627f27530d2ce80048 Mon Sep 17 00:00:00 2001
Message-Id: <e8d71f7e9fe111965f81b8627f27530d2ce80048.1633071433.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Fri, 1 Oct 2021 14:56:43 +0800
Subject: [PATCH] Save position in mark ring before jumping to definition

* lisp/help-mode.el (help-function-def--button-function): Current
point is saved in the mark ring before jumping to definition.

Following up:
https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01561.html
---
 etc/NEWS          | 3 +++
 lisp/help-mode.el | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index d0e41baaeb..1dfcc1ae09 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -29,6 +29,9 @@ applies, and please also update docstrings as needed.
 
 \f
 * Changes in Emacs 29.1
+** Help
+*** Jumping to function/variable source now saves mark before moving point
+Jumping to source from "*Help*" buffer moves the point when the source buffer is already open.  Now, the old point is pushed to mark ring.
 
 \f
 * Editing Changes in Emacs 29.1
diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index 0b404fe89f..0dc4229006 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -273,6 +273,11 @@ help-function-def--button-function
             (when (or (< position (point-min))
                       (> position (point-max)))
               (widen))
+            ;; Save mark for the old location, unless we just opened
+            ;; the buffer or the point is not actually going to move.
+            (unless (or (= 1 (point))
+                        (= (point) position))
+              (push-mark nil t))
             (goto-char position))
         (message "Unable to find location in file")))))
 
-- 
2.32.0

Re: Patch proposal: display symbol source code in help buffers

[Lars Ingebrigtsen]

Ihor Radchenko <yantar92@gmail.com> writes:

> I slightly disagree about the case of opening new file.  I personally
> find it useful when the first element added to mark ring is BOB.  When
> cycling across mark ring, BOB is a nice visual indication that I already
> looked across the whole ring, especially if I am searching for mark that
> is actually not in the mark ring (though I thought otherwise).

Yes, perhaps it'd be more consistent (and easier to reason about) if we
pushed the mark even if we newly opened the file.  (And since there
wouldn't be any marks in the buffer anyway, this shouldn't get in the
way of anybody's work flow, I think.)

So I've pushed your patch with that change.

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

Re: Patch proposal: display symbol source code in help buffers

[Ihor Radchenko]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> So I've pushed your patch with that change.

Thanks!

What about the other patch (Honor widen-automatically in
help-function-def--button-function)?

Best,
Ihor

Re: Patch proposal: display symbol source code in help buffers

[Lars Ingebrigtsen]

Ihor Radchenko <yantar92@gmail.com> writes:

> What about the other patch (Honor widen-automatically in
> help-function-def--button-function)?

Could you re-post that patch?  :-)

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

Re: Patch proposal: display symbol source code in help buffers

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Stefan Kangas <stefankangas@gmail.com>,
>   arthur.miller@live.com,  emacs-devel@gnu.org
> Date: Fri, 01 Oct 2021 15:05:57 +0800
> 
> +            ;; Save mark for the old location, unless we just opened
> +            ;; the buffer or the point is not actually going to move.
> +            (unless (or (= 1 (point))
> +                        (= (point) position))
> +              (push-mark nil t))

This assumes that if point is at BOB, we "just opened the buffer"?
What if the user moves point, then returns to BOB?

And why do you use 1 instead of point-min?

Re: Patch proposal: display symbol source code in help buffers

[Ihor Radchenko]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Ihor Radchenko <yantar92@gmail.com> writes:
>
>> What about the other patch (Honor widen-automatically in
>> help-function-def--button-function)?
>
> Could you re-post that patch?  :-)

Attached


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Honor-widen-automatically-in-help-function-def-butto.patch --]
[-- Type: text/x-diff, Size: 1420 bytes --]

From 635b8cab71d9f296bdef68dafd1d124213b777c2 Mon Sep 17 00:00:00 2001
Message-Id: <635b8cab71d9f296bdef68dafd1d124213b777c2.1633078979.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Fri, 1 Oct 2021 17:02:06 +0800
Subject: [PATCH] Honor widen-automatically in
 help-function-def--button-function

* lisp/help-mode.el (help-function-def--button-function): Ask user to
widen the buffer when `widen-automatically' is set to nil.
---
 lisp/help-mode.el | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index d61b1bdc62..2132a143c7 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -268,11 +268,14 @@ help-function-def--button-function
       (pop-to-buffer (car location))
       (run-hooks 'find-function-after-hook)
       (if position
-          (progn
+          (catch :exit
             ;; Widen the buffer if necessary to go to this position.
             (when (or (< position (point-min))
                       (> position (point-max)))
-              (widen))
+              (if (or widen-automatically
+                      (yes-or-no-p "The location is outside narrowing. Widen? "))
+                  (widen)
+                (throw :exit nil)))
             ;; Save mark for the old location, unless the point is not
             ;; actually going to move.
             (unless (= (point) position)
-- 
2.32.0

Re: Patch proposal: display symbol source code in help buffers

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Ihor Radchenko <yantar92@gmail.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  Stefan Kangas <stefankangas@gmail.com>,
>>   arthur.miller@live.com,  emacs-devel@gnu.org
>> Date: Fri, 01 Oct 2021 15:05:57 +0800
>> 
>> +            ;; Save mark for the old location, unless we just opened
>> +            ;; the buffer or the point is not actually going to move.
>> +            (unless (or (= 1 (point))
>> +                        (= (point) position))
>> +              (push-mark nil t))
>
> This assumes that if point is at BOB, we "just opened the buffer"?
> What if the user moves point, then returns to BOB?

Lars applied the patch without (= 1...) clause, so it does not matter
anymore.

> And why do you use 1 instead of point-min?

Mostly because I prefer to state it explicitly to not confuse BOB with
beginning of a narrow.  Is there any scenario when BOB is not at 1?

Best,
Ihor

Re: Patch proposal: display symbol source code in help buffers

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@gmail.com>
> Cc: larsi@gnus.org,  stefankangas@gmail.com,  arthur.miller@live.com,
>   emacs-devel@gnu.org
> Date: Fri, 01 Oct 2021 17:08:14 +0800
> 
> > This assumes that if point is at BOB, we "just opened the buffer"?
> > What if the user moves point, then returns to BOB?
> 
> Lars applied the patch without (= 1...) clause, so it does not matter
> anymore.

And I sent my comment before I saw that Lars already installed, so it
doesn't matter anymore.

> > And why do you use 1 instead of point-min?
> 
> Mostly because I prefer to state it explicitly to not confuse BOB with
> beginning of a narrow.  Is there any scenario when BOB is not at 1?

When the buffer is narrowed, of course.  Suppose some Help function
decides for some reason to narrow the buffer: then 1 won't work
anymore the way you intended.

Re: Patch proposal: display symbol source code in help buffers

[Lars Ingebrigtsen]

Ihor Radchenko <yantar92@gmail.com> writes:

> * lisp/help-mode.el (help-function-def--button-function): Ask user to
> widen the buffer when `widen-automatically' is set to nil.

I think that makes sense -- that user option was introduced to handle
stuff like this.  Does anybody object to making
help-function-def--button-function heed it instead of always widening?

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

Re: Patch proposal: display symbol source code in help buffers

[Ihor Radchenko]

Eli Zaretskii <eliz@gnu.org> writes:

>> > And why do you use 1 instead of point-min?
>> 
>> Mostly because I prefer to state it explicitly to not confuse BOB with
>> beginning of a narrow.  Is there any scenario when BOB is not at 1?
>
> When the buffer is narrowed, of course.  Suppose some Help function
> decides for some reason to narrow the buffer: then 1 won't work
> anymore the way you intended.

I understand. But then (point-min) will not be the actual beginning of
(bare) buffer.  That's why I used 1 explicitly to not confuse beginning
of norrowed part of the buffer with the actual beginning of the buffer.

Best,
Ihor