From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Basil L. Contovounesios" Subject: Re: orgalist-mode: wrong indentation in message mode after recent change in emacs Date: Mon, 01 Apr 2019 23:32:40 +0100 Message-ID: <87ef6l9x13.fsf@tcd.ie> References: <87k1gdptsn.fsf@len.workgroup> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([209.51.188.92]:59613) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hB5U7-0005sh-IO for emacs-orgmode@gnu.org; Mon, 01 Apr 2019 18:32:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hB5U5-0003fH-CE for emacs-orgmode@gnu.org; Mon, 01 Apr 2019 18:32:55 -0400 Received: from mail-ed1-x52e.google.com ([2a00:1450:4864:20::52e]:45836) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hB5U2-0003Xz-J1 for emacs-orgmode@gnu.org; Mon, 01 Apr 2019 18:32:52 -0400 Received: by mail-ed1-x52e.google.com with SMTP id m16so9813851edd.12 for ; Mon, 01 Apr 2019 15:32:44 -0700 (PDT) In-Reply-To: <87k1gdptsn.fsf@len.workgroup> (Gregor Zattler's message of "Mon, 01 Apr 2019 18:35:52 +0200") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: emacs-orgmode@gnu.org Cc: Stefan Monnier , emacs-devel@gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable [CCing Stefan as the nadvice.el expert.] Gregor Zattler writes: > Dear org-mode developers, hello Nicolas, > I use orgalist-mode while writing emails in > notmuch-message-mode. Today I updated emacs (from git), and > since then, the second line of a paragraph get's indented > (and succeeding lines too). > > As demonstrated with this email. While I write this email with > my configurations, I checked with emacs -Q as follows: visited > one org file (in order for org-mode to load), visited > orgalist.el, eval'ed this buffer and started writing an email: > bang! indented succeeding lines. > > > > I bisected emacs like so: > > make ; src/emacs -Q --eval '(org-mode)' -l > ~/.emacs.d/elpa-27.0/orgalist-1.9/orgalist.el -f compose-mail --eval > '(orgalist-mode)' -f end-of-buffer > > and this is the relevant commit: > > 2e3deb09bd42d22a9b354937ce63b151fb493d8a is the first bad commit > commit 2e3deb09bd42d22a9b354937ce63b151fb493d8a > Author: Basil L. Contovounesios > Date: Sun Mar 31 19:39:54 2019 +0100 > > Do not set indent-line-function in text-mode > > For discussion, see thread starting at: > https://lists.gnu.org/archive/html/emacs-devel/2019-03/msg01012.html > * lisp/textmodes/text-mode.el (text-mode): Do not reset > indent-line-function to its global default value of indent-relative. > * doc/lispref/modes.texi (Example Major Modes): > * etc/NEWS: Document change accordingly. [...] > I do not know what to do with this. This seems to be a relapse(?) of bug#31361[1], which you have correctly identified as being caused by my commit. The following outlines what happens now (you can safely skip the next four paragraphs if you don't care about the technical details). text-mode no longer makes indent-line-function buffer-local. orgalist-mode advises indent-line-function buffer-locally using add-function. add-function uses advice--buffer-local to find which place form to set. advice--buffer-local sees that indent-line-function is not a local variable, so it wraps it in a custom closure before making it buffer-local. orgalist-mode also advises indent-according-to-mode in order to work around bug#31361 by temporarily unwrapping the advice added to indent-line-function. This is done because indent-according-to-mode behaves differently if indent-line-function is set to indent-relative or indent-relative-first-indent-point (n=C3=A9e indent-relative-maybe). Back when text-mode made indent-line-function buffer-local before orgalist-mode advised it, (advice--cd*r indent-line-function) would simply return indent-relative, and the advice on indent-according-to-mode worked as intended. Now that add-function sets indent-line-function to a custom closure, however, (advice--cd*r indent-line-function) no longer returns indent-relative and the workaround breaks. The simplest fix would be to make indent-line-function buffer-local in orgalist-mode before advising it with add-function, as the code already expects and relies on this: --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=orgalist.diff diff --git a/orgalist.el b/orgalist.el index 3a3cfaa..cc14430 100644 --- a/orgalist.el +++ b/orgalist.el @@ -818,6 +818,11 @@ orgalist-mode (add-function :before-until (local 'fill-paragraph-function) #'orgalist--fill-item) + ;; Unless `indent-line-function' is buffer-local before it is + ;; advised with `add-function', the workaround for bug#31361 below + ;; will not work, as (advice--cd*r indent-line-function) will not + ;; compare `eq' to `indent-relative' in `indent-according-to-mode'. + (make-local-variable 'indent-line-function) (add-function :before-until (local 'indent-line-function) #'orgalist--indent-line) --=-=-= Content-Type: text/plain Another option, of course, would be to revert my commit. I'm not in the slightest against reverting, but I'm not yet convinced it's necessary, as orgalist-mode currently seems to rely on the alignment of several implementation details. I do wonder about three things, though. The first is whether orgalist-mode couldn't use a custom indent-line-function instead of advising what may or may not be set to indent-relative by the user. The second is why advice--buffer-local does what it does. Stefan, why does it behave differently depending on local-variable-p? Why can't it simply call make-local-variable before returning the symbol-value? The third is why indent-according-to-mode hard-codes the check for indent-relative and indent-relative-first-indent-point. Wouldn't it be nice if this check instead looked up some variable akin to electric-indent-functions-without-reindent, that can be more easily customised? [1]: https://debbugs.gnu.org/31361 Thanks for reporting this, and sorry for the fallout. -- Basil --=-=-=--