Re: Question on undo: Remove a finishUndo() after a recordUndo() ?

2020-02-02 Thread Scott Kostyshak
On Sat, Feb 01, 2020 at 08:41:12PM +0100, Jean-Marc Lasgouttes wrote:
> Le 01/02/2020 à 18:39, Scott Kostyshak a écrit :
> > What would the consequences be of the attached patch?
> > 
> >  From what I understand, finishUndo() is meant for when there have been
> > som undos of type INSERT or DELETE recorded, but PARAGRAPH_MOVE_DOWN
> > itself does not record any INSERTs or DELETEs. What would happen if
> > the finishUndo() is removed?
> > 
> > Is the cur.finishUndo() here meant for the case where a user is typing
> > and then executes pargraph-move-down and then continues typing? But
> > shouldn't a cur.recordUndo() end a sequence of INSERT type undos anyway?
> 
> I think the patch is correct.

Thanks for taking a look. It's in at 05f24940.

> finishUndo is only useful after a recordUndo
> of type INSERT or DELETE. One place where we use it is when moving the
> cursor around. Then we do not record any und, but we do not want the next
> inserted characters to be lumped with the ones which were inserted before
> the cursor movement.
>
> Actually, I suspect that finishUndo could be removed and replaced with a
> test that the cursor is still at the same position. But I am not 100% sure
> that it would work.

Makes sense. Cursor movement is a good example.

Scott


signature.asc
Description: PGP signature
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: Question on undo: Remove a finishUndo() after a recordUndo() ?

2020-02-01 Thread Jean-Marc Lasgouttes

Le 01/02/2020 à 18:39, Scott Kostyshak a écrit :

What would the consequences be of the attached patch?

 From what I understand, finishUndo() is meant for when there have been
som undos of type INSERT or DELETE recorded, but PARAGRAPH_MOVE_DOWN
itself does not record any INSERTs or DELETEs. What would happen if
the finishUndo() is removed?

Is the cur.finishUndo() here meant for the case where a user is typing
and then executes pargraph-move-down and then continues typing? But
shouldn't a cur.recordUndo() end a sequence of INSERT type undos anyway?


I think the patch is correct. finishUndo is only useful after a 
recordUndo of type INSERT or DELETE. One place where we use it is when 
moving the cursor around. Then we do not record any und, but we do not 
want the next inserted characters to be lumped with the ones which were 
inserted before the cursor movement.


Actually, I suspect that finishUndo could be removed and replaced with a 
test that the cursor is still at the same position. But I am not 100% 
sure that it would work.


JMarc
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Question on undo: Remove a finishUndo() after a recordUndo() ?

2020-02-01 Thread Scott Kostyshak
What would the consequences be of the attached patch?

From what I understand, finishUndo() is meant for when there have been
som undos of type INSERT or DELETE recorded, but PARAGRAPH_MOVE_DOWN
itself does not record any INSERTs or DELETEs. What would happen if
the finishUndo() is removed?

Is the cur.finishUndo() here meant for the case where a user is typing
and then executes pargraph-move-down and then continues typing? But
shouldn't a cur.recordUndo() end a sequence of INSERT type undos anyway?

Scott
From 47d42b2c4808856022ee23198b0056abee803636 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak 
Date: Sat, 1 Feb 2020 12:32:30 -0500
Subject: [PATCH] Remove a finishUndo() after a recordUndo()

---
 src/Text3.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/Text3.cpp b/src/Text3.cpp
index 8643411da1..f5444e7baa 100644
--- a/src/Text3.cpp
+++ b/src/Text3.cpp
@@ -673,7 +673,6 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
 	case LFUN_PARAGRAPH_MOVE_DOWN: {
 		pit_type const pit = cur.pit();
 		cur.recordUndo(pit, pit + 1);
-		cur.finishUndo();
 		pars_.swap(pit, pit + 1);
 		needsUpdate = true;
 		cur.forceBufferUpdate();
-- 
2.20.1



signature.asc
Description: PGP signature
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel