Re: master a397fa0: Improve docstrings auto-generated by `define-minor-mode'

Re: master a397fa0: Improve docstrings auto-generated by `define-minor-mode'

[Robert Pluim]

>>>>> On Thu,  3 Oct 2019 19:29:58 -0400 (EDT), lekktu@gmail.com (Juanma Barranquero) said:

    Juanma> branch: master
    Juanma> commit a397fa06d18d6ae37a3a1288f269e1ae9eb3b569
    Juanma> Author: Juanma Barranquero <lekktu@gmail.com>
    Juanma> Commit: Juanma Barranquero <lekktu@gmail.com>

    Juanma>     Improve docstrings auto-generated by `define-minor-mode'
    
This causes 'make bootstrap' to fail for me, although I have no clue
why:

./temacs --batch  -l loadup --temacs=pbootstrap
Loading loadup.el (source)...
dump mode: pbootstrap
Using load-path (/home/rpluim/repos/emacs-master/lisp /home/rpluim/repos/emacs-master/lisp/emacs-lisp /home/rpluim/repos/emacs-master/lisp/progmodes /home/rpluim/repos/emacs-master/lisp/language /home/rpluim/repos/emacs-master/lisp/international /home/rpluim/repos/emacs-master/lisp/textmodes /home/rpluim/repos/emacs-master/lisp/vc)
Loading emacs-lisp/byte-run (source)...
Loading emacs-lisp/backquote (source)...
Loading subr (source)...
Loading version (source)...
Loading widget (source)...
Loading custom (source)...
Loading emacs-lisp/map-ynp (source)...
Loading international/mule (source)...
Loading international/mule-conf (source)...
Loading env (source)...
Loading format (source)...
Loading bindings (source)...
Loading window (source)...
Loading /home/rpluim/repos/emacs-master/lisp/files.el (source)...
Symbol’s function definition is void: generate-new-buffer
make[2]: *** [Makefile:817: bootstrap-emacs.pdmp] Error 255
make[2]: Leaving directory '/home/rpluim/repos/emacs-master/src'
make[1]: *** [Makefile:424: src] Error 2
make[1]: Leaving directory '/home/rpluim/repos/emacs-master'
make: *** [Makefile:1119: bootstrap] Error 2

Re: master a397fa0: Improve docstrings auto-generated by `define-minor-mode'

[Juanma Barranquero]

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

On Fri, Oct 4, 2019 at 11:50 AM Robert Pluim <rpluim@gmail.com> wrote:

> This causes 'make bootstrap' to fail for me, although I have no clue
> why:

Yes, sorry. I see it too. I'll take a look asap.

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

Re: master a397fa0: Improve docstrings auto-generated by `define-minor-mode'

[Michael Albinus]

lekktu@gmail.com (Juanma Barranquero) writes:

> branch: master
> commit a397fa06d18d6ae37a3a1288f269e1ae9eb3b569
> Author: Juanma Barranquero <lekktu@gmail.com>
> Commit: Juanma Barranquero <lekktu@gmail.com>
>
>     Improve docstrings auto-generated by `define-minor-mode'
>
>     * lisp/emacs-lisp/easy-mmode.el (easy-mmode--mode-docstring):
>     When using `easy-mmode--arg-docstring' to auto-generate a
>     docstring, refill it up to `emacs-lisp-docstring-fill-column'.
> ---
>  lisp/emacs-lisp/easy-mmode.el | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
> index 5e7b29e..6d5b514 100644
> --- a/lisp/emacs-lisp/easy-mmode.el
> +++ b/lisp/emacs-lisp/easy-mmode.el
> @@ -95,11 +95,17 @@ if ARG is `toggle'; disable the mode otherwise.")
>  \\{%s}" mode-pretty-name keymap-sym))))
>      (if (string-match-p "\\bARG\\b" doc)
>          doc
> -      (let ((argdoc (format easy-mmode--arg-docstring
> -                            mode-pretty-name)))
> -        (replace-regexp-in-string "\\(\n\n\\|\\'\\)\\(.\\|\n\\)*\\'"
> -                                  (concat argdoc "\\1")
> -                                  doc nil nil 1)))))
> +      (let* ((fill-prefix nil)
> +             (docstring-fc (bound-and-true-p emacs-lisp-docstring-fill-column))
> +             (fill-column (if (integerp docstring-fc) docstring-fc 65))
> +             (argdoc (format easy-mmode--arg-docstring
> +                             mode-pretty-name)))
> +        (with-temp-buffer
> +          (insert (replace-regexp-in-string "\\(\n\n\\|\\'\\)\\(.\\|\n\\)*\\'"
> +                                            (concat argdoc "\\1")
> +                                            doc nil nil 1))
> +          (fill-region (point-min) (point-max) 'left t)
> +          (buffer-string))))))

This has broken the build on emba. See <https://emba.gnu.org/emacs/emacs/-/jobs/3602/raw>.

Best regards, Michael.

Re: master a397fa0: Improve docstrings auto-generated by `define-minor-mode'

[Juanma Barranquero]

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

I've riveted the change.

There's a circular dependency between files and easy-mmode, and the code
also uses fill-region which is loaded later.

I'd be grateful if someone with more bootstrap chops than me could
investigate it further. Even if the change cannot be used for the minor
modes defined when loading up, it's still useful for subsequent uses of
define-minor-mode.

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

Re: master a397fa0: Improve docstrings auto-generated by `define-minor-mode'

[Stefan Monnier]

Hi Juanma,

> I've riveted the change.

I just fixed the `generate-new-buffer` dependency.

> There's a circular dependency between files and easy-mmode, and the code
> also uses fill-region which is loaded later.

This can be avoided with a simple (when (fboundp 'fill-region) ...), at
the cost of not filling those docstrings generated "too early",
e.g. with the patch below.

But I have a question: your patch re-fills the whole docstring (after
adding the arg-docstring chunk to it).  Was that really the intention?
I thought this was considered too risky and we wanted to only refill the
arg-docstring paragraph (i.e. before inserting it into the rest of the
docstring).


        Stefan


diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index 6d5b5141aa..1125000e5c 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -84,8 +84,8 @@ easy-mmode-pretty-mode-name
 (defconst easy-mmode--arg-docstring
   "
 
-If called interactively, enable %s if ARG is positive, and
-disable it if ARG is zero or negative.  If called from Lisp,
+If called interactively, enable %s if ARG is positive,
+and disable it if ARG is zero or negative.  If called from Lisp,
 also enable the mode if ARG is omitted or nil, and toggle it
 if ARG is `toggle'; disable the mode otherwise.")
 
@@ -104,7 +104,8 @@ easy-mmode--mode-docstring
           (insert (replace-regexp-in-string "\\(\n\n\\|\\'\\)\\(.\\|\n\\)*\\'"
                                             (concat argdoc "\\1")
                                             doc nil nil 1))
-          (fill-region (point-min) (point-max) 'left t)
+          (when (fboundp 'fill-region)
+            (fill-region (point-min) (point-max) 'left t))
           (buffer-string))))))
 
 ;;;###autoload

Re: master a397fa0: Improve docstrings auto-generated by `define-minor-mode'

[Juanma Barranquero]

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

On Fri, Oct 4, 2019 at 3:01 PM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > I've riveted the change.

Or even "reverted"... Lesson for today: do not write messages while
skipping bedtime.

> I just fixed the `generate-new-buffer` dependency.

Thanks.

> But I have a question: your patch re-fills the whole docstring (after
> adding the arg-docstring chunk to it).  Was that really the intention?
> I thought this was considered too risky and we wanted to only refill the
> arg-docstring paragraph (i.e. before inserting it into the rest of the
> docstring).

Yes, my mistake. (Another one, I mean).

This was intended:

diff --git i/lisp/emacs-lisp/easy-mmode.el w/lisp/emacs-lisp/easy-mmode.el
index 5e7b29eddf..3224d5b985 100644
--- i/lisp/emacs-lisp/easy-mmode.el
+++ w/lisp/emacs-lisp/easy-mmode.el
@@ -96,8 +96,16 @@ easy-mmode--mode-docstring
     (if (string-match-p "\\bARG\\b" doc)
         doc
-      (let ((argdoc (format easy-mmode--arg-docstring
-                            mode-pretty-name)))
+      (let* ((fill-prefix nil)
+             (docs-fc (bound-and-true-p emacs-lisp-docstring-fill-column))
+             (fill-column (if (integerp docs-fc) docs-fc 65))
+             (argdoc (format easy-mmode--arg-docstring mode-pretty-name))
+             (filled (if (fboundp 'fill-region)
+                         (with-temp-buffer
+                           (insert argdoc)
+                           (fill-region (point-min) (point-max) 'left t)
+                           (buffer-string))
+                       argdoc)))
         (replace-regexp-in-string "\\(\n\n\\|\\'\\)\\(.\\|\n\\)*\\'"
-                                  (concat argdoc "\\1")
+                                  (concat filled "\\1")
                                   doc nil nil 1)))))

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