Re: Various change tracking issues
Lars Gullik Bjønnes wrote: > I'd like Martin to test patch one, and commit that if it seems ok. I'll commit it in a moment. > Ad. patch2 I'd like to wait for 1.4.1 with that one. OK. Jürgen
Re: Various change tracking issues
On Mon, Jan 16, 2006 at 01:25:36PM +0100, Lars Gullik Bjønnes wrote: > Juergen Spitzmueller <[EMAIL PROTECTED]> writes: > > | Juergen Spitzmueller wrote: > | > > > May I have the row-painter fix as a separate patch, please? > | > Attached are the two fixes in separate patches. > | > | Comments? > > I'd like Martin to test patch one, and commit that if it seems ok. I tried that one (in a tree containing all kinds of other stuff), exercised the User Guide, and could not see a problem with it. (I wouldn't expect a problem either; all it does is force a row refresh in some situations where none would have been forced without the patch.) - Martin pgpnQnhrq2DYp.pgp Description: PGP signature
Re: Various change tracking issues
Juergen Spitzmueller <[EMAIL PROTECTED]> writes: | Juergen Spitzmueller wrote: | > > > May I have the row-painter fix as a separate patch, please? | > Attached are the two fixes in separate patches. | | Comments? I'd like Martin to test patch one, and commit that if it seems ok. Ad. patch2 I'd like to wait for 1.4.1 with that one. -- Lgb
Re: Various change tracking issues
Juergen Spitzmueller wrote: > > > May I have the row-painter fix as a separate patch, please? > Attached are the two fixes in separate patches. Comments? Jürgen
Re: Various change tracking issues
Can anyone confirm http://bugzilla.lyx.org/show_bug.cgi?id=2214 ? I have checked that dvipost can overstrike formula and this is a latex-generating problem of lyx. Bo
Re: Various change tracking issues
Juergen Spitzmueller wrote:
> Lars Gullik Bjønnes wrote:
> > It changes some details that I am not sure will not have side-effects.
>
> I cannot exclude that in last consequence, of course, even if I've tested
> lots of possible cases. Maybe someone else could give it a hard test.
>
> > | > I think this should wait so that it can get some more testing first.
> > |
> > | Your call of course.
> >
> > May I have the row-painter fix as a separate patch, please?
>
> Yes, as soon as I'm on my home machine again (not before Thursday).
Attached are the two fixes in separate patches.
2185-1.diff: rowpainter fix: assures that the row a cursor is in is always
being redrawn, also if the cursor is at the last position of this row.
Without this patch, the screen is not updated if the last char of a row is
deleted (with DELETE) in change tracking mode. You can reproduce that easily
if you apply patch 2 without patch 1.
2185-2: the actual fix of DELETE in change tracking mode: moves the cursor one
position right after DELETE iff the char to the right of the cursor is marked
deleted. This happens in two cases: either this char has just been deleted or
it has been deleted previously. In both cases, the cursor should be moved
onwards. This only happens for single character deletions. Deletions of
selections already work correctly.
The cur.pos() < par.size() check is necessary because lookupChange asserts
that.
Jürgen
Index: rowpainter.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/rowpainter.C,v
retrieving revision 1.162
diff -p -u -r1.162 rowpainter.C
--- rowpainter.C 1 Jan 2006 23:06:23 - 1.162
+++ rowpainter.C 12 Jan 2006 12:46:41 -
@@ -735,7 +735,7 @@ bool isCursorOnRow(PainterInfo & pi, pit
for (lyx::size_type d = 0; d < cur.depth(); d++)
if (cur[d].pit() == pit
&& cur[d].pos() >= rit->pos()
- && cur[d].pos() < rit->endpos())
+ && cur[d].pos() <= rit->endpos())
return true;
return false;
}
Index: text.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/text.C,v
retrieving revision 1.639
diff -p -u -r1.639 text.C
--- text.C 19 Dec 2005 12:30:33 - 1.639
+++ text.C 12 Jan 2006 12:47:01 -
@@ -1543,6 +1543,10 @@ bool LyXText::Delete(LCursor & cur)
recordUndo(cur, Undo::DELETE, cur.pit());
setCursorIntern(cur, cur.pit(), cur.pos() + 1, false, cur.boundary());
needsUpdate = backspace(cur);
+ Paragraph & par = cur.paragraph();
+ if (cur.pos() < par.size()
+ && par.lookupChange(cur.pos()) == Change::DELETED)
+ cur.posRight();
} else if (cur.pit() != cur.lastpit()) {
LCursor scur = cur;
Re: Various change tracking issues
Bo Peng wrote: Could you please add it by yourself (http://bugzilla.lyx.org)? I feel a little bit tired... :-) Added, but I could not find your bug reports (in list all unconfirmed bugs). Check for "NEW" bugs (I confirmed them). Or check for the latest bugs. Or for component "changes"... Michael
Re: Various change tracking issues
Bo Peng wrote: CAN NOT ACCEPT THE FIRST CHANGE (?) 1. open a lyx document 2. type in a, newline, b, newline 3. turn on change tracking 4. add change 1 after line b 5, add change 2 after line a ( get a change 2 b change 1 ) 6. Go to the beginning of the document, accept change, lyx will go to change 1 and you can not go to change 2 use 'next change'. I notice that the order of change1, 2, does not matter. Lyx simple does not accept the first change. Added to bugzilla. Actually, if you think it is a bug, there is another one: Dvipost does not overstrick formula, and figures (try remove a formula). This makes output unclear when, for example, I remove $x^2$ and add $x$. The output would look like $x^2x$. Could you please add it by yourself (http://bugzilla.lyx.org)? I feel a little bit tired... :-) Michael
Re: Various change tracking issues
Bo Peng wrote: Another one (hope it has not been reported) CHANGE OF FORMULA IS NOT TRACED 1. open a lyx document 2. insert $x^2$ 3. turn on change tracking 4. go into the formula and remove ^2 5. $x$ is not marked as changed. Added to bugzilla. Shall we advertise change-tracking in 1.4.0 as experimental? I understand 1.4.0 will be released soon so most of these bugs will not be fixed in time. I agree that change tracking is not in a good state. There are many restrictions, in particular the inability to add/remove paragraph breaks. I hope that Lars accepts Jürgen's recent patch concerning "delete". It makes change tracking a LITTLE bit more convenient at least. Michael
Re: Various change tracking issues
Another one (hope it has not been reported) CHANGE OF FORMULA IS NOT TRACED 1. open a lyx document 2. insert $x^2$ 3. turn on change tracking 4. go into the formula and remove ^2 5. $x$ is not marked as changed. Shall we advertise change-tracking in 1.4.0 as experimental? I understand 1.4.0 will be released soon so most of these bugs will not be fixed in time. Bo
Re: Various change tracking issues
Another change tracking bug: CAN NOT ACCEPT THE FIRST CHANGE (?) 1. open a lyx document 2. type in a, newline, b, newline 3. turn on change tracking 4. add change 1 after line b 5, add change 2 after line a ( get a change 2 b change 1 ) 6. Go to the beginning of the document, accept change, lyx will go to change 1 and you can not go to change 2 use 'next change'. I notice that the order of change1, 2, does not matter. Lyx simple does not accept the first change. Bo
Re: Various change tracking issues
> >PASTED TEXT IS NOT CONSIDERED AS CHANGE > > > Very strange. I have added a bugzilla report... This is not very strange in that the *not-changed property' of copied lyx text is somehow preserved. If I paste from outside of lyx using my middle button, the added text is considered as change. Bo
Re: Various change tracking issues
Bo Peng wrote: I am not sure whether or not this has been discussed, but I would like to add PASTED TEXT IS NOT CONSIDERED AS CHANGE 1. new doc 2. write something, copy 3. turn on change tracking 4. paste Very strange. I have added a bugzilla report... Michael
Re: Various change tracking issues
> I have produced a set of test cases. I thought it might be better to > send them to the mailing list before adding a couple of bugzilla entries. I am not sure whether or not this has been discussed, but I would like to add PASTED TEXT IS NOT CONSIDERED AS CHANGE 1. new doc 2. write something, copy 3. turn on change tracking 4. paste Cheers, Bo
Re: Various change tracking issues
Juergen Spitzmueller wrote: It is as simple as the attached. I've tested pretty intensive, some further testing would be welcome, though. Works like a charm! Lars? Please also note my new bug reports #2202 and #2203. The first one is a bit critical because LyX crashes (but this has nothing to do with your patch). Michael
Re: Various change tracking issues
Lars Gullik Bjønnes wrote: > It changes some details that I am not sure will not have side-effects. I cannot exclude that in last consequence, of course, even if I've tested lots of possible cases. Maybe someone else could give it a hard test. > | > I think this should wait so that it can get some more testing first. > | > | Your call of course. > > May I have the row-painter fix as a separate patch, please? Yes, as soon as I'm on my home machine again (not before Thursday). Jürgen
Re: Various change tracking issues
On Tue, Jan 10, 2006 at 12:13:31PM +0100, Juergen Spitzmueller wrote: > Martin Vermeer wrote: > > > Now I am not sure anymore. Please test your patch also without the > > rowpainter change and see if it works. endpos is *one past the end*, > > i.e., the cursor is on the right side of all of the row's characters. It > > shouldn't do any change tracking marking there. > > It doesn't work without the patch, honestly. > Consider that DELETE is implemented as > - cursor right > - backspace OK then, as long as it got tested... the only risk of getting it wrong is doing a little more screen refresh than is strictly needed. Erring on the side of caution ;-) - Martin
Re: Various change tracking issues
Martin Vermeer wrote: > Now I am not sure anymore. Please test your patch also without the > rowpainter change and see if it works. endpos is *one past the end*, > i.e., the cursor is on the right side of all of the row's characters. It > shouldn't do any change tracking marking there. It doesn't work without the patch, honestly. Consider that DELETE is implemented as - cursor right - backspace Jürgen
Re: Various change tracking issues
On Tue, 2006-01-10 at 10:05 +0100, Juergen Spitzmueller wrote: > Jean-Marc Lasgouttes wrote: > > > Juergen> It is as simple as the attached. I've tested pretty > > Juergen> intensive, some further testing would be welcome, though. > > > > What does the rowpainter change do? > > Martin's latest rowpainter change assures that rows are always repainted > because of change tracking. However, he has not considered that last > position in a row (endpos). The document was not correctly repainted when > you deleted the last character in a row (with DELETE). The patch fixes > this. > > Jürgen Jürgen, Now I am not sure anymore. Please test your patch also without the rowpainter change and see if it works. endpos is *one past the end*, i.e., the cursor is on the right side of all of the row's characters. It shouldn't do any change tracking marking there. - Martin signature.asc Description: This is a digitally signed message part
Re: Various change tracking issues
Juergen Spitzmueller <[EMAIL PROTECTED]> writes: | Lars Gullik Bjønnes wrote: | | > I must admit that I find the patch not simple enough :-) | | Sure? I find it pretty straightforward. It changes some details that I am not sure will not have side-effects. | > I think this should wait so that it can get some more testing first. | | Your call of course. May I have the row-painter fix as a separate patch, please? -- Lgb
Re: Various change tracking issues
Lars Gullik Bjønnes wrote: > I must admit that I find the patch not simple enough :-) Sure? I find it pretty straightforward. > I think this should wait so that it can get some more testing first. Your call of course. Jürgen
Re: Various change tracking issues
Jean-Marc Lasgouttes wrote: > Juergen> It is as simple as the attached. I've tested pretty > Juergen> intensive, some further testing would be welcome, though. > > What does the rowpainter change do? Martin's latest rowpainter change assures that rows are always repainted because of change tracking. However, he has not considered that last position in a row (endpos). The document was not correctly repainted when you deleted the last character in a row (with DELETE). The patch fixes this. Jürgen
Re: Various change tracking issues
On Mon, Jan 09, 2006 at 04:51:41PM +0100, Jean-Marc Lasgouttes wrote: > > "Juergen" == Juergen Spitzmueller <[EMAIL PROTECTED]> writes: > > Juergen> Lars Gullik Bjønnes wrote: > >> | DELETE KEY DOES NOT MOVE THE CURSOR TO THE NEXT CHARACTER > >> > >> If this has a simple fix it should be done, otherwise it should > >> wait. > > Juergen> It is as simple as the attached. I've tested pretty > Juergen> intensive, some further testing would be welcome, though. > > What does the rowpainter change do? > > JMarc I think it is probably valid. The routine tests whether the cursor is on the current row, and with this patch it includes the case where it is at the end of the row. I remember staring at this code line myself... - Martin pgpBXrhX5h4w1.pgp Description: PGP signature
Re: Various change tracking issues
Jean-Marc Lasgouttes <[EMAIL PROTECTED]> writes: | > "Juergen" == Juergen Spitzmueller <[EMAIL PROTECTED]> writes: | | Juergen> Lars Gullik Bjønnes wrote: | >> | DELETE KEY DOES NOT MOVE THE CURSOR TO THE NEXT CHARACTER | >> | >> If this has a simple fix it should be done, otherwise it should | >> wait. | | Juergen> It is as simple as the attached. I've tested pretty | Juergen> intensive, some further testing would be welcome, though. | | What does the rowpainter change do? I must admit that I find the patch not simple enough :-) I think this should wait so that it can get some more testing first. -- Lgb
Re: Various change tracking issues
> "Juergen" == Juergen Spitzmueller <[EMAIL PROTECTED]> writes: Juergen> Lars Gullik Bjønnes wrote: >> | DELETE KEY DOES NOT MOVE THE CURSOR TO THE NEXT CHARACTER >> >> If this has a simple fix it should be done, otherwise it should >> wait. Juergen> It is as simple as the attached. I've tested pretty Juergen> intensive, some further testing would be welcome, though. What does the rowpainter change do? JMarc
Re: Various change tracking issues
Lars Gullik Bjønnes wrote:
> | DELETE KEY DOES NOT MOVE THE CURSOR TO THE NEXT CHARACTER
>
> If this has a simple fix it should be done, otherwise it should wait.
It is as simple as the attached. I've tested pretty intensive, some further
testing would be welcome, though.
Jürgen
Index: src/rowpainter.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/rowpainter.C,v
retrieving revision 1.162
diff -p -u -r1.162 rowpainter.C
--- src/rowpainter.C 1 Jan 2006 23:06:23 - 1.162
+++ src/rowpainter.C 9 Jan 2006 14:52:51 -
@@ -735,7 +735,7 @@ bool isCursorOnRow(PainterInfo & pi, pit
for (lyx::size_type d = 0; d < cur.depth(); d++)
if (cur[d].pit() == pit
&& cur[d].pos() >= rit->pos()
- && cur[d].pos() < rit->endpos())
+ && cur[d].pos() <= rit->endpos())
return true;
return false;
}
Index: src/text.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/text.C,v
retrieving revision 1.639
diff -p -u -r1.639 text.C
--- src/text.C 19 Dec 2005 12:30:33 - 1.639
+++ src/text.C 9 Jan 2006 14:52:55 -
@@ -1543,6 +1543,10 @@ bool LyXText::Delete(LCursor & cur)
recordUndo(cur, Undo::DELETE, cur.pit());
setCursorIntern(cur, cur.pit(), cur.pos() + 1, false, cur.boundary());
needsUpdate = backspace(cur);
+ Paragraph & par = cur.paragraph();
+ if (cur.pos() < par.size()
+ && par.lookupChange(cur.pos()) == Change::DELETED)
+ cur.posRight();
} else if (cur.pit() != cur.lastpit()) {
LCursor scur = cur;
Re: Various change tracking issues
On Mon, Jan 02, 2006 at 01:10:00AM +0100, Jean-Marc Lasgouttes wrote: > | What is this a response to? > > >Your 'let's always repaint the row >with the cursor in it' patch I > >belive. > > Right. My palm is not very good at > quoting messages, sorry. > > JMarc > > -- > Lgb For some reason mutt threads it wrong. Perhaps your Palm isn't good at adding threading info either :-/ - Martin pgpc39NZI6YdV.pgp Description: PGP signature
Re: Various change tracking issues
| What is this a response to? >Your 'let's always repaint the row >with the cursor in it' patch I >belive. Right. My palm is not very good at quoting messages, sorry. JMarc -- Lgb
Re: Various change tracking issues
On Sun, Jan 01, 2006 at 08:29:23PM +0100, Lars Gullik Bjønnes wrote: > Martin Vermeer <[EMAIL PROTECTED]> writes: > > | On Sun, Jan 01, 2006 at 02:17:00PM +0100, [EMAIL PROTECTED] wrote: > | > I cannot test it but it looks much > | > better. I'd say 'apply it'. > | > > | > JMarc > | > | What is this a response to? > > Your 'let's always repaint the row with the cursor in it' patch I > belive. Thought so, but wasn't sure. Committed now. - Martin pgpgf84eKTKSP.pgp Description: PGP signature
Re: Various change tracking issues
Martin Vermeer <[EMAIL PROTECTED]> writes: | On Sun, Jan 01, 2006 at 02:17:00PM +0100, [EMAIL PROTECTED] wrote: | > I cannot test it but it looks much | > better. I'd say 'apply it'. | > | > JMarc | | What is this a response to? Your 'let's always repaint the row with the cursor in it' patch I belive. -- Lgb
Re: Various change tracking issues
On Sun, Jan 01, 2006 at 02:17:00PM +0100, [EMAIL PROTECTED] wrote: > I cannot test it but it looks much > better. I'd say 'apply it'. > > JMarc What is this a response to? - Martin pgpHQabjkkkCG.pgp Description: PGP signature
Re: Various change tracking issues
On Sun, Jan 01, 2006 at 04:50:42PM +0100, Michael Gerz wrote: > Martin Vermeer wrote: > > >If inserting newlines directly _were_ allowed, they would not be rolled > >back together with the tracked changes to the surrounding characters. In > >that sense change tracking would be inconsistent. > > > >Cutting and pasting selections subvert this scheme. > > > > > In other words: If inserting newlines were allowed, they would not be > tracked. Not nice but better than disallowing newlines _at all_ in > change track mode. > > Do you know whether the range specifications would be disturbed by new > lines/pars? I think this is the ultimate question, in particular if > users are "creative". If the answer is no, we should allow new lines > directly. I think the answer is yes, and even that it would be straightforward to add registering code for newlines (pos = par.size()) to breakParagraph and backspace. (Perhaps some changes to cut & paste needed too?) But that's for 1.4.1. - Martin pgpU6ClhuaQyu.pgp Description: PGP signature
Re: Various change tracking issues
Martin Vermeer wrote: If inserting newlines directly _were_ allowed, they would not be rolled back together with the tracked changes to the surrounding characters. In that sense change tracking would be inconsistent. Cutting and pasting selections subvert this scheme. In other words: If inserting newlines were allowed, they would not be tracked. Not nice but better than disallowing newlines _at all_ in change track mode. Do you know whether the range specifications would be disturbed by new lines/pars? I think this is the ultimate question, in particular if users are "creative". If the answer is no, we should allow new lines directly. Michael
Re: Various change tracking issues
On Sun, Jan 01, 2006 at 03:40:34PM +0100, Michael Gerz wrote:
> Martin Vermeer wrote:
>
> >The code is here (text.C):
> >
> > 1026 void LyXText::breakParagraph(LCursor & cur, bool keep_layout)
> > 1027 {
> > 1028 BOOST_ASSERT(this == cur.text());
> > 1029 // allow only if at start or end, or all previous is new
> > text
> > 1030 Paragraph & cpar = cur.paragraph();
> > 1031 pit_type cpit = cur.pit();
> > 1032
> > 1033 if (cur.pos() != 0 && cur.pos() != cur.lastpos()
> > 1034 && cpar.isChangeEdited(0, cur.pos()))
> > 1035 return;
> >
> >Nothing about the 'why' though...
> >
> >- Martin
> >
> >
> Hopefully this restriction is no longer necessary. It would be nice to
> see when this code has been introduced. (maybe some historic legacy?).
>
> We can expect people (like me :-)) to look for alternative ways to
> introduce line/par breaks (see my previous mail) but the consequence
> should be the same as if the line/par break was entered directly.
My understanding is this (but John Levon should know, he did this
initially): the change tracking system builds up a table of ranges that
are being change tracked. They can be in one of three states: UNCHANGED,
INSERTED (blue) and DELETED (red overline). A range can be a single
character or a range of characters within a paragraph, i.e., can
currently not contain a newline.
To sort-of fix this deficiency, there appears to be the implied
_convention_: if a paragraph contains _only_ added characters, and no
"originally present" or deleted characters -- as tested by
!isChangeEdited -- then the newline following it is of the INSERTED
type. Otherwise it is of the UNCHANGED type.
The break-out conditions found in the breakParagraph and backspace
methods sort-of enforce this, by not allowing you to create new
paragraphs unless they will contain only newly inserted characters.
Then, the whole paragraph, including its terminating newline, will
disappear when you roll back your changes. Any paragraph containing
also "original" characters will remain in the doc, with its terminal
newline.
If inserting newlines directly _were_ allowed, they would not be rolled
back together with the tracked changes to the surrounding characters. In
that sense change tracking would be inconsistent.
Cutting and pasting selections subvert this scheme.
I hope this explanation is both clear and correct ;-)
- Martin
pgpbZdOx8BZ9p.pgp
Description: PGP signature
Re: Various change tracking issues
Martin Vermeer wrote:
The code is here (text.C):
1026 void LyXText::breakParagraph(LCursor & cur, bool keep_layout)
1027 {
1028 BOOST_ASSERT(this == cur.text());
1029 // allow only if at start or end, or all previous is new text
1030 Paragraph & cpar = cur.paragraph();
1031 pit_type cpit = cur.pit();
1032
1033 if (cur.pos() != 0 && cur.pos() != cur.lastpos()
1034 && cpar.isChangeEdited(0, cur.pos()))
1035 return;
Nothing about the 'why' though...
- Martin
Hopefully this restriction is no longer necessary. It would be nice to
see when this code has been introduced. (maybe some historic legacy?).
We can expect people (like me :-)) to look for alternative ways to
introduce line/par breaks (see my previous mail) but the consequence
should be the same as if the line/par break was entered directly.
Michael
Re: Various change tracking issues
Martin Vermeer <[EMAIL PROTECTED]> writes:
| On Sun, Jan 01, 2006 at 02:00:58PM +0100, Michael Gerz wrote:
| > Michael Gerz wrote:
|
| ...
|
|
| > More investigation on paragraph breaks:
| >
| > Does not work: Pressing Enter in the middle of an unchanged line
| > Works: Selecting & copying paragraph break, pasting it in the middle
| > of an unchanged line
| > Works: Pressing Enter at the beginning and end of an unchanged line
| >
| > => Why do the latter work but not the first approach?
| >
| > Michael
|
| The code is here (text.C):
|
|1026 void LyXText::breakParagraph(LCursor & cur, bool keep_layout)
|1027 {
|1028 BOOST_ASSERT(this == cur.text());
|1029 // allow only if at start or end, or all previous is new text
|1030 Paragraph & cpar = cur.paragraph();
|1031 pit_type cpit = cur.pit();
|1032
|1033 if (cur.pos() != 0 && cur.pos() != cur.lastpos()
|1034 && cpar.isChangeEdited(0, cur.pos()))
|1035 return;
|
| Nothing about the 'why' though...
Changes in this area should wait for 1.4.1.
--
Lgb
Re: Various change tracking issues
On Sun, Jan 01, 2006 at 02:00:58PM +0100, Michael Gerz wrote:
> Michael Gerz wrote:
...
> More investigation on paragraph breaks:
>
> Does not work: Pressing Enter in the middle of an unchanged line
> Works: Selecting & copying paragraph break, pasting it in the middle
> of an unchanged line
> Works: Pressing Enter at the beginning and end of an unchanged line
>
> => Why do the latter work but not the first approach?
>
> Michael
The code is here (text.C):
1026 void LyXText::breakParagraph(LCursor & cur, bool keep_layout)
1027 {
1028 BOOST_ASSERT(this == cur.text());
1029 // allow only if at start or end, or all previous is new text
1030 Paragraph & cpar = cur.paragraph();
1031 pit_type cpit = cur.pit();
1032
1033 if (cur.pos() != 0 && cur.pos() != cur.lastpos()
1034 && cpar.isChangeEdited(0, cur.pos()))
1035 return;
Nothing about the 'why' though...
- Martin
pgpDKiNntrxqU.pgp
Description: PGP signature
Re: Various change tracking issues
On Sun, Jan 01, 2006 at 02:00:58PM +0100, Michael Gerz wrote: > Michael Gerz wrote: > > >Juergen Spitzmueller wrote: > > > >>My take on this is as follows: Fix the regressions that have been > >>introduced by the rowSignature patch and leave all other things as > >>they are. If we start to rewrite the change tracker code now (and > >>things like paragraph split/merge will need a major rewrite), we'll > >>see LyX 1.4 not before the year 2007. > >> > >> > >In a first approximation we might support paragraph split/merge > >without change tracking. I.e. you can split paragraphs but LyX does > >not consider this as a change. Do you think this is feasable with the > >current architecture? > > > More investigation on paragraph breaks: > > Does not work: Pressing Enter in the middle of an unchanged line > Works: Selecting & copying paragraph break, pasting it in the middle > of an unchanged line > Works: Pressing Enter at the beginning and end of an unchanged line > > => Why do the latter work but not the first approach? > > Michael Confirmed... weird. - Martin pgpizAmN90Fdl.pgp Description: PGP signature
Re: Various change tracking issues
I cannot test it but it looks much better. I'd say 'apply it'. JMarc
Re: Various change tracking issues
Martin Vermeer wrote: Actually I'd prefer to commit the attached. It is more general in that it *always* repaints the row that the cursor is on, whether directly, or in some inset. So I modified the cursor position test routine, achieving a simplification too. Tested and appears to work as well as before. OK to commit? Works like a charm! Michael
Re: Various change tracking issues
Michael Gerz wrote: Juergen Spitzmueller wrote: My take on this is as follows: Fix the regressions that have been introduced by the rowSignature patch and leave all other things as they are. If we start to rewrite the change tracker code now (and things like paragraph split/merge will need a major rewrite), we'll see LyX 1.4 not before the year 2007. In a first approximation we might support paragraph split/merge without change tracking. I.e. you can split paragraphs but LyX does not consider this as a change. Do you think this is feasable with the current architecture? More investigation on paragraph breaks: Does not work: Pressing Enter in the middle of an unchanged line Works: Selecting & copying paragraph break, pasting it in the middle of an unchanged line Works: Pressing Enter at the beginning and end of an unchanged line => Why do the latter work but not the first approach? Michael
Re: Various change tracking issues
On Sun, Jan 01, 2006 at 03:25:31AM +0100, Lars Gullik Bjønnes wrote:
> Martin Vermeer <[EMAIL PROTECTED]> writes:
...
> | I'll commit the isChanged version if no-one objects. Verified that it
> | does the job for both deletions and insertions.
>
> Ok, with me.
Actually I'd prefer to commit the attached. It is more general in that
it *always* repaints the row that the cursor is on, whether directly, or
in some inset. So I modified the cursor position test routine, achieving
a simplification too.
Tested and appears to work as well as before. OK to commit?
- Martin
Index: rowpainter.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/rowpainter.C,v
retrieving revision 1.161
diff -u -p -r1.161 rowpainter.C
--- rowpainter.C30 Dec 2005 19:02:52 - 1.161
+++ rowpainter.C1 Jan 2006 08:34:15 -
@@ -729,17 +729,14 @@ lyx::size_type calculateRowSignature(Row
}
-bool isCursorInInsetInRow(PainterInfo & pi, RowList::const_iterator rit,
- Paragraph const & par)
+bool isCursorOnRow(PainterInfo & pi, pit_type pit, RowList::const_iterator rit)
{
- InsetList::const_iterator ii = par.insetlist.begin();
- InsetList::const_iterator iend = par.insetlist.end();
- for ( ; ii != iend; ++ii) {
- if (ii->pos >= rit->pos() && ii->pos < rit->endpos()
- && ii->inset->isTextInset()
- && pi.base.bv->cursor().isInside(ii->inset))
- return true;
- }
+ LCursor & cur = pi.base.bv->cursor();
+ for (lyx::size_type d = 0; d < cur.depth(); d++)
+ if (cur[d].pit() == pit
+ && cur[d].pos() >= rit->pos()
+ && cur[d].pos() < rit->endpos())
+ return true;
return false;
}
@@ -767,15 +764,13 @@ void paintPar
// Row signature; has row changed since last paint?
lyx::size_type const row_sig = calculateRowSignature(*rit, par);
- // The following code figures out if the cursor is inside
- // an inset _on this row_.
- bool cur_in_inset_in_row = isCursorInInsetInRow(pi, rit, par);
-
+ bool cursor_on_row = isCursorOnRow(pi, pit, rit);
+
// If selection is on, the current row signature differs from
// from cache, or cursor is inside an inset _on this row_,
// then paint the row
- if (repaintAll || par.rowSignature()[rowno] != row_sig
- || cur_in_inset_in_row) {
+ if (repaintAll || par.rowSignature()[rowno] != row_sig
+ || cursor_on_row) {
// Add to row signature cache
par.rowSignature()[rowno] = row_sig;
pgpko4mc1fPYj.pgp
Description: PGP signature
Re: Various change tracking issues
Martin Vermeer <[EMAIL PROTECTED]> writes:
| On Sat, Dec 31, 2005 at 07:40:40PM +0200, Martin Vermeer wrote:
| > On Sat, Dec 31, 2005 at 05:51:47PM +0100, Lars Gullik Bjønnes wrote:
| > > Martin Vermeer <[EMAIL PROTECTED]> writes:
| > >
| > > | Index: rowpainter.C
| > > | ===
| > > | RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/rowpainter.C,v
| > > | retrieving revision 1.161
| > > | diff -u -p -r1.161 rowpainter.C
| > > | --- rowpainter.C30 Dec 2005 19:02:52 - 1.161
| > > | +++ rowpainter.C31 Dec 2005 17:07:55 -
| > > | @@ -774,8 +774,9 @@ void paintPar
| > > | // If selection is on, the current row signature
differs from
| > > | // from cache, or cursor is inside an inset _on this
row_,
| > > | // then paint the row
| > > | - if (repaintAll || par.rowSignature()[rowno] != row_sig
| > > | - || cur_in_inset_in_row) {
| > > | + if (repaintAll || par.rowSignature()[rowno] != row_sig
| > > | + || par.isChangeEdited(rit->pos(),
| > >
| > > Is isChangeEdited better to use than isChanged?
| > > (why?)
| >
| > Ah, did't even notice there were two of them.
| >
| > isChanged is apparently the correct one to use.
|
| I'll commit the isChanged version if no-one objects. Verified that it
| does the job for both deletions and insertions.
Ok, with me.
--
Lgb
Re: Various change tracking issues
On Sat, Dec 31, 2005 at 07:40:40PM +0200, Martin Vermeer wrote:
> On Sat, Dec 31, 2005 at 05:51:47PM +0100, Lars Gullik Bjønnes wrote:
> > Martin Vermeer <[EMAIL PROTECTED]> writes:
> >
> > | Index: rowpainter.C
> > | ===
> > | RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/rowpainter.C,v
> > | retrieving revision 1.161
> > | diff -u -p -r1.161 rowpainter.C
> > | --- rowpainter.C 30 Dec 2005 19:02:52 - 1.161
> > | +++ rowpainter.C 31 Dec 2005 17:07:55 -
> > | @@ -774,8 +774,9 @@ void paintPar
> > | // If selection is on, the current row signature differs from
> > | // from cache, or cursor is inside an inset _on this row_,
> > | // then paint the row
> > | - if (repaintAll || par.rowSignature()[rowno] != row_sig
> > | -|| cur_in_inset_in_row) {
> > | + if (repaintAll || par.rowSignature()[rowno] != row_sig
> > | + || par.isChangeEdited(rit->pos(),
> >
> > Is isChangeEdited better to use than isChanged?
> > (why?)
>
> Ah, did't even notice there were two of them.
>
> isChanged is apparently the correct one to use.
I'll commit the isChanged version if no-one objects. Verified that it
does the job for both deletions and insertions.
- Martin
pgpDhJJC8DSak.pgp
Description: PGP signature
Re: Various change tracking issues
On Sat, Dec 31, 2005 at 05:51:47PM +0100, Lars Gullik Bjønnes wrote:
> Martin Vermeer <[EMAIL PROTECTED]> writes:
>
> | Index: rowpainter.C
> | ===
> | RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/rowpainter.C,v
> | retrieving revision 1.161
> | diff -u -p -r1.161 rowpainter.C
> | --- rowpainter.C30 Dec 2005 19:02:52 - 1.161
> | +++ rowpainter.C31 Dec 2005 17:07:55 -
> | @@ -774,8 +774,9 @@ void paintPar
> | // If selection is on, the current row signature differs from
> | // from cache, or cursor is inside an inset _on this row_,
> | // then paint the row
> | - if (repaintAll || par.rowSignature()[rowno] != row_sig
> | - || cur_in_inset_in_row) {
> | + if (repaintAll || par.rowSignature()[rowno] != row_sig
> | + || par.isChangeEdited(rit->pos(),
>
> Is isChangeEdited better to use than isChanged?
> (why?)
Ah, did't even notice there were two of them.
isChanged is apparently the correct one to use.
Four hours and 20 minutes to 2006 here :-)
- Martin
pgpFoc7KR2o0F.pgp
Description: PGP signature
Re: Various change tracking issues
Martin Vermeer <[EMAIL PROTECTED]> writes:
| Index: rowpainter.C
| ===
| RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/rowpainter.C,v
| retrieving revision 1.161
| diff -u -p -r1.161 rowpainter.C
| --- rowpainter.C 30 Dec 2005 19:02:52 - 1.161
| +++ rowpainter.C 31 Dec 2005 17:07:55 -
| @@ -774,8 +774,9 @@ void paintPar
| // If selection is on, the current row signature differs from
| // from cache, or cursor is inside an inset _on this row_,
| // then paint the row
| - if (repaintAll || par.rowSignature()[rowno] != row_sig
| -|| cur_in_inset_in_row) {
| + if (repaintAll || par.rowSignature()[rowno] != row_sig
| + || par.isChangeEdited(rit->pos(),
Is isChangeEdited better to use than isChanged?
(why?)
--
Lgb
Re: Various change tracking issues
On Sat, Dec 31, 2005 at 04:34:35PM +0100, Michael Gerz wrote: > Lars Gullik Bjønnes wrote: > > >Actually I think the simple solution for now is to use > > > >Changes::isChange on the row range, and if we have a change, just repaint > >the row always. > > > >This can be accessed through Paragraph::isChanged. > > > >So in rowpainter.C: paintPar: > > > >If you do this manually, is the repainting fixed then? > > > > > Yes, it works! And the patch doesn't look too dirty :-) > > Thanks a lot ... and a happy new year! (I am leaving now.) I see I am late to the party ;-) - Martin pgpQdyWkZdcAH.pgp Description: PGP signature
Re: Various change tracking issues
On Sat, Dec 31, 2005 at 02:47:21PM +0100, Michael Gerz wrote:
> John C. McCabe-Dansted wrote:
>
> >On Sunday 01 January 2006 01:15, Michael Gerz wrote:
> >
> >
> >>SCREEN IS NOT UPDATED AFTER DELETION
> >>
> >>1. New doc
> >>2. Enter "hello"
> >>3. Activate change tracking
> >>4. Place cursor in front of "hello"
> >>5. Press delete key
> >>=> The character is deleted internally but the screen is not updated
> >>(row signature problem?)
> >>
> >>SCREEN IS NOT UPDATED AFTER BACKSPACE
> >>
> >>dito
> >>
> >>
> >
> >The other bugs occur on my system as well.
> >
> >
> I guess this bug has been "introduced" yesterday. Martin, this looks
> like a row signature bug.
Right! It is precisely that.
Try the attached, works for me.
- Martin
Index: rowpainter.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/rowpainter.C,v
retrieving revision 1.161
diff -u -p -r1.161 rowpainter.C
--- rowpainter.C30 Dec 2005 19:02:52 - 1.161
+++ rowpainter.C31 Dec 2005 17:07:55 -
@@ -774,8 +774,9 @@ void paintPar
// If selection is on, the current row signature differs from
// from cache, or cursor is inside an inset _on this row_,
// then paint the row
- if (repaintAll || par.rowSignature()[rowno] != row_sig
- || cur_in_inset_in_row) {
+ if (repaintAll || par.rowSignature()[rowno] != row_sig
+ || par.isChangeEdited(rit->pos(), rit->endpos())
+ || cur_in_inset_in_row) {
// Add to row signature cache
par.rowSignature()[rowno] = row_sig;
pgpsAaPajlqqY.pgp
Description: PGP signature
Re: Various change tracking issues
Juergen Spitzmueller wrote: My take on this is as follows: Fix the regressions that have been introduced by the rowSignature patch and leave all other things as they are. If we start to rewrite the change tracker code now (and things like paragraph split/merge will need a major rewrite), we'll see LyX 1.4 not before the year 2007. In a first approximation we might support paragraph split/merge without change tracking. I.e. you can split paragraphs but LyX does not consider this as a change. Do you think this is feasable with the current architecture? Michael
Re: Various change tracking issues
Lars Gullik Bjønnes wrote: Actually I think the simple solution for now is to use Changes::isChange on the row range, and if we have a change, just repaint the row always. This can be accessed through Paragraph::isChanged. So in rowpainter.C: paintPar: If you do this manually, is the repainting fixed then? Yes, it works! And the patch doesn't look too dirty :-) Thanks a lot ... and a happy new year! (I am leaving now.) Michael
Re: Various change tracking issues
[EMAIL PROTECTED] (Lars Gullik Bjønnes) writes:
| Michael Gerz <[EMAIL PROTECTED]> writes:
|
| | John C. McCabe-Dansted wrote:
| |
| | >On Sunday 01 January 2006 01:15, Michael Gerz wrote:
| | >
| | >>SCREEN IS NOT UPDATED AFTER DELETION
| | >>
| | >> 1. New doc
| | >> 2. Enter "hello"
| | >> 3. Activate change tracking
| | >> 4. Place cursor in front of "hello"
| | >> 5. Press delete key
| | >> => The character is deleted internally but the screen is not updated
| | >>(row signature problem?)
| | >>
| | >>SCREEN IS NOT UPDATED AFTER BACKSPACE
| | >>
| | >> dito
| | >>
| | >
| | >The other bugs occur on my system as well.
| | >
| | I guess this bug has been "introduced" yesterday. Martin, this looks
| | like a row signature bug.
|
| yes... tracker info is not stored directly in the row. so on a change
| like this the row signature will stay the same.
Actually I think the simple solution for now is to use
Changes::isChange on the row range, and if we have a change, just repaint the
row always.
This can be accessed through Paragraph::isChanged.
So in rowpainter.C: paintPar:
bool haveChanges = par.isChanged(rit->pos(), rit->endPos());
if (repaintAll || haveChanges || ...) {
// repaint row
...
}
If you do this manually, is the repainting fixed then?
diff -u -p -r1.161 rowpainter.C
--- rowpainter.C30 Dec 2005 19:02:52 - 1.161
+++ rowpainter.C31 Dec 2005 15:46:45 -
@@ -764,6 +764,9 @@ void paintPar
for (RowList::const_iterator rit = rb; rit != re; ++rit, ++rowno) {
y += rit->ascent();
+bool const haveChanges =
+par.isChanged(rit->pos(), rit->endpos());
+
// Row signature; has row changed since last paint?
lyx::size_type const row_sig =
calculateRowSignature(*rit, par);
@@ -774,8 +777,9 @@ void paintPar
// If selection is on, the current row signature differs from
// from cache, or cursor is inside an inset _on this row_,
// then paint the row
- if (repaintAll || par.rowSignature()[rowno] != row_sig
- || cur_in_inset_in_row) {
+ if (repaintAll || haveChanges
+|| par.rowSignature()[rowno] != row_sig
+|| cur_in_inset_in_row) {
// Add to row signature cache
par.rowSignature()[rowno] = row_sig;
--
Lgb
Re: Various change tracking issues
Juergen Spitzmueller <[EMAIL PROTECTED]> writes: | Michael Gerz wrote: | > I am still wondering why the user has to accept all changes before he is | > allowed to deactivate change tracking. | > - If there is a technical reason for it, then the scenrio above will | > cause trouble sooner or later. | > - If not, then LyX restricts the application of change tracking | > unnecessarily. The user should be able to activate and deactivate change | > tracking at any time. The existing document should not be affected by | > the switch, only future modifications. | | My take on this is as follows: Fix the regressions that have been introduced | by the rowSignature patch and leave all other things as they are. If we start | to rewrite the change tracker code now (and things like paragraph split/merge | will need a major rewrite), we'll see LyX 1.4 not before the year 2007. | | I agree that especially the row split/merge stuff is annoying, but we have to | finally get 1.4 out now. BTW there are much more severe limitations in the | unofficial 1.3 ct patch, and people still seem to find it useful. I agree. -- Lgb
Re: Various change tracking issues
Michael Gerz wrote: > I am still wondering why the user has to accept all changes before he is > allowed to deactivate change tracking. > - If there is a technical reason for it, then the scenrio above will > cause trouble sooner or later. > - If not, then LyX restricts the application of change tracking > unnecessarily. The user should be able to activate and deactivate change > tracking at any time. The existing document should not be affected by > the switch, only future modifications. My take on this is as follows: Fix the regressions that have been introduced by the rowSignature patch and leave all other things as they are. If we start to rewrite the change tracker code now (and things like paragraph split/merge will need a major rewrite), we'll see LyX 1.4 not before the year 2007. I agree that especially the row split/merge stuff is annoying, but we have to finally get 1.4 out now. BTW there are much more severe limitations in the unofficial 1.3 ct patch, and people still seem to find it useful. Guten Rutsch, Jürgen
Re: Various change tracking issues
Lars Gullik Bjønnes wrote: yes... tracker info is not stored directly in the row. so on a change like this the row signature will stay the same. LFUNS that require an update should perhaps make the row redraw always. Well, I don't know all the details of LyX but to me this does not sound like a proper solution. Even today, tracker info must be associated with each character in a row (somehow). So when computing the signature, the tracker info should be considered as well. Changing the LFUNS looks like a dirty hack. Michael
Re: Various change tracking issues
Michael Gerz <[EMAIL PROTECTED]> writes: | John C. McCabe-Dansted wrote: | | >On Sunday 01 January 2006 01:15, Michael Gerz wrote: | > | >>SCREEN IS NOT UPDATED AFTER DELETION | >> | >> 1. New doc | >> 2. Enter "hello" | >> 3. Activate change tracking | >> 4. Place cursor in front of "hello" | >> 5. Press delete key | >> => The character is deleted internally but the screen is not updated | >>(row signature problem?) | >> | >>SCREEN IS NOT UPDATED AFTER BACKSPACE | >> | >> dito | >> | > | >The other bugs occur on my system as well. | > | I guess this bug has been "introduced" yesterday. Martin, this looks | like a row signature bug. yes... tracker info is not stored directly in the row. so on a change like this the row signature will stay the same. LFUNS that require an update should perhaps make the row redraw always. -- Lgb
Re: Various change tracking issues
Lars Gullik Bjønnes wrote: | UNCHANGED PARAGRAPHS CANNOT BE SPLIT a bit more severe. This is basic functionality. Yes. | => The line break is visible again with a change bar to the left | (OK!) but there is no way to accept this change because change | tracking is deactivated. IMHO, there is a conceptual problem which is | linked with bug #2030. Anoying, but I think it can wait. I am still wondering why the user has to accept all changes before he is allowed to deactivate change tracking. - If there is a technical reason for it, then the scenrio above will cause trouble sooner or later. - If not, then LyX restricts the application of change tracking unnecessarily. The user should be able to activate and deactivate change tracking at any time. The existing document should not be affected by the switch, only future modifications. What about BACKSPACE at end of buffer? Does the cursor move then? Yes, it does (which makes sense). Michael
Re: Various change tracking issues
John C. McCabe-Dansted wrote: On Sunday 01 January 2006 01:15, Michael Gerz wrote: SCREEN IS NOT UPDATED AFTER DELETION 1. New doc 2. Enter "hello" 3. Activate change tracking 4. Place cursor in front of "hello" 5. Press delete key => The character is deleted internally but the screen is not updated (row signature problem?) SCREEN IS NOT UPDATED AFTER BACKSPACE dito The other bugs occur on my system as well. I guess this bug has been "introduced" yesterday. Martin, this looks like a row signature bug. Michael
Re: Various change tracking issues
I cannot replicate the following two bugs on lyx-1.4.0pre3-Qt, My system is as follows: Kubuntu 5.10, gcc 4.02, QT 3.3.4, KDE 3.5. What version of LyX and QT (or xforms or...) are you using? Are these MS-Windows only bugs? On Sunday 01 January 2006 01:15, Michael Gerz wrote: > SCREEN IS NOT UPDATED AFTER DELETION > > 1. New doc > 2. Enter "hello" > 3. Activate change tracking > 4. Place cursor in front of "hello" > 5. Press delete key > => The character is deleted internally but the screen is not updated > (row signature problem?) > > SCREEN IS NOT UPDATED AFTER BACKSPACE > > dito The other bugs occur on my system as well. -- John C. McCabe-Dansted Masters Student
Re: Various change tracking issues
Michael Gerz <[EMAIL PROTECTED]> writes: | UNDO OF LINE BREAKS FAILS minor can wait. | UNCHANGED PARAGRAPHS CANNOT BE SPLIT a bit more severe. This is basic functionality. | CHANGES CANNOT BE ACCEPTED UNLESS CHANGE TRACKING IS ACTIVATED | 1. New doc | 2. Enter "hello world" | 3. Place cursor after "hello" | 4. Activate change tracking | 5. Press CTRL-Enter (line break) | 6. Backspace (remove line break) | 7. Deactivate change tracking | 8. Undo | => The line break is visible again with a change bar to the left | (OK!) but there is no way to accept this change because change | tracking is deactivated. IMHO, there is a conceptual problem which is | linked with bug #2030. Anoying, but I think it can wait. | SCREEN IS NOT UPDATED AFTER DELETION | SCREEN IS NOT UPDATED AFTER BACKSPACE we should have a quick look at these. | DELETE KEY DOES NOT MOVE THE CURSOR TO THE NEXT CHARACTER If this has a simple fix it should be done, otherwise it should wait. What about BACKSPACE at end of buffer? Does the cursor move then? -- Lgb
Various change tracking issues
Hello, I am sorry to say so but change tracking is not in a good shape at the moment. There are many small problems that, in combination, make change tracking almost unusuable. (If we were able to fix at least a few of them, life would be much easier) I have produced a set of test cases. I thought it might be better to send them to the mailing list before adding a couple of bugzilla entries. UNDO OF LINE BREAKS FAILS 1. New doc 2. Enter "hello world" 3. Place cursor after "hello" 4. Activate change tracking 5. Press CTRL-Enter (line break) 6. Undo => Error: Undo is deactivated! UNCHANGED PARAGRAPHS CANNOT BE SPLIT 1. New doc 2. Enter "hello world" 3. Place cursor after "hello" 4. Activate change tracking 5. Press Enter (new paragraph) => Error: Does not work! This bug is really annoying because you cannot simply deactivate ct temporarily (without accepting all changes) CHANGES CANNOT BE ACCEPTED UNLESS CHANGE TRACKING IS ACTIVATED 1. New doc 2. Enter "hello world" 3. Place cursor after "hello" 4. Activate change tracking 5. Press CTRL-Enter (line break) 6. Backspace (remove line break) 7. Deactivate change tracking 8. Undo => The line break is visible again with a change bar to the left (OK!) but there is no way to accept this change because change tracking is deactivated. IMHO, there is a conceptual problem which is linked with bug #2030. SCREEN IS NOT UPDATED AFTER DELETION 1. New doc 2. Enter "hello" 3. Activate change tracking 4. Place cursor in front of "hello" 5. Press delete key => The character is deleted internally but the screen is not updated (row signature problem?) SCREEN IS NOT UPDATED AFTER BACKSPACE dito DELETE KEY DOES NOT MOVE THE CURSOR TO THE NEXT CHARACTER 1. New doc 2. Enter "hello" 3. Activate change tracking 4. Place cursor in front of "hello" 5. Press delete key (delete first character) => cursor stays in front of first (deleted) character; there is no simple way to continue deletion; the cursor should be moved to the next character
