unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* 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

* 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: 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

* 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

* 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

* 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

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).