From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Maxim Cournoyer Newsgroups: gmane.emacs.bugs Subject: bug#56197: lisp-fill-paragraph behavior changed in Emacs 28 Date: Sat, 04 Jan 2025 19:09:42 +0900 Message-ID: <87v7uuevq1.fsf_-_@gmail.com> References: <87zgi2xcgm.fsf@gmail.com> <87y1xlj6wn.fsf@gnus.org> <834k08c3se.fsf@gnu.org> <874k08kj31.fsf@gnus.org> <87y1xkj4co.fsf@gnus.org> <87tu88j3tf.fsf@gnus.org> <87bjwzr027.fsf@lease-up.com> <86v7v7ynf0.fsf@gnu.org> <87seq8tm2f.fsf@gmail.com> <86o70wurf9.fsf@gnu.org> <87ttanrg9h.fsf@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="24415"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: larsi@gnus.org, 56197@debbugs.gnu.org, felix.lechner@lease-up.com, stefankangas@gmail.com To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Jan 04 11:10:33 2025 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1tU172-0006Av-IN for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 04 Jan 2025 11:10:32 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tU16a-0007E7-Ns; Sat, 04 Jan 2025 05:10:04 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tU16Z-0007DO-6p for bug-gnu-emacs@gnu.org; Sat, 04 Jan 2025 05:10:03 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1tU16Y-0006lo-Tu for bug-gnu-emacs@gnu.org; Sat, 04 Jan 2025 05:10:02 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:Date:References:In-Reply-To:From:To:Subject; bh=ESEdtEbGkFdsE2+8wue0FTuaanNZj+yfo8zdeUo1lzI=; b=GZNKtOcrF5lCKsNghRKdO9un++bMFxna1ueRTdwawFjdH6Fgh3E+54f2c3Ge4FEvAJMl7J3PAKFWO9ycxTdXaqdiT7w+xsPjZyTKi9NHoOX8cF/cWdWs+2vbYrxg25xdyeBEg5imZ2D6qIDUg9BqbWKKbP3dCQe1cQ3kNdlM0jqs4mre7sO0k/2aZbiSiL9kaQamrQZ1keFzoGgdCoRLDJokdiYWY3FmQwBchnOmunlf2LwgGX/t4b6L8bYa2gEfdECOCupl28X02O4JjhrWXkEG8EIk13ND/PWZo5C9n5YuiKAW7uJepdqsmmbcyhHoD/4GEphHlSGmeC55C79FFg==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1tU16Y-0007Dr-OD for bug-gnu-emacs@gnu.org; Sat, 04 Jan 2025 05:10:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Maxim Cournoyer Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 04 Jan 2025 10:10:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 56197 X-GNU-PR-Package: emacs Original-Received: via spool by 56197-submit@debbugs.gnu.org id=B56197.173598539827732 (code B ref 56197); Sat, 04 Jan 2025 10:10:02 +0000 Original-Received: (at 56197) by debbugs.gnu.org; 4 Jan 2025 10:09:58 +0000 Original-Received: from localhost ([127.0.0.1]:53449 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1tU16T-0007DC-9z for submit@debbugs.gnu.org; Sat, 04 Jan 2025 05:09:58 -0500 Original-Received: from mail-pl1-x62f.google.com ([2607:f8b0:4864:20::62f]:44330) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1tU16R-0007D0-AN for 56197@debbugs.gnu.org; Sat, 04 Jan 2025 05:09:56 -0500 Original-Received: by mail-pl1-x62f.google.com with SMTP id d9443c01a7336-2164b662090so160068035ad.1 for <56197@debbugs.gnu.org>; Sat, 04 Jan 2025 02:09:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1735985394; x=1736590194; darn=debbugs.gnu.org; h=mime-version:message-id:date:user-agent:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=ESEdtEbGkFdsE2+8wue0FTuaanNZj+yfo8zdeUo1lzI=; b=FaU4ruWmuHLMjtY+TkAtZN8e/C0xJLFRqktEade73Q02jLYyAUI+ymlVPVpre/iiTH AB8jcKnGS3lDcliTQlmmP3sdQkY6SUPxvXmkrrxJLIL502ejdvIla5e56A6rcnfOepIz IRCEPq8c3MA/F4droy8b5K3T12LvxF6xZukxKYAk1mFNb7HBfhIXU2gS/tXp3ygvG5+W y1pibn1emfwNVvlfyBQurHJuo8Eit/ZxJvu2UdA3aDWqATy8Raa5xxC1TabpgrmUNxvS FCFmtxnAUuKFeyVLpMJpLe8NoW87YWVRISPUQPmio44ljkBeB47r8Vc7UKGG26MGfP04 9Efw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1735985394; x=1736590194; h=mime-version:message-id:date:user-agent:references:in-reply-to :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ESEdtEbGkFdsE2+8wue0FTuaanNZj+yfo8zdeUo1lzI=; b=SZhHPxDPPAD9CigCN6gAEEVyeOSfZgDY9uAu4Inlf6AITt8Tei7812vHvfp+WF8q/w kMuS0zWbuUBgAn9zOZ6yxtDKU8e0LItDVq80XyqFbLFdiBYlQe8MIEL8abfoFsm2VVn3 sDYtj5IS7x+0RV5vbeD1usf7mwWJt6PsfAvvayVhZ2+ffeUNlTutsWAnczIrSyWInGSQ WNpGTtgpNPlptyPkIywdCKeRr+jLsrfyIjVEizTnt3Yxcu8JJDundJ5fpO541MqVu5VI ZKrmlMKSxpbuVGDNFzRsAwsjJk30LTquERHKe8TYPM9PIaR26Ln1rSBB4oc449HAsgvR neiQ== X-Forwarded-Encrypted: i=1; AJvYcCUgwXq5chy4Rh3eSSZTce9GT8fTDG3HLe3S+K9savldljfO7gyZILo21DLaG/1bN8zDimiaaA==@debbugs.gnu.org X-Gm-Message-State: AOJu0Yxs3JEvBswTWruixppd8bM0NoD9rz5L1AN3HhemLj68T2Sa1ThA xBiGc4hX+6s0VYlZnBCe4Jv4pHq3MWyJSEv/alH9K0ivjHbKyXma5Lo4W4qu1nY= X-Gm-Gg: ASbGncuzysQmg95488oj723FyI7bgvu9pD84EJLYCr2WJBaobRId2P4qL0y+2dtIpYh ytPa56+BdpG0g57GIf5DOsi6Ygn36e7zYJKpsjqfcy1Q3DXMUPCxSVAwKykdMXE/Ly8MVjFxfWT xfj/YL2pB51Ui5fZVEseTcvw/SVC3CDJYX3lCcZrBBP/fwjOxR09qChv4+WgLdTTK5RkAwUxlhq vthB8vbeWVM7m2T+SUSpKhCCyzILpywLw3nmIOdU74p0G/dfEYGbg== X-Google-Smtp-Source: AGHT+IHKHhGCyFRWtFKBSMT5L4ew/c8wixVI2ld0t0cVkRo6mYGud8Dm5i0ZgL69OECg6KqC3VX9BA== X-Received: by 2002:a05:6a20:7350:b0:1e1:cbbf:be0 with SMTP id adf61e73a8af0-1e5e04a2f70mr98832700637.22.1735985393898; Sat, 04 Jan 2025 02:09:53 -0800 (PST) Original-Received: from terra ([2405:6586:be0:0:c8ff:1707:9b9:af89]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72aad8347d1sm28629159b3a.66.2025.01.04.02.09.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 04 Jan 2025 02:09:52 -0800 (PST) In-Reply-To: <87ttanrg9h.fsf@gmail.com> (Maxim Cournoyer's message of "Sun, 29 Dec 2024 00:14:50 +0900") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:298353 Archived-At: --=-=-= Content-Type: text/plain Hi Eli et al., Maxim Cournoyer writes: [...] >> I don't see how a user option to control this could be useful, since >> the preferred behavior is not only buffer-local, but also specific to >> certain syntactic constructs. But I won't object to having such an >> option. > > Having the behavior defined per-project or even globally (reverting to > the the pre-Emacs 28 behavior) via a simple option seems like it'd > simplify things, and make them discoverable. I tried fixing this generally, as it seems to me that something in lisp-mode should be meet the needs of all lisp-derived languages such as Scheme and not just Elisp. I first added two tests, one of which ensures no regression to the original bug that lead to this current behavioral change (bug#28937) and the other one that should pass once the issue reported here (bug#56197) is resolved. The last patch is a WIP that didn't work; I was hoping that inserting spaces corresponding to the width of the indent in the narrowed string would cause the indent to be preserved only for the first line. I don't have other ideas at the moment; I'd appreciate if someone could tip in. --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-test-lisp-Add-new-test-for-filling-behavior-change.patch >From aacf65440070df2ba356af1d118a50993fc8f865 Mon Sep 17 00:00:00 2001 From: Maxim Cournoyer Date: Mon, 30 Dec 2024 12:02:36 +0900 Subject: [PATCH 1/3] test/lisp: Add new test for filling behavior change. Starting with Emacs 28, filling strings now happens in a narrowed scope, and looses the leading indentation and can cause the string to extend past the fill-column value (see bug#56197). * test/lisp/emacs-lisp/lisp-mode-tests.el (lisp-fill-paragraph-respects-fill-column): New test. --- test/lisp/emacs-lisp/lisp-mode-tests.el | 27 +++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/lisp/emacs-lisp/lisp-mode-tests.el b/test/lisp/emacs-lisp/lisp-mode-tests.el index da02be65d03..9a2b1ea4654 100644 --- a/test/lisp/emacs-lisp/lisp-mode-tests.el +++ b/test/lisp/emacs-lisp/lisp-mode-tests.el @@ -308,6 +308,33 @@ lisp-indent-defun (indent-region (point-min) (point-max)) (should (equal (buffer-string) orig))))) + +;;; Filling + +(ert-deftest lisp-fill-paragraph-respects-fill-column () + "Test bug#56197 -- more specifically, validate that a leading indentation +for a string is preserved in the filled string." + (with-temp-buffer + ;; The following is a contrived example that demonstrates the + ;; fill-column problem when the string to fill is indented. + (insert "\ +\(defun test () + \"This is a long string which is indented by a considerable value, +causing it to protrude from the configured `fill-column' since +lisp-fill-paragraph was refactored in version 28.\" + (message \"Hi!\"))") + (emacs-lisp-mode) + (search-backward "This is a long string") + (fill-paragraph) ;function under test + (goto-char (point-min)) + (let ((i 1) + (lines-count (count-lines (point-min) (point-max)))) + (while (< i lines-count) + (beginning-of-line i) + (end-of-line) + (should (<= (current-column) fill-column)) + (setq i (1+ i)))))) + ;;; Fontification base-commit: e32484547d0813665334bfd34b741492dde0d374 -- 2.46.0 --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0002-test-lisp-Add-a-test-checking-docstring-boundaries-w.patch >From e5685497111ef71e57948f023f8d2a03647c3d69 Mon Sep 17 00:00:00 2001 From: Maxim Cournoyer Date: Mon, 30 Dec 2024 14:26:46 +0900 Subject: [PATCH 2/3] test/lisp: Add a test checking docstring boundaries when filling. * test/lisp/emacs-lisp/lisp-mode-tests.el (lisp-fill-paragraph-docstring-boundaries): New test. --- test/lisp/emacs-lisp/lisp-mode-tests.el | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/lisp/emacs-lisp/lisp-mode-tests.el b/test/lisp/emacs-lisp/lisp-mode-tests.el index 9a2b1ea4654..eac2763c595 100644 --- a/test/lisp/emacs-lisp/lisp-mode-tests.el +++ b/test/lisp/emacs-lisp/lisp-mode-tests.el @@ -311,6 +311,25 @@ lisp-indent-defun ;;; Filling +(ert-deftest lisp-fill-paragraph-docstring-boundaries () + "Test bug#28937, ensuring filling the docstring filled is properly +bounded." + (with-temp-buffer + (insert "\ +(defun test () + \"This is a test docstring. +Here is some more text.\" + 1 + 2 + 3 + 4 + 5)") + (let ((correct (buffer-string))) + (emacs-lisp-mode) + (search-backward "This is a test docstring") + (fill-paragraph) ;function under test + (should (equal (buffer-string) correct))))) + (ert-deftest lisp-fill-paragraph-respects-fill-column () "Test bug#56197 -- more specifically, validate that a leading indentation for a string is preserved in the filled string." -- 2.46.0 --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0003-wip-work-on-solution.patch >From 5bc5de2fc6f7783b0cd71c5945755fc98431aa60 Mon Sep 17 00:00:00 2001 From: Maxim Cournoyer Date: Tue, 31 Dec 2024 10:18:30 +0900 Subject: [PATCH 3/3] wip: work on solution --- lisp/emacs-lisp/lisp-mode.el | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el index d0c32d238bc..6c6d88f73a5 100644 --- a/lisp/emacs-lisp/lisp-mode.el +++ b/lisp/emacs-lisp/lisp-mode.el @@ -1,6 +1,6 @@ ;;; lisp-mode.el --- Lisp mode, and its idiosyncratic commands -*- lexical-binding:t -*- -;; Copyright (C) 1985-1986, 1999-2024 Free Software Foundation, Inc. +;; Copyright (C) 1985-1986, 1999-2025 Free Software Foundation, Inc. ;; Maintainer: emacs-devel@gnu.org ;; Keywords: lisp, languages @@ -1480,30 +1480,34 @@ lisp-fill-paragraph (derived-mode-p 'emacs-lisp-mode)) emacs-lisp-docstring-fill-column fill-column))) - (let ((ppss (syntax-ppss)) - (start (point)) - ;; Avoid recursion if we're being called directly with - ;; `M-x lisp-fill-paragraph' in an `emacs-lisp-mode' buffer. - (fill-paragraph-function t)) + (let* ((ppss (syntax-ppss)) + (start (point)) + ;; Avoid recursion if we're being called directly with + ;; `M-x lisp-fill-paragraph' in an `emacs-lisp-mode' buffer. + (fill-paragraph-function t) + (string-start (ppss-comment-or-string-start ppss)) + (indent (save-excursion + (goto-char string-start) + (current-column)))) (save-excursion (save-restriction ;; If we're not inside a string, then do very basic ;; filling. This avoids corrupting embedded strings in ;; code. - (if (not (ppss-comment-or-string-start ppss)) + (if (not string-start) (lisp--fill-line-simple) ;; If we're in a string, then narrow (roughly) to that ;; string before filling. This avoids filling Lisp ;; statements that follow the string. (when (ppss-string-terminator ppss) - (goto-char (ppss-comment-or-string-start ppss)) + (goto-char string-start) ;; The string may be unterminated -- in that case, don't ;; narrow. (when (ignore-errors (progn (forward-sexp 1) t)) - (narrow-to-region (1+ (ppss-comment-or-string-start ppss)) + (narrow-to-region (1+ string-start) (1- (point))))) ;; Move back to where we were. (goto-char start) @@ -1516,7 +1520,16 @@ lisp-fill-paragraph (goto-char (point-min)) (forward-line 1) (narrow-to-region (point) (point-max)))) - (fill-paragraph justify))))))) + ;; Insert spaces to reproduce the same leading indent + ;; for the string inside the narrowed region, to avoid + ;; bug#56197. + (save-excursion + (goto-char (point-min)) + (insert (make-string indent ?\s))) + (fill-paragraph justify) + (save-excursion ;clean up + (goto-char (point-min)) + (delete-char indent)))))))) ;; Never return nil. t) -- 2.46.0 --=-=-= Content-Type: text/plain If that's not easily fixable, perhaps I'll submit a patch to the paredit project so that they stop relying on lisp-paragraph-fill [0] and instead use the default (which is fill-paragraph). That's how I came to notice this behavior change in GNU Guix, which is written in Scheme; using scheme-mode doesn't exhibit the issue since it's just using fill-paragraph. [0] https://paredit.org/cgit/paredit/tree/paredit.el#n1066 -- Thanks, Maxim --=-=-=--