I finally had a look at your answer, but I feel more confused than before unfortunately ;-)
Andre Poenitz wrote: > On Sun, Nov 21, 2004 at 03:37:35PM +0100, > [EMAIL PROTECTED] wrote: >> Andre, undo continues to have problems despite your last patch. > > That's bad news, especially since my next free slot on a private > 'development' machine will be in two weeks' time. I don't even have a > working LyX here to check. > >> file -> new write 3 of 4 short paragraphs, select all, delete, undo, >> undo -> normally crash >> >> >> There are some things of that patch I don't understand. >> >> In textUndoOrRedo 1) what does the bv.cursor has to do with anything? > > It should be needed to e.g. give the target for the Undo struct that will > be pushed on the redo stack. [I don't understand what you mean by 'target'] So we store a cursor position with the undo information, in addition to undo.cell which points to the inset of the paragraph range. And for some reason, we need this position to be *inside* the undo range. (or at least, in the same inset of the undo paragraph range). Why? This is particularly broken in the current undo/redo scheme: suppose you do a minor change inside some small inset, then go with the cursor to the end of the document and do an 'undo': then the full document would be recorded as 'redo' information? This would be the result of the "intersection" above. [still besides being broken, the above don't even work with current cvs] >> 2) You seem to assume that the undo cell must be inside the cursor path? >> (you just compare them by size) > > I think this is needed for the scheme to work. Now that you mention > it, it could well be that some code using recordUndo() is not aware of > this and stores a chunk that's too small. > The requirement right now is that if you are going to change some > buffer contents between positions given by dit1 and dit2, you need to > use recordUndo() to create some Undo struct containing paragraphs > from a cell pointed to by some dit0 such that dit1 and dit2 both contain > dit0 - and recordUndo() needs to be called before the change is made. > I.e if dit1 is (inset1_0, idx1_0, par1_0, ... inset1_n, idx1_n, par1_n) > and if dit2 is (inset2_0, idx2_0, par2_0, ... inset2_m, idx2_m, par2_m) > dit0 should be something like (inset0_0, ...inset0_k) with k sucht that > inset1_i == inset2_i (== inset0_i) \forall 0\leq i\leq k. > Now I suppose I assumed idx1_k == idx2_k in this situation which does > not have to be true for tabulars and most of mathed. Well, this could be > brushed over by using k = k - 1 in this case, but this wastes some > memory. Not much in the math case (cells are small there in general) but > potentially quite a bit in tabulars... OTOH this might be good enough > for 1.4.0 as we aren't often modifying more than one tabular cell at > a time. > > Ok, so lets really suppose we have idx1_k == idx2_k because some kind > soul put k = k - 1 in the right place or fixed this issue properly. > Than the Undo struct pushed on the undo stack will contain par1_k > through par2_k, and dit0 as undo.cell. undo.cur will equal dit1, even if > k != n, so undo.cur will always contain undo.cell - and that's the > assumption you were asking about. But this is ok if dit1 and dit2 are two arbitrary iterators between which we want to store the undo information (this is rarely the case -- normally one already starts with two iterators in the same slice, but I can understand that we can have an use for this). In this case are we talking about dit2 being bv.cursor for some reason? Why? > -- > There are a few convienience functions for the common case that > everything happens in the cursor paragraph or near to it, but there are > probably cases missing where undo creation needs to operate not on the > depth of the current cursor because the operation will change cursor > depth by e.g. leaving an inset. I still don't understand why do we want to keep the undo.cursor inside the undo.cell. Alfredo
