Re: r32435 - lyx-devel/trunk/src
lasgout...@lyx.org wrote:
Author: lasgouttes
Date: Tue Dec 8 23:41:10 2009
New Revision: 32435
URL: http://www.lyx.org/trac/changeset/32435
Log:
Speedup updateMacroInstances. This has some effect on document loading time:
it was a significant part of this time
This commit is obviously correct but Vincent reported something strange
with forwardInset(), in that this will jump over portion of the document
instead of merely poping out of the current inset. Or maybe it is just
that the method name is misleading? In this case, I would rename it to
nextInset().
Abdel.
Modified:
lyx-devel/trunk/src/Buffer.cpp
Modified: lyx-devel/trunk/src/Buffer.cpp
==
--- lyx-devel/trunk/src/Buffer.cpp Tue Dec 8 19:58:14 2009(r32434)
+++ lyx-devel/trunk/src/Buffer.cpp Tue Dec 8 23:41:10 2009(r32435)
@@ -2657,14 +2657,11 @@
LYXERR(Debug::MACROS, updateMacroInstances for
d-filename.onlyFileName());
DocIterator it = doc_iterator_begin(this);
- DocIterator end = doc_iterator_end(this);
- for (; it != end; it.forwardPos()) {
- // look for MathData cells in InsetMathNest insets
- Inset * inset = it.nextInset();
- if (!inset)
- continue;
-
- InsetMath * minset = inset-asInsetMath();
+ it.forwardInset();
+ DocIterator const end = doc_iterator_end(this);
+ for (; it != end; it.forwardInset()) {
+ // look for MathData cells in InsetMathNest insets
+ InsetMath * minset = it.nextInset()-asInsetMath();
if (!minset)
continue;
Re: r32435 - lyx-devel/trunk/src
This commit is obviously correct but Vincent reported something strange with forwardInset(), in that this will jump over portion of the document instead of merely poping out of the current inset. Or maybe it is just that the method name is misleading? In this case, I would rename it to nextInset(). Yes. I planned to take a look at this, but I cannot find the discussion anymore. Where was it? JMarc
Re: r32435 - lyx-devel/trunk/src
Jean-Marc Lasgouttes wrote:
This commit is obviously correct but Vincent reported something
strange with forwardInset(), in that this will jump over portion of
the document instead of merely poping out of the current inset. Or
maybe it is just that the method name is misleading? In this case, I
would rename it to nextInset().
Yes. I planned to take a look at this, but I cannot find the
discussion anymore. Where was it?
In the thread [patch] Spellchecker Bugs
Abdel.
---BeginMessage---
On 05/12/2009 02:25, Vincent van Ravesteijn wrote:
This patch fixes a few things for spellchecking.
while (from.inMathed())
from.forwardInset();
This caused LyX to skip parts of a document between two math insets:
I'd say that this is a bug of forwardInset() then.
+if (from == end)
+break;
This is needed because if from is at the end of the document (which is
possible when leaving the Math Inset), LyX will crash later.
OK with your comment above.
+if (from == to)
+continue;
This is needed to get a correct count of the number of words. However,
the reported number of words seems to be pretty random.
Ditto
I'm not sure about this part:
+while (from.inMathed()) {
+from.pop_back();
+from.pos()++;
+}
As said above, I'd rather have forwardInset() fixed.
Thanks for looking at those bugs.
Abdel.
---End Message---
Re: r32435 - lyx-devel/trunk/src
Le 9 déc. 09 à 09:26, Abdelrazak Younes a écrit : Jean-Marc Lasgouttes wrote: This commit is obviously correct but Vincent reported something strange with forwardInset(), in that this will jump over portion of the document instead of merely poping out of the current inset. Or maybe it is just that the method name is misleading? In this case, I would rename it to nextInset(). Yes. I planned to take a look at this, but I cannot find the discussion anymore. Where was it? In the thread [patch] Spellchecker Bugs I see it now. Actually, forwardInset does not iterate inside math insets, it only looks at insets that are inside text editor. So Vincents patch is very correct with this semantics. We could of course change that, but going to each and every inset of math stuff is very boring (everything is an inset) and time consuming. I think we do not want that in 99% of the cases. JMarc
Re: r32435 - lyx-devel/trunk/src
lasgout...@lyx.org wrote:
Author: lasgouttes
Date: Tue Dec 8 23:41:10 2009
New Revision: 32435
URL: http://www.lyx.org/trac/changeset/32435
Log:
Speedup updateMacroInstances. This has some effect on document loading time:
it was a significant part of this time
This commit is obviously correct but Vincent reported something strange
with forwardInset(), in that this will jump over portion of the document
instead of merely poping out of the current inset. Or maybe it is just
that the method name is misleading? In this case, I would rename it to
nextInset().
Abdel.
Modified:
lyx-devel/trunk/src/Buffer.cpp
Modified: lyx-devel/trunk/src/Buffer.cpp
==
--- lyx-devel/trunk/src/Buffer.cpp Tue Dec 8 19:58:14 2009(r32434)
+++ lyx-devel/trunk/src/Buffer.cpp Tue Dec 8 23:41:10 2009(r32435)
@@ -2657,14 +2657,11 @@
LYXERR(Debug::MACROS, "updateMacroInstances for "
<< d->filename.onlyFileName());
DocIterator it = doc_iterator_begin(this);
- DocIterator end = doc_iterator_end(this);
- for (; it != end; it.forwardPos()) {
- // look for MathData cells in InsetMathNest insets
- Inset * inset = it.nextInset();
- if (!inset)
- continue;
-
- InsetMath * minset = inset->asInsetMath();
+ it.forwardInset();
+ DocIterator const end = doc_iterator_end(this);
+ for (; it != end; it.forwardInset()) {
+ // look for MathData cells in InsetMathNest insets
+ InsetMath * minset = it.nextInset()->asInsetMath();
if (!minset)
continue;
Re: r32435 - lyx-devel/trunk/src
This commit is obviously correct but Vincent reported something strange with forwardInset(), in that this will jump over portion of the document instead of merely poping out of the current inset. Or maybe it is just that the method name is misleading? In this case, I would rename it to nextInset(). Yes. I planned to take a look at this, but I cannot find the discussion anymore. Where was it? JMarc
Re: r32435 - lyx-devel/trunk/src
Jean-Marc Lasgouttes wrote:
This commit is obviously correct but Vincent reported something
strange with forwardInset(), in that this will jump over portion of
the document instead of merely poping out of the current inset. Or
maybe it is just that the method name is misleading? In this case, I
would rename it to nextInset().
Yes. I planned to take a look at this, but I cannot find the
discussion anymore. Where was it?
In the thread "[patch] Spellchecker Bugs"
Abdel.
--- Begin Message ---
On 05/12/2009 02:25, Vincent van Ravesteijn wrote:
This patch fixes a few things for spellchecking.
while (from.inMathed())
from.forwardInset();
This caused LyX to skip parts of a document between two math insets:
I'd say that this is a bug of forwardInset() then.
+if (from == end)
+break;
This is needed because if from is at the end of the document (which is
possible when leaving the Math Inset), LyX will crash later.
OK with your comment above.
+if (from == to)
+continue;
This is needed to get a correct count of the number of words. However,
the reported number of words seems to be pretty random.
Ditto
I'm not sure about this part:
+while (from.inMathed()) {
+from.pop_back();
+from.pos()++;
+}
As said above, I'd rather have forwardInset() fixed.
Thanks for looking at those bugs.
Abdel.
--- End Message ---
Re: r32435 - lyx-devel/trunk/src
Le 9 déc. 09 à 09:26, Abdelrazak Younes a écrit : Jean-Marc Lasgouttes wrote: This commit is obviously correct but Vincent reported something strange with forwardInset(), in that this will jump over portion of the document instead of merely poping out of the current inset. Or maybe it is just that the method name is misleading? In this case, I would rename it to nextInset(). Yes. I planned to take a look at this, but I cannot find the discussion anymore. Where was it? In the thread "[patch] Spellchecker Bugs" I see it now. Actually, forwardInset does not iterate inside math insets, it only looks at insets that are inside text editor. So Vincents patch is very correct with this semantics. We could of course change that, but going to each and every inset of math stuff is very boring (everything is an inset) and time consuming. I think we do not want that in 99% of the cases. JMarc
