emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [bug] [babel] babel corrupts undo history
@ 2013-08-28  1:13 Samuel Wales
  2013-08-28 14:42 ` Eric Schulte
  0 siblings, 1 reply; 26+ messages in thread
From: Samuel Wales @ 2013-08-28  1:13 UTC (permalink / raw)
  To: emacs-orgmode

c-c ' c-c ' on this.  then undo or undo-tree-undo.

what happens is data corruption.  to me, undo is a low-level
operation that should always work, even with a syntactically
invalid block.

a relevant variable is org-src-preserve-indentation.  it is
possible to change it without changing a block.

===

#+BEGIN_SRC sh :noweb yes :results verbatim output
  (
      cat <<EOF
test
EOF
  ) 2>&1
  :
#+END_SRC

===

Emacs 24.2, recent Org, recent undo-tree-mode.

Thanks.

Samuel

P.S.  You might be wondering why I use a subshell and a
redirection followed by a null command.  This is because I
prefer to bypass Babel's error mechanism, which I find
confusing.  Doing exactly as above is the only way I know to
get Babel to work the way I prefer (although {} might or
might not work also).  I always do this for every block.  IMO it would
be great for newcomers if there were options in Babel sh blocks for
exactly this.

-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  ANYBODY can get it.

Denmark: free Karina Hansen NOW.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-08-28  1:13 [bug] [babel] babel corrupts undo history Samuel Wales
@ 2013-08-28 14:42 ` Eric Schulte
  2013-08-28 16:03   ` Aaron Ecay
  2013-08-28 17:08   ` Samuel Wales
  0 siblings, 2 replies; 26+ messages in thread
From: Eric Schulte @ 2013-08-28 14:42 UTC (permalink / raw)
  To: Samuel Wales; +Cc: emacs-orgmode

I don't understand.  Is the problem that edits made in the indirect
buffer are not spliced into the undo history of the Org-mode buffer?

Samuel Wales <samologist@gmail.com> writes:

> c-c ' c-c ' on this.  then undo or undo-tree-undo.
>
> what happens is data corruption.  to me, undo is a low-level
> operation that should always work, even with a syntactically
> invalid block.
>
> a relevant variable is org-src-preserve-indentation.  it is
> possible to change it without changing a block.
>
> ===
>
> #+BEGIN_SRC sh :noweb yes :results verbatim output
>   (
>       cat <<EOF
> test
> EOF
>   ) 2>&1
>   :
> #+END_SRC
>
> ===
>
> Emacs 24.2, recent Org, recent undo-tree-mode.
>
> Thanks.
>
> Samuel
>
> P.S.  You might be wondering why I use a subshell and a
> redirection followed by a null command.  This is because I
> prefer to bypass Babel's error mechanism, which I find
> confusing.  Doing exactly as above is the only way I know to
> get Babel to work the way I prefer (although {} might or
> might not work also).  I always do this for every block.  IMO it would
> be great for newcomers if there were options in Babel sh blocks for
> exactly this.

-- 
Eric Schulte
https://cs.unm.edu/~eschulte
PGP: 0x614CA05D

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-08-28 14:42 ` Eric Schulte
@ 2013-08-28 16:03   ` Aaron Ecay
  2013-08-28 18:52     ` Eric Schulte
  2013-08-28 17:08   ` Samuel Wales
  1 sibling, 1 reply; 26+ messages in thread
From: Aaron Ecay @ 2013-08-28 16:03 UTC (permalink / raw)
  To: Eric Schulte, Samuel Wales; +Cc: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]

Hi Eric and Samuel,

As I understand it, the problem is that the undo history gets scrambled
by the interleaving of user edits (in the indirect source-editing
buffer) and automatic changes introduced by org (un- and re-indenting
the source code).

I have the following patch, which seems to prevent the misbehavior
Samuel noticed.  It has the drawback of not keeping the fine-grained
undo information: after org-src-edit-exit, all changes made during the
edit are seen as only one change, and undone as a unit.

I think the problem of interleaving the automatic and user-driven
changes in a sensible way is tricky.  We don’t want the first invocation
of undo after org-src-edit-exit to remove the contents of the code
block, which is what a naive approach gives (since org-src-edit-exit
deletes then reinserts the code block contents).

I’ve been running with this patch for a while and not noticed any ill
effects.  But I haven’t made a concerted attempt to test undo around
code blocks, which is why I’ve held off on pushing it.  If it fixes
Samuel’s problem and looks good, perhaps it is ready to go.


[-- Attachment #2: 0001-Fix-org-src-edit-interaction-with-undo.patch --]
[-- Type: text/x-diff, Size: 1781 bytes --]

From 4a55d50e46eeebe9346cd10173b8c3a5a8baa7c6 Mon Sep 17 00:00:00 2001
From: Aaron Ecay <aaronecay@gmail.com>
Date: Wed, 28 Aug 2013 11:50:53 -0400
Subject: [PATCH] Fix org-src-edit interaction with undo.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* org-src.el (org-edit-src-exit): Place an undo boundary before
writing changes back to parent buffer.

The previous code attempted to preserve the undo information in the
indirect buffer editing the source code, but this interacts poorly
with the undo system, and can lead to undo operations scrambling the
buffer.  The new approach means that edits made in the indirect buffer
cannot be undone piece-by-piece (instead, all changes made in the
indirect buffer constitute one “change” from the point of view of
undo), but the misbehavior of undo is (hopefully) now avoided.
---
 lisp/org-src.el | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lisp/org-src.el b/lisp/org-src.el
index 0f88174..96a413e 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -753,12 +753,12 @@ with \",*\", \",#+\", \",,*\" and \",,#+\"."
       (kill-buffer buffer))
     (goto-char beg)
     (when allow-write-back-p
-      (let ((buffer-undo-list t))
-	(delete-region beg (max beg end))
-	(unless (string-match "\\`[ \t]*\\'" code)
-	  (insert code))
-	(goto-char beg)
-	(if single (just-one-space))))
+      (undo-boundary)
+      (delete-region beg (max beg end))
+      (unless (string-match "\\`[ \t]*\\'" code)
+	(insert code))
+      (goto-char beg)
+      (if single (just-one-space)))
     (if (memq t (mapcar (lambda (overlay)
 			  (eq (overlay-get overlay 'invisible)
 			      'org-hide-block))
-- 
1.8.4


[-- Attachment #3: Type: text/plain, Size: 15 bytes --]


--
Aaron Ecay

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-08-28 14:42 ` Eric Schulte
  2013-08-28 16:03   ` Aaron Ecay
@ 2013-08-28 17:08   ` Samuel Wales
  2013-08-28 17:18     ` Samuel Wales
  1 sibling, 1 reply; 26+ messages in thread
From: Samuel Wales @ 2013-08-28 17:08 UTC (permalink / raw)
  To: Eric Schulte; +Cc: emacs-orgmode

Hi Eric,

On 8/28/13, Eric Schulte <schulte.eric@gmail.com> wrote:
> I don't understand.  Is the problem that edits made in the indirect
> buffer are not spliced into the undo history of the Org-mode buffer?

No, it is buffer corruption, which is a bug.  Undo is not working
correctly in any interpretation.

Granularly incorporating changes in the base buffer and/or the
indirect buffer would definitely be useful.

But not corrupting the buffer is MORE useful.  :)

Samuel

-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  ANYBODY can get it.

Denmark: free Karina Hansen NOW.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-08-28 17:08   ` Samuel Wales
@ 2013-08-28 17:18     ` Samuel Wales
  0 siblings, 0 replies; 26+ messages in thread
From: Samuel Wales @ 2013-08-28 17:18 UTC (permalink / raw)
  To: Eric Schulte; +Cc: emacs-orgmode

Another way of putting it is that Babel editing seems to be trying to
do some fancy things with undo, and that has led to buffer corruption.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-08-28 16:03   ` Aaron Ecay
@ 2013-08-28 18:52     ` Eric Schulte
  2013-08-28 19:41       ` Samuel Wales
  0 siblings, 1 reply; 26+ messages in thread
From: Eric Schulte @ 2013-08-28 18:52 UTC (permalink / raw)
  To: Samuel Wales; +Cc: emacs-orgmode

Aaron Ecay <aaronecay@gmail.com> writes:

> Hi Eric and Samuel,
>
> As I understand it, the problem is that the undo history gets scrambled
> by the interleaving of user edits (in the indirect source-editing
> buffer) and automatic changes introduced by org (un- and re-indenting
> the source code).
>
> I have the following patch, which seems to prevent the misbehavior
> Samuel noticed.  It has the drawback of not keeping the fine-grained
> undo information: after org-src-edit-exit, all changes made during the
> edit are seen as only one change, and undone as a unit.
>
> I think the problem of interleaving the automatic and user-driven
> changes in a sensible way is tricky.  We don’t want the first invocation
> of undo after org-src-edit-exit to remove the contents of the code
> block, which is what a naive approach gives (since org-src-edit-exit
> deletes then reinserts the code block contents).
>
> I’ve been running with this patch for a while and not noticed any ill
> effects.  But I haven’t made a concerted attempt to test undo around
> code blocks, which is why I’ve held off on pushing it.  If it fixes
> Samuel’s problem and looks good, perhaps it is ready to go.
>

Aaron, thanks for this fix.

Sam, does this patch fix your problem?

If so then I think it should be applied.  I didn't write and am not
familiar with this code, so I'll leave application to someone more
competent than myself.

-- 
Eric Schulte
https://cs.unm.edu/~eschulte
PGP: 0x614CA05D

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-08-28 18:52     ` Eric Schulte
@ 2013-08-28 19:41       ` Samuel Wales
  2013-08-28 19:43         ` Samuel Wales
  0 siblings, 1 reply; 26+ messages in thread
From: Samuel Wales @ 2013-08-28 19:41 UTC (permalink / raw)
  To: Eric Schulte; +Cc: emacs-orgmode

On 8/28/13, Eric Schulte <schulte.eric@gmail.com> wrote:
> Aaron, thanks for this fix.
>
> Sam, does this patch fix your problem?

IMO it is the right strategy.

Editing does insert spaces, making the block syntactically correct for
one value of org-src-preserve-indentation [it was already correct for
the other value].  That's not corruption, just correction.

The problem is that you can't undo the spaces.  Ideally it should take
you back to where you are before you edit.  Maybe there is another
place where undo is turned off?

Still, it is a huge improvement over buffer corruption.

Thanks.

Samuel

-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  ANYBODY can get it.

Denmark: free Karina Hansen NOW.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-08-28 19:41       ` Samuel Wales
@ 2013-08-28 19:43         ` Samuel Wales
  2013-08-28 19:51           ` Samuel Wales
  0 siblings, 1 reply; 26+ messages in thread
From: Samuel Wales @ 2013-08-28 19:43 UTC (permalink / raw)
  To: Eric Schulte; +Cc: emacs-orgmode

Perhaps the undo code in org-src-in-org-buffer can be removed?  [Note:
I have no idea.]

-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  ANYBODY can get it.

Denmark: free Karina Hansen NOW.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-08-28 19:43         ` Samuel Wales
@ 2013-08-28 19:51           ` Samuel Wales
  2013-10-28 19:17             ` Aaron Ecay
  0 siblings, 1 reply; 26+ messages in thread
From: Samuel Wales @ 2013-08-28 19:51 UTC (permalink / raw)
  To: Eric Schulte; +Cc: emacs-orgmode

I think I was right.  Make this line look like this:

     ;; (setq buffer-undo-list ul)

Now it seems to work.  :)

-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  ANYBODY can get it.

Denmark: free Karina Hansen NOW.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-08-28 19:51           ` Samuel Wales
@ 2013-10-28 19:17             ` Aaron Ecay
  2013-10-28 19:45               ` Samuel Wales
  0 siblings, 1 reply; 26+ messages in thread
From: Aaron Ecay @ 2013-10-28 19:17 UTC (permalink / raw)
  To: Samuel Wales, Eric Schulte; +Cc: emacs-orgmode

Hi Sam,

I am sorry for the late reply.  It seems I forgot to apply the patch I
originally sent.  I have just done so.

It seems like what you are talking about below is a different problem,
though.  The ‘org-src-in-org-buffer’ macro is only called when saving or
tangling, not when invoking ‘org-edit-special’ (C-c ').  And at least in
a quick test, I do not see any buffer corruption when saving (C-c ' C-x
C-c ').  Do you?

Thanks,

-- 
Aaron Ecay

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-10-28 19:17             ` Aaron Ecay
@ 2013-10-28 19:45               ` Samuel Wales
  2013-10-28 20:07                 ` Aaron Ecay
  0 siblings, 1 reply; 26+ messages in thread
From: Samuel Wales @ 2013-10-28 19:45 UTC (permalink / raw)
  To: Samuel Wales, Eric Schulte, emacs-orgmode

Hi Aaron,

Below?

If you mean my fix, I don't know why it worked and cannot investigate it.

Samuel

On 10/28/13, Aaron Ecay <aaronecay@gmail.com> wrote:
> Hi Sam,
>
> I am sorry for the late reply.  It seems I forgot to apply the patch I
> originally sent.  I have just done so.
>
> It seems like what you are talking about below is a different problem,
> though.  The ‘org-src-in-org-buffer’ macro is only called when saving or
> tangling, not when invoking ‘org-edit-special’ (C-c ').  And at least in
> a quick test, I do not see any buffer corruption when saving (C-c ' C-x
> C-c ').  Do you?
>
> Thanks,
>
> --
> Aaron Ecay
>


-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  ANYBODY can get it.

Denmark: free Karina Hansen NOW.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-10-28 19:45               ` Samuel Wales
@ 2013-10-28 20:07                 ` Aaron Ecay
  2013-10-28 21:26                   ` Samuel Wales
  0 siblings, 1 reply; 26+ messages in thread
From: Aaron Ecay @ 2013-10-28 20:07 UTC (permalink / raw)
  To: Samuel Wales

2013ko urriak 28an, Samuel Wales-ek idatzi zuen:
> 
> Hi Aaron,
> 
> Below?
> 
> If you mean my fix, I don't know why it worked and cannot investigate it.
> 
> Samuel

Argh, I must have mistakenly deleted the quoted text from my reply; I
did mean the suggestion to comment out the line

(setq buffer-undo-list ul)

But since my original patch worked for you (?), all should be fine now.

-- 
Aaron Ecay

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-10-28 20:07                 ` Aaron Ecay
@ 2013-10-28 21:26                   ` Samuel Wales
  2014-03-10  2:46                     ` Samuel Wales
  0 siblings, 1 reply; 26+ messages in thread
From: Samuel Wales @ 2013-10-28 21:26 UTC (permalink / raw)
  To: Samuel Wales, Eric Schulte, emacs-orgmode

Hi Aaron,

I think, but not sure, that:

  - your original patch had the right idea, and i think it improved
it, but did not fix it
  - my tiny fix seemed to fix it, but i did not test enough
  - i avoid the bug rather than carrying along my patch or yours
  - my impression is that the bug was due to unnecessary undo fanciness
  - there might have been a patch that made it into the repo?

Samuel

On 10/28/13, Aaron Ecay <aaronecay@gmail.com> wrote:
> 2013ko urriak 28an, Samuel Wales-ek idatzi zuen:
>>
>> Hi Aaron,
>>
>> Below?
>>
>> If you mean my fix, I don't know why it worked and cannot investigate it.
>>
>> Samuel
>
> Argh, I must have mistakenly deleted the quoted text from my reply; I
> did mean the suggestion to comment out the line
>
> (setq buffer-undo-list ul)
>
> But since my original patch worked for you (?), all should be fine now.
>
> --
> Aaron Ecay
>


-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  ANYBODY can get it.

Denmark: free Karina Hansen NOW.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2013-10-28 21:26                   ` Samuel Wales
@ 2014-03-10  2:46                     ` Samuel Wales
  2014-03-18 20:48                       ` Bastien
  0 siblings, 1 reply; 26+ messages in thread
From: Samuel Wales @ 2014-03-10  2:46 UTC (permalink / raw)
  To: Samuel Wales, Eric Schulte, emacs-orgmode

as far as i know, my assessment below is correct, but i cannot confirm.

i believe that if undo-related code is ripped out of babel, then undo
will work correctly in the source buffer and in the edit buffer.  i am
not clear on what the purpose of changing undo behavior is?

On 10/28/13, Samuel Wales <samologist@gmail.com> wrote:
> Hi Aaron,
>
> I think, but not sure, that:
>
>   - your original patch had the right idea, and i think it improved
> it, but did not fix it
>   - my tiny fix seemed to fix it, but i did not test enough
>   - i avoid the bug rather than carrying along my patch or yours
>   - my impression is that the bug was due to unnecessary undo fanciness
>   - there might have been a patch that made it into the repo?
>
> Samuel
>
> On 10/28/13, Aaron Ecay <aaronecay@gmail.com> wrote:
>> 2013ko urriak 28an, Samuel Wales-ek idatzi zuen:
>>>
>>> Hi Aaron,
>>>
>>> Below?
>>>
>>> If you mean my fix, I don't know why it worked and cannot investigate
>>> it.
>>>
>>> Samuel
>>
>> Argh, I must have mistakenly deleted the quoted text from my reply; I
>> did mean the suggestion to comment out the line
>>
>> (setq buffer-undo-list ul)
>>
>> But since my original patch worked for you (?), all should be fine now.
>>
>> --
>> Aaron Ecay
>>
>
>
> --
> The Kafka Pandemic: http://thekafkapandemic.blogspot.com
>
> The disease DOES progress.  MANY people have died from it.  ANYBODY can get
> it.
>
> Denmark: free Karina Hansen NOW.
>


-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  ANYBODY can get it.

Denmark: free Karina Hansen NOW.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2014-03-10  2:46                     ` Samuel Wales
@ 2014-03-18 20:48                       ` Bastien
  2014-03-21 21:34                         ` Samuel Wales
  0 siblings, 1 reply; 26+ messages in thread
From: Bastien @ 2014-03-18 20:48 UTC (permalink / raw)
  To: Samuel Wales; +Cc: emacs-orgmode, Eric Schulte

Hi Samuel,

Samuel Wales <samologist@gmail.com> writes:

> as far as i know, my assessment below is correct, but i cannot
> confirm.

I revisited this thread.

> i believe that if undo-related code is ripped out of babel, then undo
> will work correctly in the source buffer and in the edit buffer.
> i am not clear on what the purpose of changing undo behavior is?

Time flies: maybe it was clear before, but for now undo works fine
for me, in the source editing buffer and in the original buffer too.

When I do C-c ' from the *Org Src* buffer, then C-/, the buffer is
restored to its previous state, that is: before the insertion of the
edited source.

Can you restate what the bug is?

Thanks,

-- 
 Bastien

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2014-03-18 20:48                       ` Bastien
@ 2014-03-21 21:34                         ` Samuel Wales
  2014-03-21 22:07                           ` Bastien
  0 siblings, 1 reply; 26+ messages in thread
From: Samuel Wales @ 2014-03-21 21:34 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Eric Schulte

hi bastien,

thanks for revisiting this.

i cannot do thorough bug reports at this time, but there are a few
undo-related issues in org-mode now.  here are a few from memory
[there are others]:

  - occasional buffer or undo-tree corruption from c-c ' [see previous
posts in this thread]
  - disruptive and surprising combining of undo entries when doing
agenda operations where you can't undo properly
  - buffer corruption without c-c '

typical example of last one:

===
* bastien
#+begin_src org
  bastien
  ^bastien
#+end_src
* bastien
===

put point at ^, fill-paragraph, undo.  with undo-tree, at least, the
buffer will be corrupted.

===

often there is no way to fix the buffer corruption.  undo is after all
a way to fix things.  so i'm particularly sensitive to it not working.

if org did not touch undo internal features, then i think undo would
be more trustworthy.  this is what i prefer as a user.

i think not touching undo internals is the safer, more defensive strategy.

===

one thing org sometimes does is try to set buffer-undo-list.  it's
really for speed imo.  i can't think of any reason why org really
needs it.  perhaps i am mistaken and there is a really good reason for
such things, but i suspect it has caused a lot of bugs.

in the case of c-c ', i would prefer having the indentation adding
show up as an undo entry [or whatever would happen if we ripped out
the undo-related setting].  separating the undo of the base and
indirect buffers leads to surprises.  i expect undo to just work in
both places and not to undo things that i did not edit last.

even when there is not a bug per se, when you edit a source block,
there is a gap in the undo record.  like nixon's tape gap during
watergate, it raises questions.  :/

===

to me, undo is a low-level feature that should never be buggy or
surprising.  if it is, then anything that causes those should be
ripped out, even if it means losing a fancy undo-related feature.

for me, the best undo feature of all is reliability.

:]

samuel


On 3/18/14, Bastien <bzg@gnu.org> wrote:
> Hi Samuel,
>
> Samuel Wales <samologist@gmail.com> writes:
>
>> as far as i know, my assessment below is correct, but i cannot
>> confirm.
>
> I revisited this thread.
>
>> i believe that if undo-related code is ripped out of babel, then undo
>> will work correctly in the source buffer and in the edit buffer.
>> i am not clear on what the purpose of changing undo behavior is?
>
> Time flies: maybe it was clear before, but for now undo works fine
> for me, in the source editing buffer and in the original buffer too.
>
> When I do C-c ' from the *Org Src* buffer, then C-/, the buffer is
> restored to its previous state, that is: before the insertion of the
> edited source.
>
> Can you restate what the bug is?
>
> Thanks,
>
> --
>  Bastien
>


-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  And
ANYBODY can get it.

Denmark: free Karina Hansen NOW.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2014-03-21 21:34                         ` Samuel Wales
@ 2014-03-21 22:07                           ` Bastien
  2014-03-21 22:42                             ` Samuel Wales
  0 siblings, 1 reply; 26+ messages in thread
From: Bastien @ 2014-03-21 22:07 UTC (permalink / raw)
  To: Samuel Wales; +Cc: emacs-orgmode, Eric Schulte

Hi Samuel,

Samuel Wales <samologist@gmail.com> writes:

> typical example of last one:
>
> ===
> * bastien
> #+begin_src org
>   bastien
>   ^bastien
> #+end_src
> * bastien
> ===
>
> put point at ^, fill-paragraph, undo.  with undo-tree, at least, the
> buffer will be corrupted.

Not for me.  This has surely to do with undo-tree.

> one thing org sometimes does is try to set buffer-undo-list.  it's
> really for speed imo.  i can't think of any reason why org really
> needs it.  perhaps i am mistaken and there is a really good reason for
> such things, but i suspect it has caused a lot of bugs.

I can safely say this is *never* for speeding things up, it's for
preserving the undo list state.

> in the case of c-c ', i would prefer having the indentation adding
> show up as an undo entry [or whatever would happen if we ripped out
> the undo-related setting].

Unless I misunderstand, the addition of indentation is not manually
done, so it should not be part of the undo list.  

> even when there is not a bug per se, when you edit a source block,
> there is a gap in the undo record.  like nixon's tape gap during
> watergate, it raises questions.  :/

:)

> to me, undo is a low-level feature that should never be buggy or
> surprising.  if it is, then anything that causes those should be
> ripped out, even if it means losing a fancy undo-related feature.

Fully agreed.  Let's raise bugs in this area when you have time
and when the bug can be isolated from other third-part package.
Maybe that's me not being able to reproduce the ones you mention,
but other people can chime in too, if the bug is real.

Thanks!

-- 
 Bastien

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2014-03-21 22:07                           ` Bastien
@ 2014-03-21 22:42                             ` Samuel Wales
  2014-03-21 22:45                               ` Samuel Wales
  2014-03-21 23:40                               ` Bastien
  0 siblings, 2 replies; 26+ messages in thread
From: Samuel Wales @ 2014-03-21 22:42 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Eric Schulte

hi bastien,

On 3/21/14, Bastien <bzg@gnu.org> wrote:
>> buffer will be corrupted.
>
> Not for me.  This has surely to do with undo-tree.

in emacs 23, i get the same bug with built-in undo.

>> one thing org sometimes does is try to set buffer-undo-list.  it's
>> really for speed imo.  i can't think of any reason why org really
>> needs it.  perhaps i am mistaken and there is a really good reason for
>> such things, but i suspect it has caused a lot of bugs.
>
> I can safely say this is *never* for speeding things up, it's for
> preserving the undo list state.

no, you misunderstand.  there is no sufficiently good reason for org
code to ever set such an internal variable in any buffer that the user
edits to my knowledge.

the only reason that makes sense to me is to turn off undo by setting
it to t in buffers that the user does not edit [such as a hidden
buffer], so that you avoid possible overhead.  that /is/ a potential
speed issue, even if it is mediated by memory.

thus, in practice, the variable is only really best used for speed imo.

> Unless I misunderstand, the addition of indentation is not manually
> done, so it should not be part of the undo list.

you understand here i think, but we disagree.  at present org
manipulates an undo variable that should not be manipulated, and the
result at best is that the user does this:

  c-c '
  edit
  c-c '
  edit
  undo
  undo OOPS the source edit is skipped over what just happened?
  c-c '
  undo OOPS the undo changes are all gone where did they go?

and at worst there is buffer corruption.  so i would far prefer to
have the indentation adding be NOT specially worked around by changing
the undo variable.

it is much better to not skip an entire set of changes + no bugs than
"the user will never want to see the indentation adding so let's
pretend it doesn't exist".

it might be better to have no indentation by default, but that's another issue.

>> even when there is not a bug per se, when you edit a source block,
>> there is a gap in the undo record.  like nixon's tape gap during
>> watergate, it raises questions.  :/
>
> :)

principle of least surprise is key here.

>> to me, undo is a low-level feature that should never be buggy or
>> surprising.  if it is, then anything that causes those should be
>> ripped out, even if it means losing a fancy undo-related feature.
>
> Fully agreed.  Let's raise bugs in this area when you have time
> and when the bug can be isolated from other third-part package.

i haven't tried with -Q yet but i don't think it's an undo-tree bug.

i think fixing each bug as it comes up is more error-prone for the
future than stopping the changing of undo-tree-list.

in most cases, i think the undo list manipulation should never happen
in the first place.  it just causes too many problems.

samuel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2014-03-21 22:42                             ` Samuel Wales
@ 2014-03-21 22:45                               ` Samuel Wales
  2014-03-21 23:40                               ` Bastien
  1 sibling, 0 replies; 26+ messages in thread
From: Samuel Wales @ 2014-03-21 22:45 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Eric Schulte

s/most cases/all cases that i am aware of/

On 3/21/14, Samuel Wales <samologist@gmail.com> wrote:
> in most cases, i think the undo list manipulation should never happen
> in the first place.  it just causes too many problems.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2014-03-21 22:42                             ` Samuel Wales
  2014-03-21 22:45                               ` Samuel Wales
@ 2014-03-21 23:40                               ` Bastien
  2014-03-22  0:11                                 ` Samuel Wales
  1 sibling, 1 reply; 26+ messages in thread
From: Bastien @ 2014-03-21 23:40 UTC (permalink / raw)
  To: Samuel Wales; +Cc: emacs-orgmode, Eric Schulte

Hi Samuel,

Samuel Wales <samologist@gmail.com> writes:

>   c-c '
>   edit
>   c-c '
>   edit
>   undo
>   undo OOPS the source edit is skipped over what just happened?
>   c-c '
>   undo OOPS the undo changes are all gone where did they go?

The changes happen in different buffers, there is no reason to
expect undo to let you undo changes you made from another buffer.
Or is it something something you see elsewhere in Emacs?

Agenda undo is different: manipulations in the agenda modify
the source buffer, so undo in the agenda should undo in the
source buffer.

In *Org Src*, manipulation are reflected in the source buffer
only when you save the *Org Src* buffer or when you exit it.
When you save, you can undo in the *Org Src* buffer, and changes
will be be reflected in the source buffer at the time you exit
the temporary buffer.  When you exit the buffer, there is no
undo problem left.

> and at worst there is buffer corruption.

Sorry to ask it again, but please let me know about a reprocible
recipe, I could not reproduce the problem you have.

> it is much better to not skip an entire set of changes + no bugs than
> "the user will never want to see the indentation adding so let's
> pretend it doesn't exist".

That's not my reasoning, which is "undo applies to manual changes.

-- 
 Bastien

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2014-03-21 23:40                               ` Bastien
@ 2014-03-22  0:11                                 ` Samuel Wales
  2014-03-22  0:18                                   ` Bastien
  0 siblings, 1 reply; 26+ messages in thread
From: Samuel Wales @ 2014-03-22  0:11 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Eric Schulte

On 3/21/14, Bastien <bzg@gnu.org> wrote:
> The changes happen in different buffers, there is no reason to
> expect undo to let you undo changes you made from another buffer.

you might be surprised to find that i disagree.  :]

let's concentrate on just one aspect of this.

i believe that most users expect that all changes to a buffer are
reflected in the undo list.

if i am in mybuffer.org, and i make these changes:

  a
  b
  c

and i undo, i expect c b a.

i do NOT expect c a.  that is disconcerting and surprising.

in other words, if b consists of editing a source block, then i
/still/ expect b to be undone, because the contents of that source
block are part of mybuffer.org.

those contents are plain text in mybuffer.org.

perhaps you think of an org document as being a framework inside of
which is foreign material that should be skipped over in undo?  i do
not.  i think of it as plain text that should be undoable.

this is just one aspect of it.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2014-03-22  0:11                                 ` Samuel Wales
@ 2014-03-22  0:18                                   ` Bastien
  2014-03-22  0:37                                     ` Samuel Wales
  0 siblings, 1 reply; 26+ messages in thread
From: Bastien @ 2014-03-22  0:18 UTC (permalink / raw)
  To: Samuel Wales; +Cc: emacs-orgmode, Eric Schulte

Samuel Wales <samologist@gmail.com> writes:

> in other words, if b consists of editing a source block, then i
> /still/ expect b to be undone, because the contents of that source
> block are part of mybuffer.org.

I think we are miscommunicating...

I simply say that I cannot reproduce the bug: for me, when b
consists of editing a source block, b can be undone.

At this point, I think the only way out is to have someone
else reproduce the bug too, otherwise we may miscommunicate
for way too long :)

-- 
 Bastien

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2014-03-22  0:18                                   ` Bastien
@ 2014-03-22  0:37                                     ` Samuel Wales
  2014-03-22  0:44                                       ` Samuel Wales
  0 siblings, 1 reply; 26+ messages in thread
From: Samuel Wales @ 2014-03-22  0:37 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Eric Schulte

bug #3:

insert a source block
edit after it
c-c ' to edit
edit
c-c ' to go back to mybuffer
edit before the source block
undo
undo

you should notice that the source block is not restored.

note that this is not the same buffer corruption issue i reported previously.

in that one, you insert a source block, fill it, and undo.  then the
next header line gets corrupted:

bug #2:

* header
#+begin_src org
  asdf
  asdf
#+end_src
* header2

put point inside the source block, fill, then undo.  you will see
buffer corruption after the source block.  the exact corruption
varies.  sometimes it is removal of the #.  other times it is removal
of * in 1 or 2 headlines below.

what makes it much worse is that you cannot undo to fix the buffer
corruption.  you have to revert the buffer.

there are others.  i believe the reason is that org tries to get fancy
with undo.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2014-03-22  0:37                                     ` Samuel Wales
@ 2014-03-22  0:44                                       ` Samuel Wales
  2014-03-22  8:43                                         ` Bastien
  0 siblings, 1 reply; 26+ messages in thread
From: Samuel Wales @ 2014-03-22  0:44 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Eric Schulte

here is the fix for bug #3:

change this line in org-src.el

      (let ((buffer-undo-list t))

to:

      ;; don't change my undo list here, please
      (progn

this fixes the problem.  i really don't think this kind of fancy undo
list manipulation is the right thing to do in org.


On 3/21/14, Samuel Wales <samologist@gmail.com> wrote:
> bug #3:
>
> insert a source block
> edit after it
> c-c ' to edit
> edit
> c-c ' to go back to mybuffer
> edit before the source block
> undo
> undo
>
> you should notice that the source block is not restored.
>
> note that this is not the same buffer corruption issue i reported
> previously.
>
> in that one, you insert a source block, fill it, and undo.  then the
> next header line gets corrupted:
>
> bug #2:
>
> * header
> #+begin_src org
>   asdf
>   asdf
> #+end_src
> * header2
>
> put point inside the source block, fill, then undo.  you will see
> buffer corruption after the source block.  the exact corruption
> varies.  sometimes it is removal of the #.  other times it is removal
> of * in 1 or 2 headlines below.
>
> what makes it much worse is that you cannot undo to fix the buffer
> corruption.  you have to revert the buffer.
>
> there are others.  i believe the reason is that org tries to get fancy
> with undo.
>


-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  And
ANYBODY can get it.

Denmark: free Karina Hansen NOW.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2014-03-22  0:44                                       ` Samuel Wales
@ 2014-03-22  8:43                                         ` Bastien
  2014-03-22  8:57                                           ` Samuel Wales
  0 siblings, 1 reply; 26+ messages in thread
From: Bastien @ 2014-03-22  8:43 UTC (permalink / raw)
  To: Samuel Wales; +Cc: emacs-orgmode, Eric Schulte

Samuel Wales <samologist@gmail.com> writes:

> here is the fix for bug #3:

There was a fix for this in master by Aaron.
I assumed you were reporting errors on master.
I've now cherry-pick the fix to maint, since
it's a fix.

If you find other problems, please first test
the master branch to check if they have been
fixed there.

Thanks,

-- 
 Bastien

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [bug] [babel] babel corrupts undo history
  2014-03-22  8:43                                         ` Bastien
@ 2014-03-22  8:57                                           ` Samuel Wales
  0 siblings, 0 replies; 26+ messages in thread
From: Samuel Wales @ 2014-03-22  8:57 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Eric Schulte

aha.  yes, that's it.  thank you.

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2014-03-22  8:57 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-28  1:13 [bug] [babel] babel corrupts undo history Samuel Wales
2013-08-28 14:42 ` Eric Schulte
2013-08-28 16:03   ` Aaron Ecay
2013-08-28 18:52     ` Eric Schulte
2013-08-28 19:41       ` Samuel Wales
2013-08-28 19:43         ` Samuel Wales
2013-08-28 19:51           ` Samuel Wales
2013-10-28 19:17             ` Aaron Ecay
2013-10-28 19:45               ` Samuel Wales
2013-10-28 20:07                 ` Aaron Ecay
2013-10-28 21:26                   ` Samuel Wales
2014-03-10  2:46                     ` Samuel Wales
2014-03-18 20:48                       ` Bastien
2014-03-21 21:34                         ` Samuel Wales
2014-03-21 22:07                           ` Bastien
2014-03-21 22:42                             ` Samuel Wales
2014-03-21 22:45                               ` Samuel Wales
2014-03-21 23:40                               ` Bastien
2014-03-22  0:11                                 ` Samuel Wales
2014-03-22  0:18                                   ` Bastien
2014-03-22  0:37                                     ` Samuel Wales
2014-03-22  0:44                                       ` Samuel Wales
2014-03-22  8:43                                         ` Bastien
2014-03-22  8:57                                           ` Samuel Wales
2013-08-28 17:08   ` Samuel Wales
2013-08-28 17:18     ` Samuel Wales

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