* bug#11774: org-mode causes undo boundaries to be lost @ 2012-06-24 21:27 Toby Cubitt 2012-06-24 21:39 ` Bastien 0 siblings, 1 reply; 17+ messages in thread From: Toby Cubitt @ 2012-06-24 21:27 UTC (permalink / raw) To: 11774 Steps to reproduce, starting from "emacs -Q": 1. C-x C-f test.org 2. M-x org-mode 3. M-: (add-hook 'post-command-hook (lambda () (message "%s" buffer-undo-list)) nil t) 4. type "a" 5. C-<SPC> C-<SPC> 6. type "bc" Expected behaviour: contents of `buffer-undo-list' after step 6. should be ((2 . 4) nil (1 . 2) (t . -1)) Actual behaviour: contents of `buffer-undo-list' after step 6. are ((2 . 4) (1 . 2) (t . -1)) For some unknown reason, org-mode is causing the undo boundary between the (2 . 4) and (1 . 2) entries to be removed from `buffer-undo-list'. If we try the same thing under another major mode (e.g. replace step 2 with "M-x text-mode"), then `buffer-undo-list' does contain the undo boundary, as expected. Step 3 is there to monitor what's going on. The same results are obtained if that step is omitted. However, with the post-command-hook enabled, you can see that the undo boundary is still there after typing "b" in step 6, but gets deleted after typing "c". This might look like a minor bug, but it can have severe consequences. E.g. when using undo-tree-mode in org-mode, the corrupted `buffer-undo-list' contents cause undo-tree-mode to discard the entire undo history. Toby In GNU Emacs 24.1.50.2 (i686-pc-linux-gnu, GTK+ Version 2.24.10) of 2012-06-20 on c3po Windowing system distributor `The X.Org Foundation', version 11.0.11104000 Configured using: `configure '--prefix=/usr' '--host=i686-pc-linux-gnu' '--mandir=/usr/share/man' '--infodir=/usr/share/info' '--datadir=/usr/share' '--sysconfdir=/etc' '--localstatedir=/var/lib' '--disable-dependency-tracking' '--program-suffix=-emacs-24-vcs' '--infodir=/usr/share/info/emacs-24-vcs' '--enable-locallisppath=/etc/emacs:/usr/share/emacs/site-lisp' '--with-crt-dir=/usr/lib/gcc/i686-pc-linux-gnu/4.5.3/../../../../lib' '--with-gameuser=games' '--without-compress-info' '--without-hesiod' '--without-kerberos' '--without-kerberos5' '--with-gpm' '--with-dbus' '--without-gnutls' '--without-xml2' '--without-selinux' '--without-wide-int' '--without-sound' '--with-x' '--without-ns' '--without-gconf' '--without-gsettings' '--with-toolkit-scroll-bars' '--without-gif' '--without-jpeg' '--with-png' '--with-rsvg' '--without-tiff' '--with-xpm' '--without-imagemagick' '--with-xft' '--without-libotf' '--without-m17n-flt' '--with-x-toolkit=gtk' 'EBZR_BRANCH=trunk' 'EBZR_REVNO=108667' '--build=i686-pc-linux-gnu' 'build_alias=i686-pc-linux-gnu' 'host_alias=i686-pc-linux-gnu' 'CFLAGS=-march=prescott -O2 -pipe -O2' 'LDFLAGS=-Wl,-O1 -Wl,--as-needed' 'CPPFLAGS='' Important settings: value of $LANG: en_GB.utf-8 locale-coding-system: utf-8-unix default enable-multibyte-characters: t -- Dr T. S. Cubitt Mathematics and Quantum Information group Department of Mathematics Complutense University Madrid, Spain email: tsc25@cantab.net web: www.dr-qubit.org ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: org-mode causes undo boundaries to be lost 2012-06-24 21:27 bug#11774: org-mode causes undo boundaries to be lost Toby Cubitt @ 2012-06-24 21:39 ` Bastien 2012-06-24 21:52 ` Toby Cubitt 0 siblings, 1 reply; 17+ messages in thread From: Bastien @ 2012-06-24 21:39 UTC (permalink / raw) To: Toby Cubitt; +Cc: 11774 Hi Toby, Toby Cubitt <tsc25@cantab.net> writes: > For some unknown reason, org-mode is causing the undo boundary between > the (2 . 4) and (1 . 2) entries to be removed from `buffer-undo-list'. Can you try again with (setq org-self-insert-cluster-for-undo nil) and report? Thanks, -- Bastien ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: org-mode causes undo boundaries to be lost 2012-06-24 21:39 ` Bastien @ 2012-06-24 21:52 ` Toby Cubitt 2012-06-25 11:35 ` Toby Cubitt 0 siblings, 1 reply; 17+ messages in thread From: Toby Cubitt @ 2012-06-24 21:52 UTC (permalink / raw) To: Bastien; +Cc: 11774 On Sun, Jun 24, 2012 at 11:39:08PM +0200, Bastien wrote: > Hi Toby, > > Toby Cubitt <tsc25@cantab.net> writes: > > > For some unknown reason, org-mode is causing the undo boundary between > > the (2 . 4) and (1 . 2) entries to be removed from `buffer-undo-list'. > > Can you try again with > > (setq org-self-insert-cluster-for-undo nil) > > and report? Yup, that fixes the problem. I don't fully understand the purpose of `org-self-insert-cluster-for-undo', given that the Emacs command loop already groups consecutive undo entries together, but presumably it enables a more aggressive form of clustering. If the behaviour I reported is the intended behaviour with this option set, perhaps the clustering heuristic can be improved to avoid triggering history discarding in undo-tree-mode? undo-tree-mode puts a "canary" at the end of buffer-undo-list, to detect when Emacs discards undo history behind undo-tree-mode's back. I.e. before any undo entries have been added, buffer-undo-list contains: (nil undo-tree-canary) org-mode's undo clustering deletes the undo boundary before the undo-tree-canary entry, causing undo-tree-mode to think that Emacs has discarded undo history behind its back. I could try to work around this in undo-tree-mode, but it seems to me that org-mode shouldn't be throwing away an undo boundary that comes before an entry which definitely shouldn't be clustered with anything (namely the undo-tree-canary symbol, which is meaningless to org-mode). Toby -- Dr T. S. Cubitt Mathematics and Quantum Information group Department of Mathematics Complutense University Madrid, Spain email: tsc25@cantab.net web: www.dr-qubit.org ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: org-mode causes undo boundaries to be lost 2012-06-24 21:52 ` Toby Cubitt @ 2012-06-25 11:35 ` Toby Cubitt 2012-07-02 6:51 ` bug#11774: [O] " Martin Pohlack 2012-07-03 18:33 ` bug#11774: [O] " Martin Pohlack 0 siblings, 2 replies; 17+ messages in thread From: Toby Cubitt @ 2012-06-25 11:35 UTC (permalink / raw) To: Bastien; +Cc: 11774 On Sun, Jun 24, 2012 at 11:52:41PM +0200, Toby Cubitt wrote: > On Sun, Jun 24, 2012 at 11:39:08PM +0200, Bastien wrote: > > Hi Toby, > > > > Toby Cubitt <tsc25@cantab.net> writes: > > > > > For some unknown reason, org-mode is causing the undo boundary between > > > the (2 . 4) and (1 . 2) entries to be removed from `buffer-undo-list'. > > > > Can you try again with > > > > (setq org-self-insert-cluster-for-undo nil) > > > > and report? > > Yup, that fixes the problem. > > I don't fully understand the purpose of > `org-self-insert-cluster-for-undo', given that the Emacs command loop > already groups consecutive undo entries together, but presumably it > enables a more aggressive form of clustering. > [snip] > > I could try to work around this in undo-tree-mode, but it seems to me > that org-mode shouldn't be throwing away an undo boundary that comes > before an entry which definitely shouldn't be clustered with anything > (namely the undo-tree-canary symbol, which is meaningless to org-mode). I've now added a work-around for this in undo-tree-mode (currently only in git, but I'll push to ELPA once it's sufficiently tested). I'm still not entirely convinced that the boundary discarding logic in org-self-insert-command is correct. For example, if I do the following: 1. Type some text at some location in an org-mode buffer 2. Move to another location very far away (without invoking any commands other than point motion) 3. Type some more text then org-self-insert-cluster-for-undo collapses the undo changesets for these two changes into one. Undoing then reverts both sets of changes at once, even though those changes might be so far apart that they aren't both visible at the same time in the buffer. That seems very undesirable to me. But as I can work around it in undo-tree-mode, I'll leave it up to Bastien to decide whether or not this feature needs reworking in org-mode. Best, Toby -- Dr T. S. Cubitt Mathematics and Quantum Information group Department of Mathematics Complutense University Madrid, Spain email: tsc25@cantab.net web: www.dr-qubit.org ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: [O] bug#11774: org-mode causes undo boundaries to be lost 2012-06-25 11:35 ` Toby Cubitt @ 2012-07-02 6:51 ` Martin Pohlack 2012-07-03 9:57 ` Toby Cubitt 2012-07-03 18:33 ` bug#11774: [O] " Martin Pohlack 1 sibling, 1 reply; 17+ messages in thread From: Martin Pohlack @ 2012-07-02 6:51 UTC (permalink / raw) To: Toby Cubitt; +Cc: Bastien, 11774 Hi Toby, On 25.06.2012 13:35, Toby Cubitt wrote: > On Sun, Jun 24, 2012 at 11:52:41PM +0200, Toby Cubitt wrote: >> On Sun, Jun 24, 2012 at 11:39:08PM +0200, Bastien wrote: >>> Hi Toby, >>> >>> Toby Cubitt <tsc25@cantab.net> writes: >>> >>>> For some unknown reason, org-mode is causing the undo boundary between >>>> the (2 . 4) and (1 . 2) entries to be removed from `buffer-undo-list'. >>> >>> Can you try again with >>> >>> (setq org-self-insert-cluster-for-undo nil) >>> >>> and report? >> >> Yup, that fixes the problem. >> >> I don't fully understand the purpose of >> `org-self-insert-cluster-for-undo', given that the Emacs command loop >> already groups consecutive undo entries together, but presumably it >> enables a more aggressive form of clustering. >> > [snip] >> >> I could try to work around this in undo-tree-mode, but it seems to me >> that org-mode shouldn't be throwing away an undo boundary that comes >> before an entry which definitely shouldn't be clustered with anything >> (namely the undo-tree-canary symbol, which is meaningless to org-mode). > > I've now added a work-around for this in undo-tree-mode (currently only > in git, but I'll push to ELPA once it's sufficiently tested). > > I'm still not entirely convinced that the boundary discarding logic in > org-self-insert-command is correct. For example, if I do the following: > > 1. Type some text at some location in an org-mode buffer > 2. Move to another location very far away > (without invoking any commands other than point motion) > 3. Type some more text > > then org-self-insert-cluster-for-undo collapses the undo changesets for > these two changes into one. Undoing then reverts both sets of changes at > once, even though those changes might be so far apart that they aren't > both visible at the same time in the buffer. > > That seems very undesirable to me. Having been involved in org-mode's collapsing code I am interested in this, but I cannot reproduce your problem. I used a very large org-mode file, inserted some text, moved down some pages and inserted some text again (3 chars each). Undoing was split between both parts, exactly as desired. Could you provide more details please? Thanks, Martin ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: [O] bug#11774: org-mode causes undo boundaries to be lost 2012-07-02 6:51 ` bug#11774: [O] " Martin Pohlack @ 2012-07-03 9:57 ` Toby Cubitt 2012-07-03 15:18 ` Martin Pohlack 0 siblings, 1 reply; 17+ messages in thread From: Toby Cubitt @ 2012-07-03 9:57 UTC (permalink / raw) To: Martin Pohlack; +Cc: Bastien, 11774 On Mon, Jul 02, 2012 at 08:51:48AM +0200, Martin Pohlack wrote: > > I'm still not entirely convinced that the boundary discarding logic in > > org-self-insert-command is correct. For example, if I do the following: > > > > 1. Type some text at some location in an org-mode buffer > > 2. Move to another location very far away > > (without invoking any commands other than point motion) > > 3. Type some more text > > > > then org-self-insert-cluster-for-undo collapses the undo changesets for > > these two changes into one. Undoing then reverts both sets of changes at > > once, even though those changes might be so far apart that they aren't > > both visible at the same time in the buffer. > > > > That seems very undesirable to me. > > Having been involved in org-mode's collapsing code I am interested in > this, but I cannot reproduce your problem. I used a very large org-mode > file, inserted some text, moved down some pages and inserted some text > again (3 chars each). Undoing was split between both parts, exactly as > desired. Could you provide more details please? Sure. The following steps produce the effect I described, at least for me. This is on a fairly recent (a couple of weeks old) bzr build of Emacs, and a similarly recent git build of org-mode: 1. $ emacs -Q 2. C-x C-f test.org 3. M-x org-mode [not really necessary since already in org-mode] 5. C-u 50 M-x newline 6. M-< 7. type "a" 8. M-> 9. type "bc" buffer-undo-list now contains: (nil (52 . 54) (1 . 2) nil (1 . 51) (t . -1)) Note the lack of undo boundary between (52 . 54) and (1 . 2), which means that undoing once (C-/) deletes both "bc" *and* "a" in one step. HTH, Toby -- Dr T. S. Cubitt Mathematics and Quantum Information group Department of Mathematics Complutense University Madrid, Spain email: tsc25@cantab.net web: www.dr-qubit.org ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: [O] bug#11774: org-mode causes undo boundaries to be lost 2012-07-03 9:57 ` Toby Cubitt @ 2012-07-03 15:18 ` Martin Pohlack 2012-07-03 17:29 ` Stefan Monnier [not found] ` <jwvpq8coitq.fsf-monnier+emacs@gnu.org> 0 siblings, 2 replies; 17+ messages in thread From: Martin Pohlack @ 2012-07-03 15:18 UTC (permalink / raw) To: Toby Cubitt; +Cc: Bastien, 11774 On 03.07.2012 11:57, Toby Cubitt wrote: > On Mon, Jul 02, 2012 at 08:51:48AM +0200, Martin Pohlack wrote: >>> I'm still not entirely convinced that the boundary discarding logic in >>> org-self-insert-command is correct. For example, if I do the following: >>> >>> 1. Type some text at some location in an org-mode buffer >>> 2. Move to another location very far away >>> (without invoking any commands other than point motion) >>> 3. Type some more text >>> >>> then org-self-insert-cluster-for-undo collapses the undo changesets for >>> these two changes into one. Undoing then reverts both sets of changes at >>> once, even though those changes might be so far apart that they aren't >>> both visible at the same time in the buffer. >>> >>> That seems very undesirable to me. >> >> Having been involved in org-mode's collapsing code I am interested in >> this, but I cannot reproduce your problem. I used a very large org-mode >> file, inserted some text, moved down some pages and inserted some text >> again (3 chars each). Undoing was split between both parts, exactly as >> desired. Could you provide more details please? > > > Sure. The following steps produce the effect I described, at least for > me. This is on a fairly recent (a couple of weeks old) bzr build of > Emacs, and a similarly recent git build of org-mode: > > 1. $ emacs -Q > 2. C-x C-f test.org > 3. M-x org-mode [not really necessary since already in org-mode] > 5. C-u 50 M-x newline > 6. M-< > 7. type "a" > 8. M-> > 9. type "bc" > > buffer-undo-list now contains: > > (nil (52 . 54) (1 . 2) nil (1 . 51) (t . -1)) > > Note the lack of undo boundary between (52 . 54) and (1 . 2), which means > that undoing once (C-/) deletes both "bc" *and* "a" in one step. Understood. I tried exactly the same thing with an older emacs (GNU Emacs 23.2.1 (x86_64-pc-linux-gnu, GTK+ Version 2.24.4) of 2011-04-04 on crested, modified by Debian) and org-mode (6.33x and 7.6) and have this result: (nil (53 . 54) (52 . 53) nil (1 . 2) nil (1 . 51) (t 65535 . 65535)) Which is what one wants. Someone seems to be merging the self-insert commands in your situation. Probably the native merging code has changed in recent emacs itself. If this is confirmed, we could modify org-mode's merging code to only merge undo entries that span one character. Martin ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: [O] bug#11774: org-mode causes undo boundaries to be lost 2012-07-03 15:18 ` Martin Pohlack @ 2012-07-03 17:29 ` Stefan Monnier [not found] ` <jwvpq8coitq.fsf-monnier+emacs@gnu.org> 1 sibling, 0 replies; 17+ messages in thread From: Stefan Monnier @ 2012-07-03 17:29 UTC (permalink / raw) To: Martin Pohlack; +Cc: Bastien, Toby Cubitt, 11774 > Which is what one wants. Someone seems to be merging the self-insert > commands in your situation. Probably the native merging code has > changed in recent Emacs itself. Indeed, self-insert-command used to be treated specially by the read-eval-loop and the merging was performed there. Now this command is handled like any other, and self-insert-command does the merging itself. In most cases the result is the same, but the behavior is not quite identical in the details. Stefan ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <jwvpq8coitq.fsf-monnier+emacs@gnu.org>]
* bug#11774: [O] bug#11774: bug#11774: org-mode causes undo boundaries to be lost [not found] ` <jwvpq8coitq.fsf-monnier+emacs@gnu.org> @ 2012-07-03 18:13 ` Samuel Wales 2012-07-03 22:57 ` Stefan Monnier 0 siblings, 1 reply; 17+ messages in thread From: Samuel Wales @ 2012-07-03 18:13 UTC (permalink / raw) To: Stefan Monnier; +Cc: Bastien, Martin Pohlack, Toby Cubitt, 11774 On 7/3/12, Stefan Monnier <monnier@iro.umontreal.ca> wrote: > Indeed, self-insert-command used to be treated specially by the > read-eval-loop and the merging was performed there. Now this command is > handled like any other, and self-insert-command does the merging itself. > In most cases the result is the same, but the behavior is not quite > identical in the details. Here is the way I understand it: There are two problems with self-insert-command now, while before there was only one. The first is that it hardcodes the clustering by 20. As it happens, I strongly prefer clustering by 1. That is, I do not want undo to jump around or type more than necessary. I just want to autorepeat undo (C-/) until I get to where I need to be. Otherwise it is far too jerky and unpredictable and you have to type more either to delete (if you didn't undo far back enough) or add (if you undid too far). This was possible to work around before Emacs 24. You could advise self-insert-command or wrap it. This is why Org was able to control this with a variable to support clustering or not clustering. What is new in Emacs is that self-insert-command now destroys undo-boundary. If you wrap it, it destroys all of your effort on the next call to it. This causes subtle issues such as Org and undo-tree are dealing with. It is not reasonable to work around this because of the extra functionality you need to implement (especially if you want Org to do speed commands, table operations, and tag padding, which, who doesn't?). Unlike before, changing a hardcoded number or undo-boundary and then recompiling Emacs is necessary if you want to fix it. :( The loss of this ability to configure the cluster amount, and the subtle bug introduced, IMO merit a reversion or a fix to Emacs self-insert-command. Even just turning that magic 20 number into a variable would help. At least that's my understanding. I am just a user and I am not familiar with the internals. Samuel -- The Kafka Pandemic: http://thekafkapandemic.blogspot.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: [O] bug#11774: bug#11774: org-mode causes undo boundaries to be lost 2012-07-03 18:13 ` bug#11774: [O] bug#11774: " Samuel Wales @ 2012-07-03 22:57 ` Stefan Monnier 2012-07-04 0:18 ` Samuel Wales 0 siblings, 1 reply; 17+ messages in thread From: Stefan Monnier @ 2012-07-03 22:57 UTC (permalink / raw) To: Samuel Wales; +Cc: Bastien, Martin Pohlack, Toby Cubitt, 11774 > The first is that it hardcodes the clustering by 20. I guess that's the problem which is not new. > This was possible to work around before Emacs 24. You could advise > self-insert-command or wrap it. This is why Org was able to control > this with a variable to support clustering or not clustering. IIUC Org's clustering was only introduced because self-insert-command's clustering only worked when it was handled by the read-eval-loop, so it was not done to "avoid" the clustering, but to make it work in more cases. So it might be unneeded in Emacs-24. > What is new in Emacs is that self-insert-command now destroys > undo-boundary. If you wrap it, it destroys all of your effort on the > next call to it. I don't have it fresh in my memory but if you can post some sample code showing what you're doing, and how self-insert-command's behavior makes it difficult, maybe we can make it work better. > self-insert-command. Even just turning that magic 20 number into a > variable would help. Providing it as a variable would be very easy, indeed. Stefan ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: [O] bug#11774: bug#11774: org-mode causes undo boundaries to be lost 2012-07-03 22:57 ` Stefan Monnier @ 2012-07-04 0:18 ` Samuel Wales 2012-07-04 0:24 ` bug#11774: [O] bug#11774: " Samuel Wales ` (3 more replies) 0 siblings, 4 replies; 17+ messages in thread From: Samuel Wales @ 2012-07-04 0:18 UTC (permalink / raw) To: Stefan Monnier; +Cc: Bastien, Martin Pohlack, Toby Cubitt, 11774 On 7/3/12, Stefan Monnier <monnier@iro.umontreal.ca> wrote: >> self-insert-command. Even just turning that magic 20 number into a >> variable would help. > > Providing it as a variable would be very easy, indeed. Hi Stefan, To clarify, that is actually the only thing that I need as a user for self-insert-command. It would make an enormous difference in my use of Emacs. Huge. So I don't need undo-boundary to work in any particular way -- IF I have the ability to cluster self-insert-command by 1 instead of that hardcoded 20 throughout Emacs (including Org of course). My need for undo-boundary to work the way it did before is only so that I could call undo-boundary after every invocation of self-insert-command. Therefore, IF we have that variable, then undo-boundary considerations should be simply to DTRT for undo-tree, org-self-insert-command, and other code, in such a way that subtle bugs are prevented. Maybe the user should be able to set undo boundaries and have them work after self-insert-command? Dunno, I'm not familiar with internals enough to opine. Hope that clarifies. I will follow the discussion as long as I'm CC'ed as I am now. Thanks. Samuel -- The Kafka Pandemic: http://thekafkapandemic.blogspot.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: [O] bug#11774: bug#11774: bug#11774: org-mode causes undo boundaries to be lost 2012-07-04 0:18 ` Samuel Wales @ 2012-07-04 0:24 ` Samuel Wales 2012-07-04 9:40 ` bug#11774: [O] " Toby Cubitt ` (2 subsequent siblings) 3 siblings, 0 replies; 17+ messages in thread From: Samuel Wales @ 2012-07-04 0:24 UTC (permalink / raw) To: Stefan Monnier; +Cc: Bastien, Martin Pohlack, Toby Cubitt, 11774 On 7/3/12, Samuel Wales <samologist@gmail.com> wrote: > My need for undo-boundary to work the way it did That should read, "My need for self-insert-command to work the way it did". ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: [O] bug#11774: bug#11774: org-mode causes undo boundaries to be lost 2012-07-04 0:18 ` Samuel Wales 2012-07-04 0:24 ` bug#11774: [O] bug#11774: " Samuel Wales @ 2012-07-04 9:40 ` Toby Cubitt 2012-07-18 13:21 ` Stefan Monnier [not found] ` <jwvpq7t1a4t.fsf-monnier+emacs@gnu.org> 3 siblings, 0 replies; 17+ messages in thread From: Toby Cubitt @ 2012-07-04 9:40 UTC (permalink / raw) To: Samuel Wales; +Cc: Bastien, Martin Pohlack, 11774 On Tue, Jul 03, 2012 at 05:18:50PM -0700, Samuel Wales wrote: > On 7/3/12, Stefan Monnier <monnier@iro.umontreal.ca> wrote: > >> self-insert-command. Even just turning that magic 20 number into a > >> variable would help. > > > > Providing it as a variable would be very easy, indeed. > > Therefore, IF we have that variable, then undo-boundary considerations > should be simply to DTRT for undo-tree, org-self-insert-command, > and other code, in such a way that subtle bugs are prevented. Just to clarify the situation with undo-tree: it doesn't care how or when undo boundaries are inserted, and never did. The problem was only that undo boundaries that had nothing to do with self-insert-command were being *deleted*, due to the subtle interaction between the new Emacs-24 self-insert-command and org-mode's org-self-insert-cluster-for-undo feature. I've already pushed a change to the git version of undo-tree to make it work even if something else deletes undo boundaries that it shouldn't have touched (e.g. org-mode + Emacs-24). Deleting boundaries that have nothing to do with self-insert-command is still a bug, in my opinion, but it's a bug that no longer has catastrophic consequences in undo-tree-mode. It sounds from the preceding discussion that the right solution is simply to disable org-self-insert-cluster-for-undo in Emacs-24, since the purpose of org-mode's clustering was to recover the normal clustering behaviour and the standard self-insert-command clustering now works in org-mode with Emacs-24. The question of how to customize the clustering granularity seems to me to be a separate question (which can presumably be solved by making the hard-coded 20 into a variable). Toby -- Dr T. S. Cubitt Mathematics and Quantum Information group Department of Mathematics Complutense University Madrid, Spain email: tsc25@cantab.net web: www.dr-qubit.org ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: [O] bug#11774: bug#11774: org-mode causes undo boundaries to be lost 2012-07-04 0:18 ` Samuel Wales 2012-07-04 0:24 ` bug#11774: [O] bug#11774: " Samuel Wales 2012-07-04 9:40 ` bug#11774: [O] " Toby Cubitt @ 2012-07-18 13:21 ` Stefan Monnier [not found] ` <jwvpq7t1a4t.fsf-monnier+emacs@gnu.org> 3 siblings, 0 replies; 17+ messages in thread From: Stefan Monnier @ 2012-07-18 13:21 UTC (permalink / raw) To: Samuel Wales; +Cc: Bastien, 11774-done, Martin Pohlack, Toby Cubitt >>> self-insert-command. Even just turning that magic 20 number into a >>> variable would help. >> Providing it as a variable would be very easy, indeed. > Maybe the user should be able to set undo boundaries and > have them work after self-insert-command? Dunno, I'm > not familiar with internals enough to opine. I installed the patch below which makes self-insert-command more careful to only remove undo boundaries that were auto-added. So (add-hook 'post-self-insert-hook #'undo-boundary 'append) should give you pretty much the behavior you were looking for. Stefan === modified file 'src/ChangeLog' --- src/ChangeLog 2012-07-18 05:44:36 +0000 +++ src/ChangeLog 2012-07-18 13:17:22 +0000 @@ -1,3 +1,10 @@ +2012-07-18 Stefan Monnier <monnier@iro.umontreal.ca> + + * lisp.h (last_undo_boundary): Declare new var. + * keyboard.c (command_loop_1): Set it. + * cmds.c (Fself_insert_command): Use it to only remove boundaries that + were auto-added by the command loop (bug#11774). + 2012-07-18 Dmitry Antipov <dmantipov@yandex.ru> Return more descriptive data from Fgarbage_collect. === modified file 'src/cmds.c' --- src/cmds.c 2012-06-16 12:24:15 +0000 +++ src/cmds.c 2012-07-18 13:08:43 +0000 @@ -296,7 +296,10 @@ if (remove_boundary && CONSP (BVAR (current_buffer, undo_list)) - && NILP (XCAR (BVAR (current_buffer, undo_list)))) + && NILP (XCAR (BVAR (current_buffer, undo_list))) + /* Only remove auto-added boundaries, not boundaries + added be explicit calls to undo-boundary. */ + && EQ (BVAR (current_buffer, undo_list), last_undo_boundary)) /* Remove the undo_boundary that was just pushed. */ BVAR (current_buffer, undo_list) = XCDR (BVAR (current_buffer, undo_list)); === modified file 'src/keyboard.c' --- src/keyboard.c 2012-07-12 03:45:46 +0000 +++ src/keyboard.c 2012-07-18 13:13:31 +0000 @@ -1318,6 +1318,9 @@ } #endif +/* The last boundary auto-added to buffer-undo-list. */ +Lisp_Object last_undo_boundary; + /* FIXME: This is wrong rather than test window-system, we should call a new set-selection, which will then dispatch to x-set-selection, or tty-set-selection, or w32-set-selection, ... */ @@ -1565,7 +1568,13 @@ #endif if (NILP (KVAR (current_kboard, Vprefix_arg))) /* FIXME: Why? --Stef */ + { + Lisp_Object undo = BVAR (current_buffer, undo_list); Fundo_boundary (); + last_undo_boundary + = (EQ (undo, BVAR (current_buffer, undo_list)) + ? Qnil : BVAR (current_buffer, undo_list)); + } Fcommand_execute (Vthis_command, Qnil, Qnil, Qnil); #ifdef HAVE_WINDOW_SYSTEM === modified file 'src/lisp.h' --- src/lisp.h 2012-07-18 05:44:36 +0000 +++ src/lisp.h 2012-07-18 13:05:33 +0000 @@ -2921,7 +2921,7 @@ extern void syms_of_search (void); extern void clear_regexp_cache (void); -/* Defined in minibuf.c */ +/* Defined in minibuf.c. */ extern Lisp_Object Qcompletion_ignore_case; extern Lisp_Object Vminibuffer_list; @@ -2930,25 +2930,25 @@ extern void init_minibuf_once (void); extern void syms_of_minibuf (void); -/* Defined in callint.c */ +/* Defined in callint.c. */ extern Lisp_Object Qminus, Qplus; extern Lisp_Object Qwhen; extern Lisp_Object Qcall_interactively, Qmouse_leave_buffer_hook; extern void syms_of_callint (void); -/* Defined in casefiddle.c */ +/* Defined in casefiddle.c. */ extern Lisp_Object Qidentity; extern void syms_of_casefiddle (void); extern void keys_of_casefiddle (void); -/* Defined in casetab.c */ +/* Defined in casetab.c. */ extern void init_casetab_once (void); extern void syms_of_casetab (void); -/* Defined in keyboard.c */ +/* Defined in keyboard.c. */ extern Lisp_Object echo_message_buffer; extern struct kboard *echo_kboard; @@ -2956,6 +2956,7 @@ extern Lisp_Object Qdisabled, QCfilter; extern Lisp_Object Qup, Qdown, Qbottom; extern Lisp_Object Qtop; +extern Lisp_Object last_undo_boundary; extern int input_pending; extern Lisp_Object menu_bar_items (Lisp_Object); extern Lisp_Object tool_bar_items (Lisp_Object, int *); @@ -2976,13 +2977,13 @@ extern void syms_of_keyboard (void); extern void keys_of_keyboard (void); -/* Defined in indent.c */ +/* Defined in indent.c. */ extern ptrdiff_t current_column (void); extern void invalidate_current_column (void); extern int indented_beyond_p (ptrdiff_t, ptrdiff_t, EMACS_INT); extern void syms_of_indent (void); -/* Defined in frame.c */ +/* Defined in frame.c. */ extern Lisp_Object Qonly; extern Lisp_Object Qvisible; extern void store_frame_param (struct frame *, Lisp_Object, Lisp_Object); @@ -2995,7 +2996,7 @@ extern void frames_discard_buffer (Lisp_Object); extern void syms_of_frame (void); -/* Defined in emacs.c */ +/* Defined in emacs.c. */ extern char **initial_argv; extern int initial_argc; #if defined (HAVE_X_WINDOWS) || defined (HAVE_NS) ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <jwvpq7t1a4t.fsf-monnier+emacs@gnu.org>]
* bug#11774: bug#11774: bug#11774: org-mode causes undo boundaries to be lost [not found] ` <jwvpq7t1a4t.fsf-monnier+emacs@gnu.org> @ 2012-08-01 14:26 ` Bastien [not found] ` <CAJcAo8ux9Dw5Nu6x0jm59mWFaLWFG6SSeMs9dju-Jgy5nWkUcA@mail.gmail.com> 1 sibling, 0 replies; 17+ messages in thread From: Bastien @ 2012-08-01 14:26 UTC (permalink / raw) To: 11774 Stefan Monnier <monnier@IRO.UMontreal.CA> writes: >>>> self-insert-command. Even just turning that magic 20 number into a >>>> variable would help. >>> Providing it as a variable would be very easy, indeed. >> Maybe the user should be able to set undo boundaries and >> have them work after self-insert-command? Dunno, I'm >> not familiar with internals enough to opine. > > I installed the patch below which makes self-insert-command more careful > to only remove undo boundaries that were auto-added. > So (add-hook 'post-self-insert-hook #'undo-boundary 'append) should give you > pretty much the behavior you were looking for. On Org's side, `org-self-insert-cluster-for-undo' now defaults to nil for Emacs >=24.1, t otherwise. Thanks, -- Bastien ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <CAJcAo8ux9Dw5Nu6x0jm59mWFaLWFG6SSeMs9dju-Jgy5nWkUcA@mail.gmail.com>]
* bug#11774: [O] bug#11774: bug#11774: org-mode causes undo boundaries to be lost [not found] ` <CAJcAo8ux9Dw5Nu6x0jm59mWFaLWFG6SSeMs9dju-Jgy5nWkUcA@mail.gmail.com> @ 2014-11-03 14:35 ` Stefan Monnier 0 siblings, 0 replies; 17+ messages in thread From: Stefan Monnier @ 2014-11-03 14:35 UTC (permalink / raw) To: Samuel Wales; +Cc: Bastien, 11774-done, Martin Pohlack, Toby Cubitt > There is one possible bug. I have undo-boundary on self-insert-hook. > If I do newline-and-indent, for some reason both the newline and the > indent get undo boundaries. My expectation is that typing RET should > only have one undo boundary. > I don't know if that is user expectation error, but seems worth mentioning. Maybe the issue is simply that you need to be careful to put your undo-boundary "late" on the post-self-insert-hook (i.e. to use the `append' argument of add-hook). If not, then please post a new bug-report about it, so we can see on which side it should be solved. Stefan ^ permalink raw reply [flat|nested] 17+ messages in thread
* bug#11774: [O] bug#11774: org-mode causes undo boundaries to be lost 2012-06-25 11:35 ` Toby Cubitt 2012-07-02 6:51 ` bug#11774: [O] " Martin Pohlack @ 2012-07-03 18:33 ` Martin Pohlack 1 sibling, 0 replies; 17+ messages in thread From: Martin Pohlack @ 2012-07-03 18:33 UTC (permalink / raw) To: Toby Cubitt; +Cc: Bastien, 11774 On 25.06.2012 13:35, Toby Cubitt wrote: > On Sun, Jun 24, 2012 at 11:52:41PM +0200, Toby Cubitt wrote: >> On Sun, Jun 24, 2012 at 11:39:08PM +0200, Bastien wrote: >>> Hi Toby, >>> >>> Toby Cubitt <tsc25@cantab.net> writes: >>> >>>> For some unknown reason, org-mode is causing the undo boundary between >>>> the (2 . 4) and (1 . 2) entries to be removed from `buffer-undo-list'. >>> >>> Can you try again with >>> >>> (setq org-self-insert-cluster-for-undo nil) >>> >>> and report? >> >> Yup, that fixes the problem. >> >> I don't fully understand the purpose of >> `org-self-insert-cluster-for-undo', given that the Emacs command loop >> already groups consecutive undo entries together, but presumably it >> enables a more aggressive form of clustering. Just to clarify this little piece here: I originally introduces this clustering code to mimic emacs' behavior in text mode. Org-mode's aggressive interception seemed to prevent that and undoing in org-mode felt unnatural compared to vanilla text mode. It was never meant to be *more* aggressive than emacs' original behavior. The grouping did not happen at the time. The original thread is here: http://lists.gnu.org/archive/html/emacs-orgmode/2009-02/msg00691.html Martin ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-11-03 14:35 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-06-24 21:27 bug#11774: org-mode causes undo boundaries to be lost Toby Cubitt 2012-06-24 21:39 ` Bastien 2012-06-24 21:52 ` Toby Cubitt 2012-06-25 11:35 ` Toby Cubitt 2012-07-02 6:51 ` bug#11774: [O] " Martin Pohlack 2012-07-03 9:57 ` Toby Cubitt 2012-07-03 15:18 ` Martin Pohlack 2012-07-03 17:29 ` Stefan Monnier [not found] ` <jwvpq8coitq.fsf-monnier+emacs@gnu.org> 2012-07-03 18:13 ` bug#11774: [O] bug#11774: " Samuel Wales 2012-07-03 22:57 ` Stefan Monnier 2012-07-04 0:18 ` Samuel Wales 2012-07-04 0:24 ` bug#11774: [O] bug#11774: " Samuel Wales 2012-07-04 9:40 ` bug#11774: [O] " Toby Cubitt 2012-07-18 13:21 ` Stefan Monnier [not found] ` <jwvpq7t1a4t.fsf-monnier+emacs@gnu.org> 2012-08-01 14:26 ` bug#11774: " Bastien [not found] ` <CAJcAo8ux9Dw5Nu6x0jm59mWFaLWFG6SSeMs9dju-Jgy5nWkUcA@mail.gmail.com> 2014-11-03 14:35 ` bug#11774: [O] " Stefan Monnier 2012-07-03 18:33 ` bug#11774: [O] " Martin Pohlack
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.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).