Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Abdelrazak Younes]

Richard Heck wrote:

Bo Peng wrote:

The way to solve this might be to put some appropriate code into
InsetCaption::notifyCursorLeaves().


I do not think it is a good idea to update Toc during editing, because
simple add/remove of sections, change of environment will break Toc,
so it is close to impossible to consider all cases and update Toc.
Yes, you are absolutely right. It's too bad, really. I think Abdel was 
right to want it to update automatically, but there are too many cases.


Wait, when you modify a section text, only the corresponding TocItem is 
updated, not the full TocBackend (Have a look at 
TocBackend::updateItem()). The item updating is done only for sections, 
not for captions because of the limitation of the Inset::addToToc() 
method. I see that you changed that now so we can synchronize the view 
and the TocBackend for captions and listings too in a very cheap way now.


I don't have the time to read all the messages since last week but if 
you decided to deleted these update calls please give me a summary of 
the discussion.


Abdel.

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Abdelrazak Younes]

Bo Peng wrote:

The way to solve this might be to put some appropriate code into
InsetCaption::notifyCursorLeaves().


I do not think it is a good idea to update Toc during editing, because
simple add/remove of sections, change of environment will break Toc,
so it is close to impossible to consider all cases and update Toc.

Because TocBackend::update() is expansive, I even think we should
remove all such calls triggered by, e.g., changing the environment of
a paragraph to section.


Please don't touch at that. When you change a section depth, the full 
renumbering is done (in updateLabels()). It is only natural to update 
also the TocBackend at the same time. Actually this update is much 
quicker than the section renumbering.


Abdel.

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

Please don't touch at that. When you change a section depth, the full
renumbering is done (in updateLabels()). It is only natural to update
also the TocBackend at the same time. Actually this update is much
quicker than the section renumbering.


We have not done anything, and if the operators are cheap as you
described, it is perfectly fine to me to make TocUpdate automatic.

Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Abdelrazak Younes]

Bo Peng wrote:

Please don't touch at that. When you change a section depth, the full
renumbering is done (in updateLabels()). It is only natural to update
also the TocBackend at the same time. Actually this update is much
quicker than the section renumbering.


We have not done anything, and if the operators are cheap as you
described, it is perfectly fine to me to make TocUpdate automatic.


Good, thanks.

Abdel.

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

 We have not done anything, and if the operators are cheap as you
 described, it is perfectly fine to me to make TocUpdate automatic.

Good, thanks.


But then it is your job to get it done. :-)

Seriously, as we have discussed, the problem lies in 'when to update
Toc'. It is unwise to update Toc when we enter each character when
editing a caption, but there is no proper way to know 'editing is done
here'...

Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Abdelrazak Younes]

Bo Peng wrote:

 We have not done anything, and if the operators are cheap as you
 described, it is perfectly fine to me to make TocUpdate automatic.

Good, thanks.


But then it is your job to get it done. :-)


I am slowly coming back, don't ask me too much ;-)



Seriously, as we have discussed, the problem lies in 'when to update
Toc'. It is unwise to update Toc when we enter each character when
editing a caption, but there is no proper way to know 'editing is done
here'...


The proper way is to check the Buffer structure when you are editing the 
caption with a call to checkBufferStructure(), maybe this is already 
done. Then you also need to correct InsetCaption::addToToc(), the same 
way as you did with InsetListings. Finally, you'll have to make sure 
that TocBackend::updateItem() properly find the correct TocItem.


Not a lot of work but a lot of testing is required.

Abdel.

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Abdelrazak Younes]

Richard Heck wrote:

Bo Peng wrote:

The way to solve this might be to put some appropriate code into
InsetCaption::notifyCursorLeaves().


I do not think it is a good idea to update Toc during editing, because
simple add/remove of sections, change of environment will break Toc,
so it is close to impossible to consider all cases and update Toc.
Yes, you are absolutely right. It's too bad, really. I think Abdel was 
right to want it to update automatically, but there are too many cases.


Wait, when you modify a section text, only the corresponding TocItem is 
updated, not the full TocBackend (Have a look at 
TocBackend::updateItem()). The item updating is done only for sections, 
not for captions because of the limitation of the Inset::addToToc() 
method. I see that you changed that now so we can synchronize the view 
and the TocBackend for captions and listings too in a very cheap way now.


I don't have the time to read all the messages since last week but if 
you decided to deleted these update calls please give me a summary of 
the discussion.


Abdel.

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Abdelrazak Younes]

Bo Peng wrote:

The way to solve this might be to put some appropriate code into
InsetCaption::notifyCursorLeaves().


I do not think it is a good idea to update Toc during editing, because
simple add/remove of sections, change of environment will break Toc,
so it is close to impossible to consider all cases and update Toc.

Because TocBackend::update() is expansive, I even think we should
remove all such calls triggered by, e.g., changing the environment of
a paragraph to section.


Please don't touch at that. When you change a section depth, the full 
renumbering is done (in updateLabels()). It is only natural to update 
also the TocBackend at the same time. Actually this update is much 
quicker than the section renumbering.


Abdel.

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

Please don't touch at that. When you change a section depth, the full
renumbering is done (in updateLabels()). It is only natural to update
also the TocBackend at the same time. Actually this update is much
quicker than the section renumbering.


We have not done anything, and if the operators are cheap as you
described, it is perfectly fine to me to make TocUpdate automatic.

Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Abdelrazak Younes]

Bo Peng wrote:

Please don't touch at that. When you change a section depth, the full
renumbering is done (in updateLabels()). It is only natural to update
also the TocBackend at the same time. Actually this update is much
quicker than the section renumbering.


We have not done anything, and if the operators are cheap as you
described, it is perfectly fine to me to make TocUpdate automatic.


Good, thanks.

Abdel.

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

> We have not done anything, and if the operators are cheap as you
> described, it is perfectly fine to me to make TocUpdate automatic.

Good, thanks.


But then it is your job to get it done. :-)

Seriously, as we have discussed, the problem lies in 'when to update
Toc'. It is unwise to update Toc when we enter each character when
editing a caption, but there is no proper way to know 'editing is done
here'...

Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Abdelrazak Younes]

Bo Peng wrote:

> We have not done anything, and if the operators are cheap as you
> described, it is perfectly fine to me to make TocUpdate automatic.

Good, thanks.


But then it is your job to get it done. :-)


I am slowly coming back, don't ask me too much ;-)



Seriously, as we have discussed, the problem lies in 'when to update
Toc'. It is unwise to update Toc when we enter each character when
editing a caption, but there is no proper way to know 'editing is done
here'...


The proper way is to check the Buffer structure when you are editing the 
caption with a call to checkBufferStructure(), maybe this is already 
done. Then you also need to correct InsetCaption::addToToc(), the same 
way as you did with InsetListings. Finally, you'll have to make sure 
that TocBackend::updateItem() properly find the correct TocItem.


Not a lot of work but a lot of testing is required.

Abdel.

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Richard Heck]

Bo Peng wrote:

The way to solve this might be to put some appropriate code into
InsetCaption::notifyCursorLeaves().


I do not think it is a good idea to update Toc during editing, because
simple add/remove of sections, change of environment will break Toc,
so it is close to impossible to consider all cases and update Toc.
Yes, you are absolutely right. It's too bad, really. I think Abdel was 
right to want it to update automatically, but there are too many cases.

Because TocBackend::update() is expansive, I even think we should
remove all such calls triggered by, e.g., changing the environment of
a paragraph to section. 

There don't seem to be many of these.
[EMAIL PROTECTED] src]$ srcgrep 'tocBackend().update()'
buffer_funcs.cpp:704:   cbuf.tocBackend().update();
frontends/controllers/ControlToc.cpp:120:   
kernel().buffer().getMasterBuffer()-tocBackend().update();

MenuBackend.cpp:708:const_castBuffer*(buf)-tocBackend().update();
(I checked for 'update()', too.) So we just need to delete the one in 
buffer_funcs.cpp, which is in updateLabels().

Instead, we can think of a good way to update
Toc when the outline panel is *used*. (The case when the navigation
menu is *used* is solved by r18693.)
Again, absolutely correct, I think. But if so, then I also think the 
update button should be made more prominent, maybe even put at the top, 
because people are otherwise going to expect the outline to update 
automatically. (That's what got all of this started.) And the 
update-on-use feature is critical, anyway, and even more so though if 
we're not regularly updating. As I said in another thread, I've caused 
crashes without it, though I can't seem to find a recipe.


Richard

--
==
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

There don't seem to be many of these.
[EMAIL PROTECTED] src]$ srcgrep 'tocBackend().update()'
buffer_funcs.cpp:704:   cbuf.tocBackend().update();
frontends/controllers/ControlToc.cpp:120:
kernel().buffer().getMasterBuffer()-tocBackend().update();
MenuBackend.cpp:708:const_castBuffer*(buf)-tocBackend().update();
(I checked for 'update()', too.) So we just need to delete the one in
buffer_funcs.cpp, which is in updateLabels().


After we figure out how to 'update on use', these can be removed.


Again, absolutely correct, I think. But if so, then I also think the
update button should be made more prominent, maybe even put at the top,
because people are otherwise going to expect the outline to update
automatically. (That's what got all of this started.) And the
update-on-use feature is critical, anyway, and even more so though if
we're not regularly updating. As I said in another thread, I've caused
crashes without it, though I can't seem to find a recipe.


How about:

1. keep a copy of old Toc in the outline panel
2. when a user clicks in the outline panel, update Toc
3. compare the old and new Toc,
 a. if there is no change, proceed as usual
 b. if there are changes, update outline panel,
- handle user request if we can still complete the user's request
(item name etc),
- otherwise, let the user know he has to click again

Of course, repositioning of the refresh button is also needed.

Cheers,
Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Richard Heck]

Bo Peng wrote:

The way to solve this might be to put some appropriate code into
InsetCaption::notifyCursorLeaves().


I do not think it is a good idea to update Toc during editing, because
simple add/remove of sections, change of environment will break Toc,
so it is close to impossible to consider all cases and update Toc.
Yes, you are absolutely right. It's too bad, really. I think Abdel was 
right to want it to update automatically, but there are too many cases.

Because TocBackend::update() is expansive, I even think we should
remove all such calls triggered by, e.g., changing the environment of
a paragraph to section. 

There don't seem to be many of these.
[EMAIL PROTECTED] src]$ srcgrep 'tocBackend().update()'
buffer_funcs.cpp:704:   cbuf.tocBackend().update();
frontends/controllers/ControlToc.cpp:120:   
kernel().buffer().getMasterBuffer()->tocBackend().update();

MenuBackend.cpp:708:const_cast(buf)->tocBackend().update();
(I checked for 'update()', too.) So we just need to delete the one in 
buffer_funcs.cpp, which is in updateLabels().

Instead, we can think of a good way to update
Toc when the outline panel is *used*. (The case when the navigation
menu is *used* is solved by r18693.)
Again, absolutely correct, I think. But if so, then I also think the 
update button should be made more prominent, maybe even put at the top, 
because people are otherwise going to expect the outline to update 
automatically. (That's what got all of this started.) And the 
update-on-use feature is critical, anyway, and even more so though if 
we're not regularly updating. As I said in another thread, I've caused 
crashes without it, though I can't seem to find a recipe.


Richard

--
==
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

There don't seem to be many of these.
[EMAIL PROTECTED] src]$ srcgrep 'tocBackend().update()'
buffer_funcs.cpp:704:   cbuf.tocBackend().update();
frontends/controllers/ControlToc.cpp:120:
kernel().buffer().getMasterBuffer()->tocBackend().update();
MenuBackend.cpp:708:const_cast(buf)->tocBackend().update();
(I checked for 'update()', too.) So we just need to delete the one in
buffer_funcs.cpp, which is in updateLabels().


After we figure out how to 'update on use', these can be removed.


Again, absolutely correct, I think. But if so, then I also think the
update button should be made more prominent, maybe even put at the top,
because people are otherwise going to expect the outline to update
automatically. (That's what got all of this started.) And the
update-on-use feature is critical, anyway, and even more so though if
we're not regularly updating. As I said in another thread, I've caused
crashes without it, though I can't seem to find a recipe.


How about:

1. keep a copy of old Toc in the outline panel
2. when a user clicks in the outline panel, update Toc
3. compare the old and new Toc,
 a. if there is no change, proceed as usual
 b. if there are changes, update outline panel,
- handle user request if we can still complete the user's request
(item name etc),
- otherwise, let the user know he has to click again

Of course, repositioning of the refresh button is also needed.

Cheers,
Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Jean-Marc Lasgouttes]

 Richard == Richard Heck [EMAIL PROTECTED] writes:

Richard Jean-Marc Lasgouttes wrote:
 update toc and list of xxx seems to me an expensive operation.
 Can it be done on demand? I mean, maybe we can add a
 'refresh'/'regenerate' button in the outline panel; and update
 'list of listings' and such only if this menu item is clicked.
 
  This is how it used to be in the good old time, but Abdel decided
 that automatic was cooler...
 
Richard Cooler, definitely, and this is what users will expect,
Richard especially with the navigate menu. So there is also the
Richard question about how one might regenerate the navigation menu.

The navigation menu is regenerated just before displaying it. At least
this is what it used to do, without much overhead.

JMarc

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Richard Heck]

Jean-Marc Lasgouttes wrote:

The navigation menu is regenerated just before displaying it. At least
this is what it used to do, without much overhead.
  
Well, it doesn't do this now. Open the Embedded Objects manual; go to 
some figure, and delete some characters from the caption. (Do not 
highlight and delete. That regenerates. Just delete.) Exit the caption 
(again, you have to be careful how you do this, as some actions 
regenerate) and open NavigateList of Figures. Not updated.


Richard

--
==
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

Well, it doesn't do this now. Open the Embedded Objects manual; go to
some figure, and delete some characters from the caption. (Do not
highlight and delete. That regenerates. Just delete.) Exit the caption
(again, you have to be careful how you do this, as some actions
regenerate) and open NavigateList of Figures. Not updated.


I noticed this as well and this is pretty annoying. Maybe I will track
it down  later.

Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Richard Heck]

Bo Peng wrote:

Well, it doesn't do this now. Open the Embedded Objects manual; go to
some figure, and delete some characters from the caption. (Do not
highlight and delete. That regenerates. Just delete.) Exit the caption
(again, you have to be careful how you do this, as some actions
regenerate) and open NavigateList of Figures. Not updated.


I noticed this as well and this is pretty annoying. Maybe I will track 
it down  later.
Anything that fixed the TOC behavior would also fix this. But it would 
be possible to separate these and have the TOC update only by explicit 
request (say) whereas the menu updated when it was shown.


Richard

--
==
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

Anything that fixed the TOC behavior would also fix this. But it would
be possible to separate these and have the TOC update only by explicit
request (say) whereas the menu updated when it was shown.


When I insert 'update toc' message to 'TocBackend::update()', I find
that it is called whenever section title is updated, but not when, for
example, a caption of InsetListings is modified.

I am *not* going to call update each time when some caption is
updated, because there are too many such cases, and it is *not*
necessary to update Toc, which is by my standard an expansive option,
each time. Because Outline panel already has a 'update' button, I
guess for this purpose, I will simply call update when navigation menu
is clicked.

Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

The way to solve this might be to put some appropriate code into
InsetCaption::notifyCursorLeaves().


I do not think it is a good idea to update Toc during editing, because
simple add/remove of sections, change of environment will break Toc,
so it is close to impossible to consider all cases and update Toc.

Because TocBackend::update() is expansive, I even think we should
remove all such calls triggered by, e.g., changing the environment of
a paragraph to section. Instead, we can think of a good way to update
Toc when the outline panel is *used*. (The case when the navigation
menu is *used* is solved by r18693.)

Cheers,
Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Jean-Marc Lasgouttes]

> "Richard" == Richard Heck <[EMAIL PROTECTED]> writes:

Richard> Jean-Marc Lasgouttes wrote:
 update toc and list of xxx seems to me an expensive operation.
 Can it be done on demand? I mean, maybe we can add a
 'refresh'/'regenerate' button in the outline panel; and update
 'list of listings' and such only if this menu item is clicked.
 
>>  This is how it used to be in the good old time, but Abdel decided
>> that automatic was cooler...
>> 
Richard> Cooler, definitely, and this is what users will expect,
Richard> especially with the navigate menu. So there is also the
Richard> question about how one might regenerate the navigation menu.

The navigation menu is regenerated just before displaying it. At least
this is what it used to do, without much overhead.

JMarc

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Richard Heck]

Jean-Marc Lasgouttes wrote:

The navigation menu is regenerated just before displaying it. At least
this is what it used to do, without much overhead.
  
Well, it doesn't do this now. Open the Embedded Objects manual; go to 
some figure, and delete some characters from the caption. (Do not 
highlight and delete. That regenerates. Just delete.) Exit the caption 
(again, you have to be careful how you do this, as some actions 
regenerate) and open Navigate>List of Figures. Not updated.


Richard

--
==
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

Well, it doesn't do this now. Open the Embedded Objects manual; go to
some figure, and delete some characters from the caption. (Do not
highlight and delete. That regenerates. Just delete.) Exit the caption
(again, you have to be careful how you do this, as some actions
regenerate) and open Navigate>List of Figures. Not updated.


I noticed this as well and this is pretty annoying. Maybe I will track
it down  later.

Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Richard Heck]

Bo Peng wrote:

Well, it doesn't do this now. Open the Embedded Objects manual; go to
some figure, and delete some characters from the caption. (Do not
highlight and delete. That regenerates. Just delete.) Exit the caption
(again, you have to be careful how you do this, as some actions
regenerate) and open Navigate>List of Figures. Not updated.


I noticed this as well and this is pretty annoying. Maybe I will track 
it down  later.
Anything that fixed the TOC behavior would also fix this. But it would 
be possible to separate these and have the TOC update only by explicit 
request (say) whereas the menu updated when it was shown.


Richard

--
==
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

Anything that fixed the TOC behavior would also fix this. But it would
be possible to separate these and have the TOC update only by explicit
request (say) whereas the menu updated when it was shown.


When I insert 'update toc' message to 'TocBackend::update()', I find
that it is called whenever section title is updated, but not when, for
example, a caption of InsetListings is modified.

I am *not* going to call update each time when some caption is
updated, because there are too many such cases, and it is *not*
necessary to update Toc, which is by my standard an expansive option,
each time. Because Outline panel already has a 'update' button, I
guess for this purpose, I will simply call update when navigation menu
is clicked.

Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

The way to solve this might be to put some appropriate code into
InsetCaption::notifyCursorLeaves().


I do not think it is a good idea to update Toc during editing, because
simple add/remove of sections, change of environment will break Toc,
so it is close to impossible to consider all cases and update Toc.

Because TocBackend::update() is expansive, I even think we should
remove all such calls triggered by, e.g., changing the environment of
a paragraph to section. Instead, we can think of a good way to update
Toc when the outline panel is *used*. (The case when the navigation
menu is *used* is solved by r18693.)

Cheers,
Bo

[PATCH] Part of Bug 3719 -- TOC out of sync

[Richard Heck]

Richard Heck wrote:

Bo Peng wrote:
Because you are on this topic, can you also check when navigation - 
list of listings and other entries are updated? When I add a listings 
insets with caption, I can see an empty entry but not the caption.

This is a similar issue. The update is done by this call:
   cbuf.tocBackend().update();
at line 705 of buffer_funcs.cp, in updateLabels(). So if you create a 
new paragraph, again, you get no update and the menu is wrong, but if 
you then backspace, you do get an update and it's right. The question 
is thus why this isn't being called for LFUN_CAPTION_INSERT. I'll try 
to look at that later, or you can do it.
This is wrong. updateLabels() IS called from LFUN_CAPTION_INSERT. But it 
is NOT called after the caption itself is edited. So you get the same 
problem if you change an existing caption and then view the navigation 
menu or, for that matter, the TOC.


The way to solve this might be to put some appropriate code into 
InsetCaption::notifyCursorLeaves(). I tried this but don't have anything 
that works. (Mostly, I cause crashes.) In fact, however, this won't 
quite work anyway, because notifyCursorLeaves() doesn't seem to be 
called if the cursor leaves the inset via keyboard navigation, though it 
is called if you move the cursor out with the mouse. That seems wrong, 
i.e., it seems like a bug.


Now, my sense from the comment in Inset.h is that notifyCursorLeaves() 
wasn't really intended as this kind of hook. But it is used this way in 
mathed/InsetMathHull.cpp, where it's used to call do the preview.


JMarc?

Richard

--
==
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

This is wrong. updateLabels() IS called from LFUN_CAPTION_INSERT. But it
is NOT called after the caption itself is edited. So you get the same
problem if you change an existing caption and then view the navigation
menu or, for that matter, the TOC.

The way to solve this might be to put some appropriate code into
InsetCaption::notifyCursorLeaves(). I tried this but don't have anything
that works. (Mostly, I cause crashes.) In fact, however, this won't
quite work anyway, because notifyCursorLeaves() doesn't seem to be
called if the cursor leaves the inset via keyboard navigation, though it
is called if you move the cursor out with the mouse. That seems wrong,
i.e., it seems like a bug.


update toc and list of xxx seems to me an expensive operation. Can it
be done on demand? I mean, maybe we can add a 'refresh'/'regenerate'
button in the outline panel; and update 'list of listings' and such
only if this menu item is clicked.

Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Richard Heck]

Bo Peng wrote:

This is wrong. updateLabels() IS called from LFUN_CAPTION_INSERT. But it
is NOT called after the caption itself is edited. So you get the same
problem if you change an existing caption and then view the navigation
menu or, for that matter, the TOC.

The way to solve this might be to put some appropriate code into
InsetCaption::notifyCursorLeaves(). I tried this but don't have anything
that works. (Mostly, I cause crashes.) In fact, however, this won't
quite work anyway, because notifyCursorLeaves() doesn't seem to be
called if the cursor leaves the inset via keyboard navigation, though it
is called if you move the cursor out with the mouse. That seems wrong,
i.e., it seems like a bug.


update toc and list of xxx seems to me an expensive operation. Can it
be done on demand? I mean, maybe we can add a 'refresh'/'regenerate'
button in the outline panel; and update 'list of listings' and such
only if this menu item is clicked.
This kind of update seems to be done quite a lot. Indeed, it seems to me 
that there's really too much updating or menus and toolbars and the like 
going on in general, but it's probably best to be safe here, unless we 
really know what we're doing.


The attached should solve the issue modulo the question when 
notifyCursorLeaves() gets called. I've just borrowed the code from 
ControlToc::updateBackend(). I think it was the missing call to 
structureChanged() that was giving me the crashes I was experiencing.


There is a refresh button in the outline dialog already. It also has the 
effect that it updates the menu.


Richard

--
==
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto

Index: src/insets/InsetCaption.h
===
--- src/insets/InsetCaption.h	(revision 18677)
+++ src/insets/InsetCaption.h	(working copy)
@@ -59,6 +59,8 @@
 	///
 	virtual bool getStatus(Cursor  cur, FuncRequest const  cmd, FuncStatus ) const;
 	///
+	virtual bool notifyCursorLeaves(Cursor );
+	///
 	virtual bool wide() const { return false; }
 	///
 	int latex(Buffer const  buf, odocstream  os,
Index: src/insets/InsetCaption.cpp
===
--- src/insets/InsetCaption.cpp	(revision 18677)
+++ src/insets/InsetCaption.cpp	(working copy)
@@ -232,6 +232,15 @@
 }
 
 
+bool InsetCaption::notifyCursorLeaves(Cursor  cur) 
+{ 
+	//Fix for 3719?
+	cur.buffer().tocBackend().update();
+	cur.buffer().structureChanged();
+	return false; 
+}
+
+
 int InsetCaption::latex(Buffer const  buf, odocstream  os,
 			OutputParams const  runparams_in) const
 {

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Jean-Marc Lasgouttes]

 Richard == Richard Heck [EMAIL PROTECTED] writes:

Richard This is wrong. updateLabels() IS called from
Richard LFUN_CAPTION_INSERT. But it is NOT called after the caption
Richard itself is edited. So you get the same problem if you change
Richard an existing caption and then view the navigation menu or, for
Richard that matter, the TOC.

Richard The way to solve this might be to put some appropriate code
Richard into InsetCaption::notifyCursorLeaves(). I tried this but
Richard don't have anything that works. (Mostly, I cause crashes.) In
Richard fact, however, this won't quite work anyway, because
Richard notifyCursorLeaves() doesn't seem to be called if the cursor
Richard leaves the inset via keyboard navigation, though it is called
Richard if you move the cursor out with the mouse. That seems wrong,
Richard i.e., it seems like a bug.

Richard Now, my sense from the comment in Inset.h is that
Richard notifyCursorLeaves() wasn't really intended as this kind of
Richard hook. But it is used this way in mathed/InsetMathHull.cpp,
Richard where it's used to call do the preview.

notifyCursorleave is mostly working in mathed. I do not remember the
specifics of how it works, sorry. I would think it should be done at
the same place dEPM is triggered.

JMarc

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Jean-Marc Lasgouttes]

  update toc and list of xxx seems to me an expensive operation. Can
 it be done on demand? I mean, maybe we can add a
 'refresh'/'regenerate' button in the outline panel; and update
 'list of listings' and such only if this menu item is clicked.

This is how it used to be in the good old time, but Abdel decided that
automatic was cooler...

JMarc

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Richard Heck]

Jean-Marc Lasgouttes wrote:

 update toc and list of xxx seems to me an expensive operation. Can
it be done on demand? I mean, maybe we can add a
'refresh'/'regenerate' button in the outline panel; and update
'list of listings' and such only if this menu item is clicked.
  


This is how it used to be in the good old time, but Abdel decided that
automatic was cooler...
  
Cooler, definitely, and this is what users will expect, especially with 
the navigate menu. So there is also the question about how one might 
regenerate the navigation menu. (The call to TocBackend::update() and 
then emitting the structureChanged() signal accomplishes both tasks, so 
far as I can see.) I suppose one could add a menu entry to the 
navigation menu that would regenerate it, too. We'd also need to 
regenerate before doing a goTo().


This is the original bug 3719, which causes lots of problems. For 
example: Open a file with some sections and such in it; open the TOC; go 
to the middle of some section; add a couple paragraphs. Now hit the up 
or down arrow to try to move the section. Nothing happens, except that 
the highlighting moves. If I keep doing this, I can sometimes trigger a 
crash:

/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../../include/c++/4.1.1/debug/vector:199:
   error: attempt to subscript container with out-of-bounds index 35, but
   container only holds 35 elements.

Objects involved in the operation:
sequence this @ 0x0xb54b600 {
 type = N15__gnu_debug_def6vectorIwSaIwEEE;
}
But I don't have a recipe

rh

--
==
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto

[PATCH] Part of Bug 3719 -- TOC out of sync

[Richard Heck]

Richard Heck wrote:

Bo Peng wrote:
Because you are on this topic, can you also check when navigation -> 
list of listings and other entries are updated? When I add a listings 
insets with caption, I can see an empty entry but not the caption.

This is a similar issue. The update is done by this call:
   cbuf.tocBackend().update();
at line 705 of buffer_funcs.cp, in updateLabels(). So if you create a 
new paragraph, again, you get no update and the menu is wrong, but if 
you then backspace, you do get an update and it's right. The question 
is thus why this isn't being called for LFUN_CAPTION_INSERT. I'll try 
to look at that later, or you can do it.
This is wrong. updateLabels() IS called from LFUN_CAPTION_INSERT. But it 
is NOT called after the caption itself is edited. So you get the same 
problem if you change an existing caption and then view the navigation 
menu or, for that matter, the TOC.


The way to solve this might be to put some appropriate code into 
InsetCaption::notifyCursorLeaves(). I tried this but don't have anything 
that works. (Mostly, I cause crashes.) In fact, however, this won't 
quite work anyway, because notifyCursorLeaves() doesn't seem to be 
called if the cursor leaves the inset via keyboard navigation, though it 
is called if you move the cursor out with the mouse. That seems wrong, 
i.e., it seems like a bug.


Now, my sense from the comment in Inset.h is that notifyCursorLeaves() 
wasn't really intended as this kind of hook. But it is used this way in 
mathed/InsetMathHull.cpp, where it's used to call do the preview.


JMarc?

Richard

--
==
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Bo Peng]

This is wrong. updateLabels() IS called from LFUN_CAPTION_INSERT. But it
is NOT called after the caption itself is edited. So you get the same
problem if you change an existing caption and then view the navigation
menu or, for that matter, the TOC.

The way to solve this might be to put some appropriate code into
InsetCaption::notifyCursorLeaves(). I tried this but don't have anything
that works. (Mostly, I cause crashes.) In fact, however, this won't
quite work anyway, because notifyCursorLeaves() doesn't seem to be
called if the cursor leaves the inset via keyboard navigation, though it
is called if you move the cursor out with the mouse. That seems wrong,
i.e., it seems like a bug.


update toc and list of xxx seems to me an expensive operation. Can it
be done on demand? I mean, maybe we can add a 'refresh'/'regenerate'
button in the outline panel; and update 'list of listings' and such
only if this menu item is clicked.

Bo

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Richard Heck]

Bo Peng wrote:

This is wrong. updateLabels() IS called from LFUN_CAPTION_INSERT. But it
is NOT called after the caption itself is edited. So you get the same
problem if you change an existing caption and then view the navigation
menu or, for that matter, the TOC.

The way to solve this might be to put some appropriate code into
InsetCaption::notifyCursorLeaves(). I tried this but don't have anything
that works. (Mostly, I cause crashes.) In fact, however, this won't
quite work anyway, because notifyCursorLeaves() doesn't seem to be
called if the cursor leaves the inset via keyboard navigation, though it
is called if you move the cursor out with the mouse. That seems wrong,
i.e., it seems like a bug.


update toc and list of xxx seems to me an expensive operation. Can it
be done on demand? I mean, maybe we can add a 'refresh'/'regenerate'
button in the outline panel; and update 'list of listings' and such
only if this menu item is clicked.
This kind of update seems to be done quite a lot. Indeed, it seems to me 
that there's really too much updating or menus and toolbars and the like 
going on in general, but it's probably best to be safe here, unless we 
really know what we're doing.


The attached should solve the issue modulo the question when 
notifyCursorLeaves() gets called. I've just borrowed the code from 
ControlToc::updateBackend(). I think it was the missing call to 
structureChanged() that was giving me the crashes I was experiencing.


There is a refresh button in the outline dialog already. It also has the 
effect that it updates the menu.


Richard

--
==
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto

Index: src/insets/InsetCaption.h
===
--- src/insets/InsetCaption.h	(revision 18677)
+++ src/insets/InsetCaption.h	(working copy)
@@ -59,6 +59,8 @@
 	///
 	virtual bool getStatus(Cursor & cur, FuncRequest const & cmd, FuncStatus &) const;
 	///
+	virtual bool notifyCursorLeaves(Cursor &);
+	///
 	virtual bool wide() const { return false; }
 	///
 	int latex(Buffer const & buf, odocstream & os,
Index: src/insets/InsetCaption.cpp
===
--- src/insets/InsetCaption.cpp	(revision 18677)
+++ src/insets/InsetCaption.cpp	(working copy)
@@ -232,6 +232,15 @@
 }
 
 
+bool InsetCaption::notifyCursorLeaves(Cursor & cur) 
+{ 
+	//Fix for 3719?
+	cur.buffer().tocBackend().update();
+	cur.buffer().structureChanged();
+	return false; 
+}
+
+
 int InsetCaption::latex(Buffer const & buf, odocstream & os,
 			OutputParams const & runparams_in) const
 {

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Jean-Marc Lasgouttes]

> "Richard" == Richard Heck <[EMAIL PROTECTED]> writes:

Richard> This is wrong. updateLabels() IS called from
Richard> LFUN_CAPTION_INSERT. But it is NOT called after the caption
Richard> itself is edited. So you get the same problem if you change
Richard> an existing caption and then view the navigation menu or, for
Richard> that matter, the TOC.

Richard> The way to solve this might be to put some appropriate code
Richard> into InsetCaption::notifyCursorLeaves(). I tried this but
Richard> don't have anything that works. (Mostly, I cause crashes.) In
Richard> fact, however, this won't quite work anyway, because
Richard> notifyCursorLeaves() doesn't seem to be called if the cursor
Richard> leaves the inset via keyboard navigation, though it is called
Richard> if you move the cursor out with the mouse. That seems wrong,
Richard> i.e., it seems like a bug.

Richard> Now, my sense from the comment in Inset.h is that
Richard> notifyCursorLeaves() wasn't really intended as this kind of
Richard> hook. But it is used this way in mathed/InsetMathHull.cpp,
Richard> where it's used to call do the preview.

notifyCursorleave is mostly working in mathed. I do not remember the
specifics of how it works, sorry. I would think it should be done at
the same place dEPM is triggered.

JMarc

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Jean-Marc Lasgouttes]

>>  update toc and list of xxx seems to me an expensive operation. Can
>> it be done on demand? I mean, maybe we can add a
>> 'refresh'/'regenerate' button in the outline panel; and update
>> 'list of listings' and such only if this menu item is clicked.

This is how it used to be in the good old time, but Abdel decided that
automatic was cooler...

JMarc

Re: [PATCH] Part of Bug 3719 -- TOC out of sync

[Richard Heck]

Jean-Marc Lasgouttes wrote:

 update toc and list of xxx seems to me an expensive operation. Can
it be done on demand? I mean, maybe we can add a
'refresh'/'regenerate' button in the outline panel; and update
'list of listings' and such only if this menu item is clicked.
  


This is how it used to be in the good old time, but Abdel decided that
automatic was cooler...
  
Cooler, definitely, and this is what users will expect, especially with 
the navigate menu. So there is also the question about how one might 
regenerate the navigation menu. (The call to TocBackend::update() and 
then emitting the structureChanged() signal accomplishes both tasks, so 
far as I can see.) I suppose one could add a menu entry to the 
navigation menu that would regenerate it, too. We'd also need to 
regenerate before doing a goTo().


This is the original bug 3719, which causes lots of problems. For 
example: Open a file with some sections and such in it; open the TOC; go 
to the middle of some section; add a couple paragraphs. Now hit the up 
or down arrow to try to move the section. Nothing happens, except that 
the highlighting moves. If I keep doing this, I can sometimes trigger a 
crash:

/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../../include/c++/4.1.1/debug/vector:199:
   error: attempt to subscript container with out-of-bounds index 35, but
   container only holds 35 elements.

Objects involved in the operation:
sequence "this" @ 0x0xb54b600 {
 type = N15__gnu_debug_def6vectorIwSaIwEEE;
}
But I don't have a recipe

rh

--
==
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto