* orgalist-mode: wrong indentation in message mode after recent change in emacs
@ 2019-04-01 16:35 Gregor Zattler
2019-04-01 22:32 ` Basil L. Contovounesios
2019-04-12 21:10 ` Amin Bandali
0 siblings, 2 replies; 9+ messages in thread
From: Gregor Zattler @ 2019-04-01 16:35 UTC (permalink / raw)
To: emacs-orgmode, emacs-devel; +Cc: Basil L. Contovounesios
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 <contovob@tcd.ie>
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.
:040000 040000 88d3790ae5446f2ef84b66733c2b9e9bd27676a8 53a988cc46c8505b5751120b6f0813c5f252b171 M doc
:040000 040000 0f12bd57efcc2a481b468ecefb0369ef3b3996c9 055f07d1469cee692acc35ba9ce5ec2611a6f7db M etc
:040000 040000 0c34b9e6e31c833a7c3c4e464e0ccbf597156850 d330f001af22700ec84b0d1596571fa4548eafa8 M lisp
I do not know what to do with this.
Ciao; Gregor
--
-... --- .-. . -.. ..--.. ...-.-
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: orgalist-mode: wrong indentation in message mode after recent change in emacs 2019-04-01 16:35 orgalist-mode: wrong indentation in message mode after recent change in emacs Gregor Zattler @ 2019-04-01 22:32 ` Basil L. Contovounesios 2019-04-02 11:08 ` Stefan Monnier ` (2 more replies) 2019-04-12 21:10 ` Amin Bandali 1 sibling, 3 replies; 9+ messages in thread From: Basil L. Contovounesios @ 2019-04-01 22:32 UTC (permalink / raw) To: emacs-orgmode; +Cc: Stefan Monnier, emacs-devel [-- Attachment #1: Type: text/plain, Size: 3146 bytes --] [CCing Stefan as the nadvice.el expert.] Gregor Zattler <telegraph@gmx.net> 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 <contovob@tcd.ie> > 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ée 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: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: orgalist.diff --] [-- Type: text/x-diff, Size: 725 bytes --] 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) [-- Attachment #3: Type: text/plain, Size: 1029 bytes --] 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: orgalist-mode: wrong indentation in message mode after recent change in emacs 2019-04-01 22:32 ` Basil L. Contovounesios @ 2019-04-02 11:08 ` Stefan Monnier 2019-04-11 20:32 ` Stefan Monnier 2019-04-23 8:20 ` Nicolas Goaziou 2 siblings, 0 replies; 9+ messages in thread From: Stefan Monnier @ 2019-04-02 11:08 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: emacs-orgmode, emacs-devel > 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. I think this qualifies as a problem in nadvice: the ad-hoc closure introduced to "redirect to the default-value" should be handled by advice--cd*r (or rather by some new function which can then be used instead of advice--cd*r). Stefan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: orgalist-mode: wrong indentation in message mode after recent change in emacs 2019-04-01 22:32 ` Basil L. Contovounesios 2019-04-02 11:08 ` Stefan Monnier @ 2019-04-11 20:32 ` Stefan Monnier 2019-04-23 8:20 ` Nicolas Goaziou 2 siblings, 0 replies; 9+ messages in thread From: Stefan Monnier @ 2019-04-11 20:32 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: emacs-orgmode, emacs-devel > 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? Agreed. Comparing functions is always fraught with dangers, as we are witnessing here (this very fundamental problem was arguably the original motivation for inventing type-classes in Haskell to avoid the ugly ad-hoc "eqtypes" of SML). We need the comparison here for backward compatibility, but we should supplement it with a variable, like we did with `electric-indent-inhibit` (and not with electric-indent-functions-without-reindent which just suffers from the same fundamental problem). Stefan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: orgalist-mode: wrong indentation in message mode after recent change in emacs 2019-04-01 22:32 ` Basil L. Contovounesios 2019-04-02 11:08 ` Stefan Monnier 2019-04-11 20:32 ` Stefan Monnier @ 2019-04-23 8:20 ` Nicolas Goaziou 2019-04-23 10:53 ` Basil L. Contovounesios 2 siblings, 1 reply; 9+ messages in thread From: Nicolas Goaziou @ 2019-04-23 8:20 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: emacs-orgmode, Stefan Monnier, emacs-devel Hello, "Basil L. Contovounesios" <contovob@tcd.ie> writes: > 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. I don't know how that would work in practice. But a minor mode taking control over `indent-line-function' sounds wrong. > 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? So what is the current status of this? Do I still need to add a workaround around a workaround around a genuine Emacs bug? :) Regards, -- Nicolas Goaziou ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: orgalist-mode: wrong indentation in message mode after recent change in emacs 2019-04-23 8:20 ` Nicolas Goaziou @ 2019-04-23 10:53 ` Basil L. Contovounesios 2019-04-23 12:15 ` Stefan Monnier 2019-12-31 10:30 ` Nicolas Goaziou 0 siblings, 2 replies; 9+ messages in thread From: Basil L. Contovounesios @ 2019-04-23 10:53 UTC (permalink / raw) To: emacs-orgmode; +Cc: Stefan Monnier, emacs-devel Nicolas Goaziou <mail@nicolasgoaziou.fr> writes: > "Basil L. Contovounesios" <contovob@tcd.ie> writes: > >> 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. > > I don't know how that would work in practice. Me neither. > But a minor mode taking control over `indent-line-function' sounds > wrong. Well, orgalist already "takes control" over indent-line-function and indent-according-to-mode via advice, and the latter advice seems to assume that indent-line-function is set to the default indent-relative or indent-relative-first-indent-point. >> 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? > > So what is the current status of this? Do I still need to add > a workaround around a workaround around a genuine Emacs bug? :) Yes. :) I think the patch I proposed in my previous email should be applied to orgalist, as a first step at the very least. Thanks, -- Basil ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: orgalist-mode: wrong indentation in message mode after recent change in emacs 2019-04-23 10:53 ` Basil L. Contovounesios @ 2019-04-23 12:15 ` Stefan Monnier 2019-12-31 10:30 ` Nicolas Goaziou 1 sibling, 0 replies; 9+ messages in thread From: Stefan Monnier @ 2019-04-23 12:15 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: emacs-orgmode, emacs-devel >>> 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. >> I don't know how that would work in practice. > Me neither. >> But a minor mode taking control over `indent-line-function' sounds >> wrong. > Well, orgalist already "takes control" over indent-line-function and > indent-according-to-mode via advice, and the latter advice seems to > assume that indent-line-function is set to the default indent-relative > or indent-relative-first-indent-point. I haven't bothered to look at the advice to have an opinion here, so I'll let you guys figure out this part. >>> 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? Normally a hook runs both its local part and its global part, where the global part is represented in the local list by the special element `t`. When you do `make-local-variable` you're basically *copying* the global part to the local part, with the usual implications: when the global part is later modified those modifications won't be properly reflected in the local copy. That's why we had `make-local-hook` which is now automatically performed by `add-hook` depending on the `local` arg. The exact same thing goes for `add-function` when applied to a variable. In the current case, I think calling `make-local-variable` is likely harmless because *we* know the global value should likely never change, but that's not something that `add-function` knows to be true in general. So instead, `add-function` sets the local value to a function that looks up the global value and runs it, which is the moral equivalent of the `t` element on normal hooks. >>> The third is why indent-according-to-mode hard-codes the check for >>> indent-relative and indent-relative-first-indent-point. History. Comparing functions is always a bad idea. But I couldn't and still can't see how to avoid it here without introducing worse problems. >>> 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? Yes, tho it probably wouldn't help much here: not only we'd still be comparing functions, but we'd also need for orgalist to go through the trouble of manually adding the closure dynamically created by `add-function` to this list. Stefan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: orgalist-mode: wrong indentation in message mode after recent change in emacs 2019-04-23 10:53 ` Basil L. Contovounesios 2019-04-23 12:15 ` Stefan Monnier @ 2019-12-31 10:30 ` Nicolas Goaziou 1 sibling, 0 replies; 9+ messages in thread From: Nicolas Goaziou @ 2019-12-31 10:30 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: emacs-orgmode, Stefan Monnier, emacs-devel Hello, "Basil L. Contovounesios" <contovob@tcd.ie> writes: > Yes. :) I think the patch I proposed in my previous email should be > applied to orgalist, as a first step at the very least. Yay! 35 weeks later, but still before the coming year, I applied your patch to Orgalist code base. Thank you. Happy New Year, -- Nicolas Goaziou ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: orgalist-mode: wrong indentation in message mode after recent change in emacs 2019-04-01 16:35 orgalist-mode: wrong indentation in message mode after recent change in emacs Gregor Zattler 2019-04-01 22:32 ` Basil L. Contovounesios @ 2019-04-12 21:10 ` Amin Bandali 1 sibling, 0 replies; 9+ messages in thread From: Amin Bandali @ 2019-04-12 21:10 UTC (permalink / raw) To: emacs-orgmode Hi all, Just confirming that I suffer from this as well on latest master. It’d be great if this could be fixed. Cheers, amin ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-12-31 10:30 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-01 16:35 orgalist-mode: wrong indentation in message mode after recent change in emacs Gregor Zattler 2019-04-01 22:32 ` Basil L. Contovounesios 2019-04-02 11:08 ` Stefan Monnier 2019-04-11 20:32 ` Stefan Monnier 2019-04-23 8:20 ` Nicolas Goaziou 2019-04-23 10:53 ` Basil L. Contovounesios 2019-04-23 12:15 ` Stefan Monnier 2019-12-31 10:30 ` Nicolas Goaziou 2019-04-12 21:10 ` Amin Bandali
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).