Re: Problem stripLeadingSpaces()

2007-01-21 Thread Abdelrazak Younes

Michael Gerz wrote:

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:
I don't understand, why the space in the first line should be 
deleted?


deleted space means a space that was marked as deleted.

Use the attached lyx file. Press return after hello. The leading 
spaces are marked as deleted but the screen is not updated.


Well, I still don't understand because the screen seems fine here, 
see attached.


No, the screen is not fine: the second space should be marked as 
deleted!


Why is that? Before I hit Enter, the second space was not deleted; 
was should it be deleted after?
The semantics of stripLeadingSpaces() is, well, to strip _all_ leading 
spaces. Since the second space is a leading space, too, it is marked as 
deleted in CT mode. stripLeadingSpaces() works as intended but the 
screen is not updated afterwards.


Right now, stripLeadingSpaces() returns the number of _physically_ 
deleted spaces.


OK, IIUC, then you will have two deleted space then, right? Again, IIUC, 
the paragraph contents should still contain the two spaces but 
isDeleted(0) and (1) would return true, right?
If yes, then as I said, there is a problem even before the screen update 
problem because while debugging I noticed that there was only _one_ 
leading space, not two.


I can change this to return the number of physically and 
logically characters. This may help but may also prevent some 
optimizations in DEPM.


I think that DEPM is indeed the culprit here and we have to teach it to 
disregard deleted spaces (and deleted paragraphs for the matter).




I also don't understand why we return false at the end of 
deleteEmptyParMech() even if stripLeadingSpaces returns !0. Ah, and we 
should clarify/document the meaning of the return value of 
deleteEmptyParMech...


What do you think? I am a bit lost without your assistance...


I have to study the code before answering you. Maybe what I said above 
will help you.


Abdel.



Re: Problem stripLeadingSpaces()

2007-01-21 Thread Abdelrazak Younes

Michael Gerz wrote:

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:
I don't understand, why the space in the first line should be 
deleted?


deleted space means a space that was marked as deleted.

Use the attached lyx file. Press return after hello. The leading 
spaces are marked as deleted but the screen is not updated.


Well, I still don't understand because the screen seems fine here, 
see attached.


No, the screen is not fine: the second space should be marked as 
deleted!


Why is that? Before I hit Enter, the second space was not deleted; 
was should it be deleted after?
The semantics of stripLeadingSpaces() is, well, to strip _all_ leading 
spaces. Since the second space is a leading space, too, it is marked as 
deleted in CT mode. stripLeadingSpaces() works as intended but the 
screen is not updated afterwards.


Right now, stripLeadingSpaces() returns the number of _physically_ 
deleted spaces. I can change this to return the number of physically and 
logically characters.


Yes, I guess we should do that.

This may help but may also prevent some 
optimizations in DEPM.


I also don't understand why we return false at the end of 
deleteEmptyParMech() even if stripLeadingSpaces returns !0.



I've change that and now the crash is gone because going up with the 
arrow key will trigger the DEPM and the metrics will then be updated.


Still, I guess my commit only cures the symptom because the second 
paragraph is still wrong. At least it's now wrong in the paragraph 
contents also. If you want the space to be deleted as soon as the new 
paragraph is created, we need to call stripLeadingSpaces() on the new 
paragraph also, and we don't do that in DEPM.


Ah, and we 
should clarify/document the meaning of the return value of 
deleteEmptyParMech...


Yes.

Abdel.



What do you think? I am a bit lost without your assistance...

Michael






Re: Problem stripLeadingSpaces()

2007-01-21 Thread Michael Gerz

Abdelrazak Younes schrieb:
OK, IIUC, then you will have two deleted space then, right? Again, 
IIUC, the paragraph contents should still contain the two spaces but 
isDeleted(0) and (1) would return true, right?

Yes  Yes.
If yes, then as I said, there is a problem even before the screen 
update problem because while debugging I noticed that there was only 
_one_ leading space, not two.

Hmmm. I see two (striked-out) spaces after moving around the cursor.

Let's see how the situation changed with your latest patch.

Michael


Re: Problem stripLeadingSpaces()

2007-01-21 Thread Michael Gerz

Abdelrazak Younes schrieb:


Right now, stripLeadingSpaces() returns the number of _physically_ 
deleted spaces. I can change this to return the number of physically 
and logically characters.


Yes, I guess we should do that.

Ok, the patch will be committed in a few minutes.

Michael


Re: Problem stripLeadingSpaces()

2007-01-21 Thread Abdelrazak Younes

Michael Gerz wrote:

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:
I don't understand, why the space in the first line should be 
deleted?


 means a space that was marked as deleted.

Use the attached lyx file. Press return after "hello". The leading 
spaces are marked as deleted but the screen is not updated.


Well, I still don't understand because the screen seems fine here, 
see attached.


No, the screen is not fine: the second space should be marked as 
deleted!


Why is that? Before I hit "Enter", the second space was not deleted; 
was should it be deleted after?
The semantics of stripLeadingSpaces() is, well, to strip _all_ leading 
spaces. Since the second space is a leading space, too, it is marked as 
deleted in CT mode. stripLeadingSpaces() works as intended but the 
screen is not updated afterwards.


Right now, stripLeadingSpaces() returns the number of _physically_ 
deleted spaces.


OK, IIUC, then you will have two deleted space then, right? Again, IIUC, 
the paragraph contents should still contain the two spaces but 
isDeleted(0) and (1) would return true, right?
If yes, then as I said, there is a problem even before the screen update 
problem because while debugging I noticed that there was only _one_ 
leading space, not two.


I can change this to return the number of physically and 
logically characters. This may help but may also prevent some 
optimizations in DEPM.


I think that DEPM is indeed the culprit here and we have to teach it to 
disregard deleted spaces (and deleted paragraphs for the matter).




I also don't understand why we return false at the end of 
deleteEmptyParMech() even if stripLeadingSpaces returns !0. Ah, and we 
should clarify/document the meaning of the return value of 
deleteEmptyParMech...


What do you think? I am a bit lost without your assistance...


I have to study the code before answering you. Maybe what I said above 
will help you.


Abdel.



Re: Problem stripLeadingSpaces()

2007-01-21 Thread Abdelrazak Younes

Michael Gerz wrote:

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:
I don't understand, why the space in the first line should be 
deleted?


 means a space that was marked as deleted.

Use the attached lyx file. Press return after "hello". The leading 
spaces are marked as deleted but the screen is not updated.


Well, I still don't understand because the screen seems fine here, 
see attached.


No, the screen is not fine: the second space should be marked as 
deleted!


Why is that? Before I hit "Enter", the second space was not deleted; 
was should it be deleted after?
The semantics of stripLeadingSpaces() is, well, to strip _all_ leading 
spaces. Since the second space is a leading space, too, it is marked as 
deleted in CT mode. stripLeadingSpaces() works as intended but the 
screen is not updated afterwards.


Right now, stripLeadingSpaces() returns the number of _physically_ 
deleted spaces. I can change this to return the number of physically and 
logically characters.


Yes, I guess we should do that.

This may help but may also prevent some 
optimizations in DEPM.


I also don't understand why we return false at the end of 
deleteEmptyParMech() even if stripLeadingSpaces returns !0.



I've change that and now the crash is gone because going up with the 
arrow key will trigger the DEPM and the metrics will then be updated.


Still, I guess my commit only cures the symptom because the second 
paragraph is still wrong. At least it's now wrong in the paragraph 
contents also. If you want the space to be deleted as soon as the new 
paragraph is created, we need to call stripLeadingSpaces() on the new 
paragraph also, and we don't do that in DEPM.


Ah, and we 
should clarify/document the meaning of the return value of 
deleteEmptyParMech...


Yes.

Abdel.



What do you think? I am a bit lost without your assistance...

Michael






Re: Problem stripLeadingSpaces()

2007-01-21 Thread Michael Gerz

Abdelrazak Younes schrieb:
OK, IIUC, then you will have two deleted space then, right? Again, 
IIUC, the paragraph contents should still contain the two spaces but 
isDeleted(0) and (1) would return true, right?

Yes & Yes.
If yes, then as I said, there is a problem even before the screen 
update problem because while debugging I noticed that there was only 
_one_ leading space, not two.

Hmmm. I see two (striked-out) spaces after moving around the cursor.

Let's see how the situation changed with your latest patch.

Michael


Re: Problem stripLeadingSpaces()

2007-01-21 Thread Michael Gerz

Abdelrazak Younes schrieb:


Right now, stripLeadingSpaces() returns the number of _physically_ 
deleted spaces. I can change this to return the number of physically 
and logically characters.


Yes, I guess we should do that.

Ok, the patch will be committed in a few minutes.

Michael


Re: Problem stripLeadingSpaces()

2007-01-20 Thread Michael Gerz

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:
I don't understand, why the space in the first line should be 
deleted?


deleted space means a space that was marked as deleted.

Use the attached lyx file. Press return after hello. The leading 
spaces are marked as deleted but the screen is not updated.


Well, I still don't understand because the screen seems fine here, 
see attached.


No, the screen is not fine: the second space should be marked as 
deleted!


Why is that? Before I hit Enter, the second space was not deleted; 
was should it be deleted after?
The semantics of stripLeadingSpaces() is, well, to strip _all_ leading 
spaces. Since the second space is a leading space, too, it is marked as 
deleted in CT mode. stripLeadingSpaces() works as intended but the 
screen is not updated afterwards.


Right now, stripLeadingSpaces() returns the number of _physically_ 
deleted spaces. I can change this to return the number of physically and 
logically characters. This may help but may also prevent some 
optimizations in DEPM.


I also don't understand why we return false at the end of 
deleteEmptyParMech() even if stripLeadingSpaces returns !0. Ah, and we 
should clarify/document the meaning of the return value of 
deleteEmptyParMech...


What do you think? I am a bit lost without your assistance...

Michael



Re: Problem stripLeadingSpaces()

2007-01-20 Thread Michael Gerz

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:
I don't understand, why the space in the first line should be 
deleted?


 means a space that was marked as deleted.

Use the attached lyx file. Press return after "hello". The leading 
spaces are marked as deleted but the screen is not updated.


Well, I still don't understand because the screen seems fine here, 
see attached.


No, the screen is not fine: the second space should be marked as 
deleted!


Why is that? Before I hit "Enter", the second space was not deleted; 
was should it be deleted after?
The semantics of stripLeadingSpaces() is, well, to strip _all_ leading 
spaces. Since the second space is a leading space, too, it is marked as 
deleted in CT mode. stripLeadingSpaces() works as intended but the 
screen is not updated afterwards.


Right now, stripLeadingSpaces() returns the number of _physically_ 
deleted spaces. I can change this to return the number of physically and 
logically characters. This may help but may also prevent some 
optimizations in DEPM.


I also don't understand why we return false at the end of 
deleteEmptyParMech() even if stripLeadingSpaces returns !0. Ah, and we 
should clarify/document the meaning of the return value of 
deleteEmptyParMech...


What do you think? I am a bit lost without your assistance...

Michael



Re: Problem stripLeadingSpaces()

2007-01-16 Thread Abdelrazak Younes

Michael Gerz wrote:

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:

I don't understand, why the space in the first line should be deleted?


deleted space means a space that was marked as deleted.

Use the attached lyx file. Press return after hello. The leading 
spaces are marked as deleted but the screen is not updated.


Well, I still don't understand because the screen seems fine here, see 
attached.


No, the screen is not fine: the second space should be marked as deleted!


Why is that? Before I hit Enter, the second space was not deleted; was 
should it be deleted after?


I am missing something, obviously.



Re: Problem stripLeadingSpaces()

2007-01-16 Thread Abdelrazak Younes

Michael Gerz wrote:

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:

I don't understand, why the space in the first line should be deleted?


 means a space that was marked as deleted.

Use the attached lyx file. Press return after "hello". The leading 
spaces are marked as deleted but the screen is not updated.


Well, I still don't understand because the screen seems fine here, see 
attached.


No, the screen is not fine: the second space should be marked as deleted!


Why is that? Before I hit "Enter", the second space was not deleted; was 
should it be deleted after?


I am missing something, obviously.



Problem stripLeadingSpaces()

2007-01-14 Thread Michael Gerz

Hi,

I rethought the logic of stripLeadingSpaces() and came to the conclusion 
that it is not a good idea to remove the spaces in change tracking mode: 
imagine that your co-author inserts a par break that you don't like. If 
you revert his par break (revert = reject change, not undo!), you would 
like to see the spaces restored.


I changed stripLeadingSpaces accordingly (see attached patch) but 
unfortunately I ran into two problems that look like painting/update 
problems.


By switching CT on and off, create a one-line document that looks like this:

hellodeleted spacespaceworld

Now, in CT mode, press return after hello. The expected output is:

hello
deleted spacedeleted spaceworld

However, this is what I get on screen:

hello
deleted spacespaceworld

In other words, the deletion of the second space is not shown on screen.

Even worse, if I move the cursor up (to first line) and down (back to 
second line), LyX crashes!


I can't imagine that my fix is responsible for this crash. Abdel, do you 
have any idea of what goes wrong?


Michael
Index: src/paragraph.h
===
--- src/paragraph.h (Revision 16671)
+++ src/paragraph.h (Arbeitskopie)
@@ -346,7 +346,7 @@
int getPositionOfInset(InsetBase const * inset) const;
 
/// Returns the number of line breaks and white-space stripped at the 
start
-   int stripLeadingSpaces();
+   int stripLeadingSpaces(bool trackChanges);
 
/// return true if we allow multiple spaces
bool isFreeSpacing() const;
Index: src/text2.C
===
--- src/text2.C (Revision 16671)
+++ src/text2.C (Arbeitskopie)
@@ -1231,7 +1231,7 @@
return true;
}
 
-   if (oldpar.stripLeadingSpaces())
+   if (oldpar.stripLeadingSpaces(cur.buffer().params().trackChanges))
need_anchor_change = true;
 
return false;
Index: src/CutAndPaste.C
===
--- src/CutAndPaste.C   (Revision 16671)
+++ src/CutAndPaste.C   (Arbeitskopie)
@@ -270,7 +270,7 @@
pars[last_paste].makeSameLayout(pars[last_paste + 1]);
mergeParagraph(buffer.params(), pars, last_paste);
} else {
-   pars[last_paste + 1].stripLeadingSpaces();
+   pars[last_paste + 
1].stripLeadingSpaces(buffer.params().trackChanges);
++last_paste;
}
}
@@ -309,7 +309,7 @@
(pit + 1 != endpit || pars[pit].hasSameLayout(pars[pit + 
1]))) {
pos_type const thissize = pars[pit].size();
if (doclear)
-   pars[pit + 1].stripLeadingSpaces();
+   pars[pit + 
1].stripLeadingSpaces(params.trackChanges);
mergeParagraph(params, pars, pit);
--endpit;
if (pit == endpit)
@@ -539,7 +539,7 @@
 
// sometimes necessary
if (doclear)
-   text-paragraphs()[begpit].stripLeadingSpaces();
+   
text-paragraphs()[begpit].stripLeadingSpaces(bp.trackChanges);
 
// cutSelection can invalidate the cursor so we need to set
// it anew. (Lgb)
Index: src/paragraph.C
===
--- src/paragraph.C (Revision 16671)
+++ src/paragraph.C (Arbeitskopie)
@@ -560,19 +560,22 @@
 }
 
 
-int Paragraph::stripLeadingSpaces()
+int Paragraph::stripLeadingSpaces(bool trackChanges)
 {
if (isFreeSpacing())
return 0;
 
-   int i = 0;
-   while (!empty()  (isNewline(0) || isLineSeparator(0))
-!isDeleted(0)) {
-   eraseChar(0, false); // no change tracking here
-   ++i;
+   int pos = 0;
+   int count = 0;
+
+   while (pos  size()  (isNewline(pos) || isLineSeparator(pos))) {
+   if (eraseChar(pos, trackChanges))
+   ++count;
+   else
+   ++pos;
}
 
-   return i;
+   return count;
 }
 
 


Re: Problem stripLeadingSpaces()

2007-01-14 Thread Abdelrazak Younes

Michael Gerz wrote:

Hi,

I rethought the logic of stripLeadingSpaces() and came to the conclusion 
that it is not a good idea to remove the spaces in change tracking mode: 
imagine that your co-author inserts a par break that you don't like. If 
you revert his par break (revert = reject change, not undo!), you would 
like to see the spaces restored.


I changed stripLeadingSpaces accordingly (see attached patch) but 
unfortunately I ran into two problems that look like painting/update 
problems.


By switching CT on and off, create a one-line document that looks like 
this:


hellodeleted spacespaceworld

Now, in CT mode, press return after hello. The expected output is:

hello
deleted spacedeleted spaceworld


I don't understand, why the space in the first line should be deleted?
Two deleted space does not sound right anyway.




However, this is what I get on screen:

hello
deleted spacespaceworld


I don't understand exactly what I am supposed to do get this behaviour. 
When I hit return I have the same thing on the second line than what was 
in the first line after hello.




In other words, the deletion of the second space is not shown on screen.


Maybe dEPM is triggered? You probably have to modify this method to take 
into account TrackChange.




Even worse, if I move the cursor up (to first line) and down (back to 
second line), LyX crashes!


I manage to reproduce that. Do you remember the Peter's fix in 
numberOfHfills() that got reverted at the end. This is exactly what 
happens here.


I can't imagine that my fix is responsible for this crash. Abdel, do you 
have any idea of what goes wrong?


I think that you have to distinguish the document model from what is on 
screen. On screen, the row representation does not care if one char is a 
space or a deleted space, seven chars are seven chars.


I guess this is the problem and you have to skip deleted char in 
numberOfHfills() or something like that.


Abdel.



Re: Problem stripLeadingSpaces()

2007-01-14 Thread Michael Gerz

Abdelrazak Younes schrieb:

I don't understand, why the space in the first line should be deleted?


deleted space means a space that was marked as deleted.

Use the attached lyx file. Press return after hello. The leading 
spaces are marked as deleted but the screen is not updated.


Michael


strip.lyx
Description: application/lyx


Re: Problem stripLeadingSpaces()

2007-01-14 Thread Abdelrazak Younes

Abdelrazak Younes wrote:

Michael Gerz wrote:

I think that you have to distinguish the document model from what is 
on screen. On screen, the row representation does not care if one char 
is a space or a deleted space, seven chars are seven chars.


I guess this is the problem and you have to skip deleted char in 
numberOfHfills() or something like that.
Yes, this is exactly it. The problem is that the first (and unique) row 
of the first ParagraphMetrics has one more character than what is in the 
Paragraph. The Paragraph content does not seem to reflect the deleted 
characters.  In the following loop, the position of the last char or the 
row does not correspond to the position of the last char in the paragraph.


   for (pos_type p = first; p  last; ++p) {
   if (par.isHfill(p))
   ++n;
   }

Abdel.



Re: Problem stripLeadingSpaces()

2007-01-14 Thread Abdelrazak Younes

Michael Gerz wrote:

Abdelrazak Younes schrieb:

I don't understand, why the space in the first line should be deleted?


deleted space means a space that was marked as deleted.

Use the attached lyx file. Press return after hello. The leading 
spaces are marked as deleted but the screen is not updated.


Well, I still don't understand because the screen seems fine here, see 
attached.


Abdel.




Re: Problem stripLeadingSpaces()

2007-01-14 Thread Michael Gerz

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:

I don't understand, why the space in the first line should be deleted?


deleted space means a space that was marked as deleted.

Use the attached lyx file. Press return after hello. The leading 
spaces are marked as deleted but the screen is not updated.


Well, I still don't understand because the screen seems fine here, see 
attached.


No, the screen is not fine: the second space should be marked as deleted!

Michael



Re: Problem stripLeadingSpaces()

2007-01-14 Thread Michael Gerz

Abdelrazak Younes schrieb:
Yes, this is exactly it. The problem is that the first (and unique) 
row of the first ParagraphMetrics has one more character than what is 
in the Paragraph. The Paragraph content does not seem to reflect the 
deleted characters.  In the following loop, the position of the last 
char or the row does not correspond to the position of the last char 
in the paragraph.


   for (pos_type p = first; p  last; ++p) {
   if (par.isHfill(p))
   ++n;
   }


Your comment clearly indicates that there is a missing metrics/screen 
update somewhere.


As said before, when adding the par break to my example file, both 
leading spaces should be marked as deleted. You can easily validate this 
by pressing return at the beginning of the first line. Magically, the 
two spaces are marked red in the (now) third line.


Michael

PS: I committed the innocent patch to stripLeadingSpaces(). The screen 
update has to be handled somewhere outside this method.





Problem stripLeadingSpaces()

2007-01-14 Thread Michael Gerz

Hi,

I rethought the logic of stripLeadingSpaces() and came to the conclusion 
that it is not a good idea to remove the spaces in change tracking mode: 
imagine that your co-author inserts a par break that you don't like. If 
you revert his par break (revert = reject change, not undo!), you would 
like to see the spaces restored.


I changed stripLeadingSpaces accordingly (see attached patch) but 
unfortunately I ran into two problems that look like painting/update 
problems.


By switching CT on and off, create a one-line document that looks like this:

helloworld

Now, in CT mode, press return after "hello". The expected output is:

hello
world

However, this is what I get on screen:

hello
world

In other words, the deletion of the second space is not shown on screen.

Even worse, if I move the cursor up (to first line) and down (back to 
second line), LyX crashes!


I can't imagine that my fix is responsible for this crash. Abdel, do you 
have any idea of what goes wrong?


Michael
Index: src/paragraph.h
===
--- src/paragraph.h (Revision 16671)
+++ src/paragraph.h (Arbeitskopie)
@@ -346,7 +346,7 @@
int getPositionOfInset(InsetBase const * inset) const;
 
/// Returns the number of line breaks and white-space stripped at the 
start
-   int stripLeadingSpaces();
+   int stripLeadingSpaces(bool trackChanges);
 
/// return true if we allow multiple spaces
bool isFreeSpacing() const;
Index: src/text2.C
===
--- src/text2.C (Revision 16671)
+++ src/text2.C (Arbeitskopie)
@@ -1231,7 +1231,7 @@
return true;
}
 
-   if (oldpar.stripLeadingSpaces())
+   if (oldpar.stripLeadingSpaces(cur.buffer().params().trackChanges))
need_anchor_change = true;
 
return false;
Index: src/CutAndPaste.C
===
--- src/CutAndPaste.C   (Revision 16671)
+++ src/CutAndPaste.C   (Arbeitskopie)
@@ -270,7 +270,7 @@
pars[last_paste].makeSameLayout(pars[last_paste + 1]);
mergeParagraph(buffer.params(), pars, last_paste);
} else {
-   pars[last_paste + 1].stripLeadingSpaces();
+   pars[last_paste + 
1].stripLeadingSpaces(buffer.params().trackChanges);
++last_paste;
}
}
@@ -309,7 +309,7 @@
(pit + 1 != endpit || pars[pit].hasSameLayout(pars[pit + 
1]))) {
pos_type const thissize = pars[pit].size();
if (doclear)
-   pars[pit + 1].stripLeadingSpaces();
+   pars[pit + 
1].stripLeadingSpaces(params.trackChanges);
mergeParagraph(params, pars, pit);
--endpit;
if (pit == endpit)
@@ -539,7 +539,7 @@
 
// sometimes necessary
if (doclear)
-   text->paragraphs()[begpit].stripLeadingSpaces();
+   
text->paragraphs()[begpit].stripLeadingSpaces(bp.trackChanges);
 
// cutSelection can invalidate the cursor so we need to set
// it anew. (Lgb)
Index: src/paragraph.C
===
--- src/paragraph.C (Revision 16671)
+++ src/paragraph.C (Arbeitskopie)
@@ -560,19 +560,22 @@
 }
 
 
-int Paragraph::stripLeadingSpaces()
+int Paragraph::stripLeadingSpaces(bool trackChanges)
 {
if (isFreeSpacing())
return 0;
 
-   int i = 0;
-   while (!empty() && (isNewline(0) || isLineSeparator(0))
-   && !isDeleted(0)) {
-   eraseChar(0, false); // no change tracking here
-   ++i;
+   int pos = 0;
+   int count = 0;
+
+   while (pos < size() && (isNewline(pos) || isLineSeparator(pos))) {
+   if (eraseChar(pos, trackChanges))
+   ++count;
+   else
+   ++pos;
}
 
-   return i;
+   return count;
 }
 
 


Re: Problem stripLeadingSpaces()

2007-01-14 Thread Abdelrazak Younes

Michael Gerz wrote:

Hi,

I rethought the logic of stripLeadingSpaces() and came to the conclusion 
that it is not a good idea to remove the spaces in change tracking mode: 
imagine that your co-author inserts a par break that you don't like. If 
you revert his par break (revert = reject change, not undo!), you would 
like to see the spaces restored.


I changed stripLeadingSpaces accordingly (see attached patch) but 
unfortunately I ran into two problems that look like painting/update 
problems.


By switching CT on and off, create a one-line document that looks like 
this:


helloworld

Now, in CT mode, press return after "hello". The expected output is:

hello
world


I don't understand, why the space in the first line should be deleted?
Two  does not sound right anyway.




However, this is what I get on screen:

hello
world


I don't understand exactly what I am supposed to do get this behaviour. 
When I hit return I have the same thing on the second line than what was 
in the first line after "hello".




In other words, the deletion of the second space is not shown on screen.


Maybe dEPM is triggered? You probably have to modify this method to take 
into account TrackChange.




Even worse, if I move the cursor up (to first line) and down (back to 
second line), LyX crashes!


I manage to reproduce that. Do you remember the Peter's fix in 
numberOfHfills() that got reverted at the end. This is exactly what 
happens here.


I can't imagine that my fix is responsible for this crash. Abdel, do you 
have any idea of what goes wrong?


I think that you have to distinguish the document model from what is on 
screen. On screen, the row representation does not care if one char is a 
 or a , seven chars are seven chars.


I guess this is the problem and you have to skip deleted char in 
numberOfHfills() or something like that.


Abdel.



Re: Problem stripLeadingSpaces()

2007-01-14 Thread Michael Gerz

Abdelrazak Younes schrieb:

I don't understand, why the space in the first line should be deleted?


 means a space that was marked as deleted.

Use the attached lyx file. Press return after "hello". The leading 
spaces are marked as deleted but the screen is not updated.


Michael


strip.lyx
Description: application/lyx


Re: Problem stripLeadingSpaces()

2007-01-14 Thread Abdelrazak Younes

Abdelrazak Younes wrote:

Michael Gerz wrote:

I think that you have to distinguish the document model from what is 
on screen. On screen, the row representation does not care if one char 
is a  or a , seven chars are seven chars.


I guess this is the problem and you have to skip deleted char in 
numberOfHfills() or something like that.
Yes, this is exactly it. The problem is that the first (and unique) row 
of the first ParagraphMetrics has one more character than what is in the 
Paragraph. The Paragraph content does not seem to reflect the deleted 
characters.  In the following loop, the position of the last char or the 
row does not correspond to the position of the last char in the paragraph.


   for (pos_type p = first; p < last; ++p) {
   if (par.isHfill(p))
   ++n;
   }

Abdel.



Re: Problem stripLeadingSpaces()

2007-01-14 Thread Abdelrazak Younes

Michael Gerz wrote:

Abdelrazak Younes schrieb:

I don't understand, why the space in the first line should be deleted?


 means a space that was marked as deleted.

Use the attached lyx file. Press return after "hello". The leading 
spaces are marked as deleted but the screen is not updated.


Well, I still don't understand because the screen seems fine here, see 
attached.


Abdel.




Re: Problem stripLeadingSpaces()

2007-01-14 Thread Michael Gerz

Abdelrazak Younes schrieb:

Michael Gerz wrote:

Abdelrazak Younes schrieb:

I don't understand, why the space in the first line should be deleted?


 means a space that was marked as deleted.

Use the attached lyx file. Press return after "hello". The leading 
spaces are marked as deleted but the screen is not updated.


Well, I still don't understand because the screen seems fine here, see 
attached.


No, the screen is not fine: the second space should be marked as deleted!

Michael



Re: Problem stripLeadingSpaces()

2007-01-14 Thread Michael Gerz

Abdelrazak Younes schrieb:
Yes, this is exactly it. The problem is that the first (and unique) 
row of the first ParagraphMetrics has one more character than what is 
in the Paragraph. The Paragraph content does not seem to reflect the 
deleted characters.  In the following loop, the position of the last 
char or the row does not correspond to the position of the last char 
in the paragraph.


   for (pos_type p = first; p < last; ++p) {
   if (par.isHfill(p))
   ++n;
   }


Your comment clearly indicates that there is a missing metrics/screen 
update somewhere.


As said before, when adding the par break to my example file, both 
leading spaces should be marked as deleted. You can easily validate this 
by pressing return at the beginning of the first line. Magically, the 
two spaces are marked red in the (now) third line.


Michael

PS: I committed the innocent patch to stripLeadingSpaces(). The screen 
update has to be handled somewhere outside this method.