all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#24622: 26.0.50; lisp-fill-paragraph broken
@ 2016-10-05 17:22 martin rudalics
  2017-10-13  2:23 ` Alex
  0 siblings, 1 reply; 8+ messages in thread
From: martin rudalics @ 2016-10-05 17:22 UTC (permalink / raw)
  To: 24622

This commit

commit 866e3c050fe64fee81f29a335a50a11b2562422e
Author: Lars Ingebrigtsen <larsi@gnus.org>
Date:   Thu Apr 28 12:05:15 2016 +0200

     Don't consider colons to be paragraphs starting chars in strings

     * lisp/emacs-lisp/lisp-mode.el (lisp-fill-paragraph): Don't
     consider colons to start paragraphs in (doc) strings
     (bug#7751).

breaks filling doc-strings of defcustoms.  As an example consider the
defcustom of ‘window-min-height’ in window.el:


(defcustom window-min-height 4
   "The minimum total height, in lines, of any window.
The value has to accommodate one text line, a mode and header
line, a horizontal scroll bar and a bottom divider, if present.
A value less than `window-safe-min-height' is ignored.  The value
of this variable is honored when windows are resized or split.

Applications should never rebind this variable.  To resize a
window to a height less than the one specified here, an
application should instead call `window-resize' with a non-nil
IGNORE argument.  In order to have `split-window' make a window
shorter, explicitly specify the SIZE argument of that function."
   :type 'integer
   :version "24.1"
   :group 'windows)


Put point at the beginning of the last line of the doc-string and do

M-: (fill-paragraph)

This gets me here


(defcustom window-min-height 4
   "The minimum total height, in lines, of any window.
The value has to accommodate one text line, a mode and header
line, a horizontal scroll bar and a bottom divider, if present.
A value less than `window-safe-min-height' is ignored.  The value
of this variable is honored when windows are resized or split.

Applications should never rebind this variable.  To resize a
window to a height less than the one specified here, an
application should instead call `window-resize' with a non-nil
IGNORE argument.  In order to have `split-window' make a window
shorter, explicitly specify the SIZE argument of that function."
:type 'integer :version "24.1" :group 'windows)


martin






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#24622: 26.0.50; lisp-fill-paragraph broken
  2016-10-05 17:22 bug#24622: 26.0.50; lisp-fill-paragraph broken martin rudalics
@ 2017-10-13  2:23 ` Alex
  2017-10-14  8:35   ` martin rudalics
  2017-10-21  8:11   ` Eli Zaretskii
  0 siblings, 2 replies; 8+ messages in thread
From: Alex @ 2017-10-13  2:23 UTC (permalink / raw)
  To: martin rudalics; +Cc: Lars Ingebrigtsen, 24622

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

martin rudalics <rudalics@gmx.at> writes:

> This commit
>
> commit 866e3c050fe64fee81f29a335a50a11b2562422e
> Author: Lars Ingebrigtsen <larsi@gnus.org>
> Date:   Thu Apr 28 12:05:15 2016 +0200
>
>     Don't consider colons to be paragraphs starting chars in strings
>
>     * lisp/emacs-lisp/lisp-mode.el (lisp-fill-paragraph): Don't
>     consider colons to start paragraphs in (doc) strings
>     (bug#7751).
>
> breaks filling doc-strings of defcustoms.  As an example consider the
> defcustom of ‘window-min-height’ in window.el:

I've included a diff below that appears to solve both bug#7751 and this
one, though I can't guarantee that it doesn't blow something else up.
Does anyone have any complaints?

In any case, I think this bug should be solved before 26.1 is released.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: paragraph --]
[-- Type: text/x-diff, Size: 1620 bytes --]

diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index fd12635d85..93435e1b4b 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -1267,7 +1267,8 @@ lisp-fill-paragraph
       ;; case).  The `;' and `:' stop the paragraph being filled at following
       ;; comment lines and at keywords (e.g., in `defcustom').  Left parens are
       ;; escaped to keep font-locking, filling, & paren matching in the source
-      ;; file happy.
+      ;; file happy.  The `:' must be preceded by whitespace so that keywords
+      ;; inside of the docstring don't start new paragraphs (Bug#7751).
       ;;
       ;; `paragraph-separate': A clever regexp distinguishes the first line of
       ;; a docstring and identifies it as a paragraph separator, so that it
@@ -1280,13 +1281,7 @@ lisp-fill-paragraph
       ;; `emacs-lisp-docstring-fill-column' if that value is an integer.
       (let ((paragraph-start
              (concat paragraph-start
-                     (format "\\|\\s-*\\([(;%s\"]\\|`(\\|#'(\\)"
-                             ;; If we're inside a string (like the doc
-                             ;; string), don't consider a colon to be
-                             ;; a paragraph-start character.
-                             (if (nth 3 (syntax-ppss))
-                                 ""
-                               ":"))))
+                     "\\|\\s-*\\([(;\"]\\|\\s-:\\|`(\\|#'(\\)"))
 	    (paragraph-separate
 	     (concat paragraph-separate "\\|\\s-*\".*[,\\.]$"))
             (fill-column (if (and (integerp emacs-lisp-docstring-fill-column)

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#24622: 26.0.50; lisp-fill-paragraph broken
  2017-10-13  2:23 ` Alex
@ 2017-10-14  8:35   ` martin rudalics
  2017-10-21  8:11   ` Eli Zaretskii
  1 sibling, 0 replies; 8+ messages in thread
From: martin rudalics @ 2017-10-14  8:35 UTC (permalink / raw)
  To: Alex; +Cc: Lars Ingebrigtsen, 24622

 >>      Don't consider colons to be paragraphs starting chars in strings
 >>
 >>      * lisp/emacs-lisp/lisp-mode.el (lisp-fill-paragraph): Don't
 >>      consider colons to start paragraphs in (doc) strings
 >>      (bug#7751).
 >>
 >> breaks filling doc-strings of defcustoms.  As an example consider the
 >> defcustom of ‘window-min-height’ in window.el:
 >
 > I've included a diff below that appears to solve both bug#7751 and this
 > one, though I can't guarantee that it doesn't blow something else up.
 > Does anyone have any complaints?
 >
 > In any case, I think this bug should be solved before 26.1 is released.

I obviously think so too.  Lars, how do we proceed here?  I'd vote
for checking in Alex's patch.

martin






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#24622: 26.0.50; lisp-fill-paragraph broken
  2017-10-13  2:23 ` Alex
  2017-10-14  8:35   ` martin rudalics
@ 2017-10-21  8:11   ` Eli Zaretskii
  2017-10-22  8:10     ` Alex
  1 sibling, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2017-10-21  8:11 UTC (permalink / raw)
  To: Alex; +Cc: larsi, 24622

> From: Alex <agrambot@gmail.com>
> Date: Thu, 12 Oct 2017 20:23:01 -0600
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, 24622@debbugs.gnu.org
> 
> > This commit
> >
> > commit 866e3c050fe64fee81f29a335a50a11b2562422e
> > Author: Lars Ingebrigtsen <larsi@gnus.org>
> > Date:   Thu Apr 28 12:05:15 2016 +0200
> >
> >     Don't consider colons to be paragraphs starting chars in strings
> >
> >     * lisp/emacs-lisp/lisp-mode.el (lisp-fill-paragraph): Don't
> >     consider colons to start paragraphs in (doc) strings
> >     (bug#7751).
> >
> > breaks filling doc-strings of defcustoms.  As an example consider the
> > defcustom of ‘window-min-height’ in window.el:
> 
> I've included a diff below that appears to solve both bug#7751 and this
> one, though I can't guarantee that it doesn't blow something else up.
> Does anyone have any complaints?
> 
> In any case, I think this bug should be solved before 26.1 is released.

Since there were no more comments, please push to the release branch.

Thanks.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#24622: 26.0.50; lisp-fill-paragraph broken
  2017-10-21  8:11   ` Eli Zaretskii
@ 2017-10-22  8:10     ` Alex
  2017-10-22 14:15       ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Alex @ 2017-10-22  8:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, 24622

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

Eli Zaretskii <eliz@gnu.org> writes:

> Since there were no more comments, please push to the release branch.
>
> Thanks.

I added some tests to the patch. Is the following acceptable?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fill --]
[-- Type: text/x-diff, Size: 3761 bytes --]

From f3bdd14347be5ba2e8e8ca91a9b056b768398f73 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Sun, 22 Oct 2017 01:46:05 -0600
Subject: [PATCH] Don't fill keywords after Emacs Lisp docstring

This approach does mean that keywords that have spaces before them
inside of docstrings aren't filled, but I think this is should be fine
until Bug#28937 is fixed.

* lisp/emacs-lisp/lisp-mode.el (lisp-fill-paragraph): Add a colon to
paragraph-start unconditionally, but require that it proceeds at least
one space.  (Bug#24622)
* test/lisp/emacs-lisp/lisp-tests.el: New tests for Bug#24622 and
Bug#7751.
---
 lisp/emacs-lisp/lisp-mode.el       | 11 +++--------
 test/lisp/emacs-lisp/lisp-tests.el | 31 +++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index fd12635d85..93435e1b4b 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -1267,7 +1267,8 @@ lisp-fill-paragraph
       ;; case).  The `;' and `:' stop the paragraph being filled at following
       ;; comment lines and at keywords (e.g., in `defcustom').  Left parens are
       ;; escaped to keep font-locking, filling, & paren matching in the source
-      ;; file happy.
+      ;; file happy.  The `:' must be preceded by whitespace so that keywords
+      ;; inside of the docstring don't start new paragraphs (Bug#7751).
       ;;
       ;; `paragraph-separate': A clever regexp distinguishes the first line of
       ;; a docstring and identifies it as a paragraph separator, so that it
@@ -1280,13 +1281,7 @@ lisp-fill-paragraph
       ;; `emacs-lisp-docstring-fill-column' if that value is an integer.
       (let ((paragraph-start
              (concat paragraph-start
-                     (format "\\|\\s-*\\([(;%s\"]\\|`(\\|#'(\\)"
-                             ;; If we're inside a string (like the doc
-                             ;; string), don't consider a colon to be
-                             ;; a paragraph-start character.
-                             (if (nth 3 (syntax-ppss))
-                                 ""
-                               ":"))))
+                     "\\|\\s-*\\([(;\"]\\|\\s-:\\|`(\\|#'(\\)"))
 	    (paragraph-separate
 	     (concat paragraph-separate "\\|\\s-*\".*[,\\.]$"))
             (fill-column (if (and (integerp emacs-lisp-docstring-fill-column)
diff --git a/test/lisp/emacs-lisp/lisp-tests.el b/test/lisp/emacs-lisp/lisp-tests.el
index ae1302bdce..6fb2f8adc9 100644
--- a/test/lisp/emacs-lisp/lisp-tests.el
+++ b/test/lisp/emacs-lisp/lisp-tests.el
@@ -589,5 +589,36 @@ elisp-tests-with-temp-buffer
     (should (= (point) before))
     (should (= (mark) after))))
 
+(ert-deftest lisp-fill-paragraph-colon ()
+  "Keywords below Emacs Lisp docstrings should not be filled (Bug#24622).
+Keywords inside docstrings should be filled (Bug#7751)."
+  (elisp-tests-with-temp-buffer
+      "
+\(defcustom custom value
+  \"First\n
+Second\n
+=!inside=Third line\"
+  =!keywords=:type 'sexp
+  :version \"26.1\"
+  :group 'lisp-tests)"
+    (goto-char inside)
+    (fill-paragraph)
+    (goto-char keywords)
+    (beginning-of-line)
+    (should (looking-at "  :type 'sexp\n  :version \"26.1\"\n  :")))
+  (elisp-tests-with-temp-buffer
+      "
+\(defun foo ()
+  \"Summary.
+=!inside=Testing keywords: :one :two :three\"
+  (body))" ; FIXME: Remove parens around body to test bug#28937 once it's fixed
+    (goto-char inside)
+    (let ((emacs-lisp-docstring-fill-column 30))
+      (fill-paragraph))
+    (forward-line)
+    (should (looking-at ":three"))
+    (end-of-line)
+    (should-not (eq (preceding-char) ?\)))))
+
 (provide 'lisp-tests)
 ;;; lisp-tests.el ends here
-- 
2.14.2


[-- Attachment #3: Type: text/plain, Size: 153 bytes --]


I added in a reference to bug#28937, which I just reported. Fixing that
would fix also fix this bug, but that solution might not be ready for
Emacs 26.

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#24622: 26.0.50; lisp-fill-paragraph broken
  2017-10-22  8:10     ` Alex
@ 2017-10-22 14:15       ` Eli Zaretskii
  2017-10-22 19:11         ` Alex
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2017-10-22 14:15 UTC (permalink / raw)
  To: Alex; +Cc: larsi, 24622

> From: Alex <agrambot@gmail.com>
> Cc: larsi@gnus.org,  24622@debbugs.gnu.org
> Date: Sun, 22 Oct 2017 02:10:47 -0600
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Since there were no more comments, please push to the release branch.
> >
> > Thanks.
> 
> I added some tests to the patch. Is the following acceptable?

Yes, thanks.

> I added in a reference to bug#28937, which I just reported. Fixing that
> would fix also fix this bug, but that solution might not be ready for
> Emacs 26.

If you intend to solve this differently on master, and you think you
will do that soon, you can indicate that this patch should not be
merged to master.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#24622: 26.0.50; lisp-fill-paragraph broken
  2017-10-22 14:15       ` Eli Zaretskii
@ 2017-10-22 19:11         ` Alex
  2017-10-23  8:05           ` martin rudalics
  0 siblings, 1 reply; 8+ messages in thread
From: Alex @ 2017-10-22 19:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, 24622-done

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: larsi@gnus.org,  24622@debbugs.gnu.org
>> Date: Sun, 22 Oct 2017 02:10:47 -0600
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Since there were no more comments, please push to the release branch.
>> >
>> > Thanks.
>> 
>> I added some tests to the patch. Is the following acceptable?
>
> Yes, thanks.

Okay, pushed to emacs-26 as a012ec766c9d9bac0a56e814589a4b3b93311c28.

>> I added in a reference to bug#28937, which I just reported. Fixing that
>> would fix also fix this bug, but that solution might not be ready for
>> Emacs 26.
>
> If you intend to solve this differently on master, and you think you
> will do that soon, you can indicate that this patch should not be
> merged to master.

Thanks for the tip, but I likely won't get to it soon. I think this
should be merged to master in the meantime.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#24622: 26.0.50; lisp-fill-paragraph broken
  2017-10-22 19:11         ` Alex
@ 2017-10-23  8:05           ` martin rudalics
  0 siblings, 0 replies; 8+ messages in thread
From: martin rudalics @ 2017-10-23  8:05 UTC (permalink / raw)
  To: 24622, agrambot

 > Okay, pushed to emacs-26 as a012ec766c9d9bac0a56e814589a4b3b93311c28.

Thanks Alex for fixing this.

martin





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-10-23  8:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-05 17:22 bug#24622: 26.0.50; lisp-fill-paragraph broken martin rudalics
2017-10-13  2:23 ` Alex
2017-10-14  8:35   ` martin rudalics
2017-10-21  8:11   ` Eli Zaretskii
2017-10-22  8:10     ` Alex
2017-10-22 14:15       ` Eli Zaretskii
2017-10-22 19:11         ` Alex
2017-10-23  8:05           ` martin rudalics

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.