bug#44885: 28.0.50; [PATCH] ElDoc buffer mode and separator

bug#44885: 28.0.50; [PATCH] ElDoc buffer mode and separator

[Andrii Kolomoiets]

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

Hi,

Patch attached.

The configurable separator will make it possible to show multiple docs
in user pleasant way.

The mode will allow tweak ElDoc doc buffer appearance by adding hooks.

With little customization and custom display function in
display-buffer-alist 'M-x eldoc-doc-buffer' can show the ElDoc buffer
like some kind of tooltip.  See attached screenshot.

Thanks!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ElDoc-buffer-mode-and-separator.patch --]
[-- Type: text/x-patch, Size: 3127 bytes --]

From d13320ce2126b944cb03272b5546e08251b266db Mon Sep 17 00:00:00 2001
From: Andrii Kolomoiets <andreyk.mad@gmail.com>
Date: Thu, 26 Nov 2020 14:21:54 +0200
Subject: [PATCH] ElDoc buffer mode and separator

* lisp/emacs-lisp/eldoc.el (eldoc-doc-buffer-separator): New option.
(eldoc-doc-mode-map): New keymap.
(eldoc-doc-mode): New mode.
(eldoc--format-doc-buffer): Use them.
---
 lisp/emacs-lisp/eldoc.el | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index 78cb8f08c3..e2c8d001d6 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -129,6 +129,11 @@ eldoc-echo-area-prefer-doc-buffer
 is only skipped if the documentation doesn't fit there."
   :type 'boolean)

+(defcustom eldoc-doc-buffer-separator "\n"
+  "String used by Eldoc to separate docs in the ElDoc documentation buffer."
+  :type 'string
+  :version "28.1")
+
 (defface eldoc-highlight-function-argument
   '((t (:inherit bold)))
   "Face used for the argument at point in a function's argument list.
@@ -465,6 +470,24 @@ eldoc-doc-buffer
                                              (buffer-name)))
     (display-buffer (current-buffer))))

+(defvar eldoc-doc-mode-map
+  (let ((map (make-sparse-keymap)))
+    (suppress-keymap map)
+    (define-key map "q" 'quit-window)
+    (define-key map " " 'scroll-up-command)
+    (define-key map [?\S-\ ] 'scroll-down-command)
+    (define-key map "\C-?" 'scroll-down-command)
+    (define-key map "?" 'describe-mode)
+    (define-key map "h" 'describe-mode)
+    (define-key map ">" 'end-of-buffer)
+    (define-key map "<" 'beginning-of-buffer)
+    map)
+  "Keymap used in ElDoc documentation buffer.")
+
+(define-derived-mode eldoc-doc-mode fundamental-mode "ElDoc doc"
+  "Major mode for ElDoc documentation buffer."
+  (setq buffer-read-only t))
+
 (defun eldoc--format-doc-buffer (docs)
   "Ensure DOCS are displayed in an *eldoc* buffer."
   (interactive (list t))
@@ -472,12 +495,13 @@ eldoc--format-doc-buffer
                            eldoc--doc-buffer
                          (setq eldoc--doc-buffer
                                (get-buffer-create " *eldoc*")))
+    (unless (eq major-mode 'eldoc-doc-mode)
+      (eldoc-doc-mode))
     (unless (eq docs eldoc--doc-buffer-docs)
       (setq-local eldoc--doc-buffer-docs docs)
       (let ((inhibit-read-only t)
             (things-reported-on))
-        (erase-buffer) (setq buffer-read-only t)
-        (local-set-key "q" 'quit-window)
+        (erase-buffer)
         (cl-loop for (docs . rest) on docs
                  for (this-doc . plist) = docs
                  for thing = (plist-get plist :thing)
@@ -490,7 +514,7 @@ eldoc--format-doc-buffer
                         ": "
                         this-doc))
                  do (insert this-doc)
-                 when rest do (insert "\n")
+                 when rest do (insert eldoc-doc-buffer-separator)
                  finally (goto-char (point-min)))
         ;; Rename the buffer, taking into account whether it was
         ;; hidden or not
--
2.15.1

[-- Attachment #3: eldoc.png --]
[-- Type: image/png, Size: 64179 bytes --]

bug#44885: 28.0.50; [PATCH] ElDoc buffer mode and separator

[João Távora]

Andrii Kolomoiets <andreyk.mad@gmail.com> writes:

> Hi,
>
> Patch attached.
>
> The configurable separator will make it possible to show multiple docs
> in user pleasant way.

Thanks. The patch looks generally good but it has an important flaw that
can be fixed. See comments below.

> The mode will allow tweak ElDoc doc buffer appearance by adding hooks.

Yes.  I also have some ideas about how to sophisticate this buffer.

One of them involves having some backends, like the Elisp backends,
transmit a special key on the side -- much like :thing or :face -- that
specifies a function that fills in the whole buffer.  Thus, M-x eldoc in
a symbol of the Elisp buffer could eventually format this Eldoc doc
buffer much in the same way that currently M-x describe-symbol does.

And thus the Elisp disconnect between M-x eldoc and M-x describe-* would
be bridged.

The only incompatibility I see between my idea and your current proposal
is the major mode that you picked for the Eldoc doc buffer.  I wonder if
you could make it inherit from Help mode?  Then it would glue better
with my idea.

> With little customization and custom display function in
> display-buffer-alist 'M-x eldoc-doc-buffer' can show the ElDoc buffer
> like some kind of tooltip.

Very interesting. Can you share the display-buffer-alist hack that
allowed you to do this?  I didn't think about the possibility of
tweaking it like this, but it's certainly "legal".  I wonder if you
cann't do the same by adding a different function to
eldoc-display-functions, which was how I intented it to work.

> +(defvar eldoc-doc-mode-map
> +  (let ((map (make-sparse-keymap)))
> +    (suppress-keymap map)
> +    (define-key map "q" 'quit-window)
> +    (define-key map " " 'scroll-up-command)
> +    (define-key map [?\S-\ ] 'scroll-down-command)
> +    (define-key map "\C-?" 'scroll-down-command)
> +    (define-key map "?" 'describe-mode)
> +    (define-key map "h" 'describe-mode)
> +    (define-key map ">" 'end-of-buffer)
> +    (define-key map "<" 'beginning-of-buffer)
> +    map)
> +  "Keymap used in ElDoc documentation buffer.")
> +
> +(define-derived-mode eldoc-doc-mode fundamental-mode "ElDoc doc"
> +  "Major mode for ElDoc documentation buffer."
> +  (setq buffer-read-only t))

As I said above, I wonder if inheriting from help-mode wouldn't give us
most of this for free.  Maybe it would bring us _too much_ though...  So
some inheritance snipping would be needed.

>                   do (insert this-doc)
> -                 when rest do (insert "\n")
> +                 when rest do (insert eldoc-doc-buffer-separator)

I like this and I like the separator, however, notice that the current
implementation of the eldoc-display-in-echo-area also uses this buffer
as an implementation detail.  So this would break eldoc-display-in-echo
area.

The solution would be for eldoc-display-in-echo-area to use its own
"hidden" buffer and then eldoc-display-in-buffer would be free to format
the buffer as it sees fit.

So you could extract the buffer-formatting code to a common helper, use
it in eldoc-display-in-echo-area with a "\n" separator and in
eldoc-display-in-buffer with an arbitrary user-chosen separator.  The
performance hit of formatting two buffers would likely be negligible.

João

bug#44885: 28.0.50; [PATCH] ElDoc buffer mode and separator

[Basil L. Contovounesios]

Andrii Kolomoiets <andreyk.mad@gmail.com> writes:

> * lisp/emacs-lisp/eldoc.el (eldoc-doc-buffer-separator): New option.

Does this need calling out in etc/NEWS?

> +    (define-key map [?\S-\ ] 'scroll-down-command)

Nit: please prefer '\s' over '\ '.

> +(define-derived-mode eldoc-doc-mode fundamental-mode "ElDoc doc"
> +  "Major mode for ElDoc documentation buffer."

If it's the major mode for the ElDoc buffer, why not call it
eldoc-buffer-mode so as to avoid the repetition/ambiguity of "doc"?

> +  (setq buffer-read-only t))
> +
>  (defun eldoc--format-doc-buffer (docs)
>    "Ensure DOCS are displayed in an *eldoc* buffer."
>    (interactive (list t))
> @@ -472,12 +495,13 @@ eldoc--format-doc-buffer
>                             eldoc--doc-buffer
>                           (setq eldoc--doc-buffer
>                                 (get-buffer-create " *eldoc*")))
> +    (unless (eq major-mode 'eldoc-doc-mode)

Why not derived-mode-p?

Thanks,

-- 
Basil

bug#44885: 28.0.50; [PATCH] ElDoc buffer mode and separator

[Andrii Kolomoiets]

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

João Távora <joaotavora@gmail.com> writes:

> The only incompatibility I see between my idea and your current proposal
> is the major mode that you picked for the Eldoc doc buffer.  I wonder if
> you could make it inherit from Help mode?  Then it would glue better
> with my idea.

Sure.

>> +(define-derived-mode eldoc-doc-mode fundamental-mode "ElDoc doc"
>> +  "Major mode for ElDoc documentation buffer."
>> +  (setq buffer-read-only t))
>
> As I said above, I wonder if inheriting from help-mode wouldn't give us
> most of this for free.  Maybe it would bring us _too much_ though...

The only reason I didn't inherit from `special-mode` is to avoid binding
the `g` key to the `revert-buffer` function.

> So some inheritance snipping would be needed.

OK, will do.

>> With little customization and custom display function in
>> display-buffer-alist 'M-x eldoc-doc-buffer' can show the ElDoc buffer
>> like some kind of tooltip.
>
> Very interesting. Can you share the display-buffer-alist hack that
> allowed you to do this?  I didn't think about the possibility of
> tweaking it like this, but it's certainly "legal".

See attached eldoc-frame.el.
In emacs -Q:
1. M-x load-file eldoc-frame.el
2. Type "(setq" and wait a bit for eldoc message to appear in the echo
   area
3. C-c h

> I wonder if you cann't do the same by adding a different function to
> eldoc-display-functions, which was how I intented it to work.

This will work even better.  This way the "tooltip" can fit to content
as soon as new docs will be shown.

>>                   do (insert this-doc)
>> -                 when rest do (insert "\n")
>> +                 when rest do (insert eldoc-doc-buffer-separator)
>
> I like this and I like the separator, however, notice that the current
> implementation of the eldoc-display-in-echo-area also uses this buffer
> as an implementation detail.  So this would break eldoc-display-in-echo
> area.

I was hoping it would look like an enhancement -- eldoc-separator
everywhere! :)

> The solution would be for eldoc-display-in-echo-area to use its own
> "hidden" buffer and then eldoc-display-in-buffer would be free to format
> the buffer as it sees fit.
>
> So you could extract the buffer-formatting code to a common helper, use
> it in eldoc-display-in-echo-area with a "\n" separator and in
> eldoc-display-in-buffer with an arbitrary user-chosen separator.  The
> performance hit of formatting two buffers would likely be negligible.

OK, will do.


[-- Attachment #2: eldoc-frame.el --]
[-- Type: application/emacs-lisp, Size: 2381 bytes --]

bug#44885: 28.0.50; [PATCH] ElDoc buffer mode and separator

[Andrii Kolomoiets]

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

>> * lisp/emacs-lisp/eldoc.el (eldoc-doc-buffer-separator): New option.
>
> Does this need calling out in etc/NEWS?

Sure.  But I left it for later, when the patch will be ready.

>> +    (define-key map [?\S-\ ] 'scroll-down-command)
>
> Nit: please prefer '\s' over '\ '.

This is copy-paste from the `special-mode-map` and will be removed once
eldoc-buffer mode will be derived from special-mode.

>> +(define-derived-mode eldoc-doc-mode fundamental-mode "ElDoc doc"
>> +  "Major mode for ElDoc documentation buffer."
>
> If it's the major mode for the ElDoc buffer, why not call it
> eldoc-buffer-mode so as to avoid the repetition/ambiguity of "doc"?

Indeed.  I think I was confused by the `eldoc-doc-buffer` command name.

>> +  (setq buffer-read-only t))
>> +
>>  (defun eldoc--format-doc-buffer (docs)
>>    "Ensure DOCS are displayed in an *eldoc* buffer."
>>    (interactive (list t))
>> @@ -472,12 +495,13 @@ eldoc--format-doc-buffer
>>                             eldoc--doc-buffer
>>                           (setq eldoc--doc-buffer
>>                                 (get-buffer-create " *eldoc*")))
>> +    (unless (eq major-mode 'eldoc-doc-mode)
>
> Why not derived-mode-p?

You are right.  Will fix.

Thanks!

bug#44885: 28.0.50; [PATCH] ElDoc buffer mode and separator

[Felician Nemeth]

I'm late to the game, but I think this new mode should be derived from
view-mode instead of fundamental-mode.  It would make the code cleaner
too.

bug#44885: 28.0.50; [PATCH] ElDoc buffer mode and separator

[Lars Ingebrigtsen]

Andrii Kolomoiets <andreyk.mad@gmail.com> writes:

>>> +    (unless (eq major-mode 'eldoc-doc-mode)
>>
>> Why not derived-mode-p?
>
> You are right.  Will fix.

This was over half a year ago, but there was no follow-up to this (and
it seemed like everybody agreed that this was a good feature).  Andrii,
did you continue working on this patch?

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

bug#44885: 28.0.50; [PATCH] ElDoc buffer mode and separator

[João Távora]

This needs more design I think.  If Andrii doesn't, I'll try to look at
it again soon, as it's kind of on my list of things to go into master
before 28 is cut.

João

On Wed, Sep 8, 2021 at 10:44 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
> Andrii Kolomoiets <andreyk.mad@gmail.com> writes:
>
> >>> +    (unless (eq major-mode 'eldoc-doc-mode)
> >>
> >> Why not derived-mode-p?
> >
> > You are right.  Will fix.
>
> This was over half a year ago, but there was no follow-up to this (and
> it seemed like everybody agreed that this was a good feature).  Andrii,
> did you continue working on this patch?
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no



-- 
João Távora

bug#44885: 28.0.50; [PATCH] ElDoc buffer mode and separator

[Andrii Kolomoiets]

Lars, I didn't have time to continue working on this patch.

João, please feel free to make your own implementation or to modify this
patch.  Let me know if I can help you with anything.

Thanks! And sorry for delay with the patch.


João Távora <joaotavora@gmail.com> writes:

> This needs more design I think.  If Andrii doesn't, I'll try to look at
> it again soon, as it's kind of on my list of things to go into master
> before 28 is cut.
>
> João
>
> On Wed, Sep 8, 2021 at 10:44 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:
>>
>> Andrii Kolomoiets <andreyk.mad@gmail.com> writes:
>>
>> >>> +    (unless (eq major-mode 'eldoc-doc-mode)
>> >>
>> >> Why not derived-mode-p?
>> >
>> > You are right.  Will fix.
>>
>> This was over half a year ago, but there was no follow-up to this (and
>> it seemed like everybody agreed that this was a good feature).  Andrii,
>> did you continue working on this patch?
>>
>> --
>> (domestic pets only, the antidote for overdose, milk.)
>>    bloggy blog: http://lars.ingebrigtsen.no

-- 
Andrii

bug#44885: 28.0.50; [PATCH] ElDoc buffer mode and separator

[Lars Ingebrigtsen]

Andrii Kolomoiets <andreyk.mad@gmail.com> writes:

> +(defvar eldoc-doc-mode-map
> +  (let ((map (make-sparse-keymap)))
> +    (suppress-keymap map)

This was fixed in 97abe8511a by just using `special-mode' in this buffer
instead.