Re: Fix bug 3092.
Bo Peng wrote: I agree this has nothing to do here but we have to ask this question to the user nevertheless, don't we? So where is this part of the code transferred? quitWriteBuffer() and close() are almost identical. By calling LFUN_BUFFER_CLOSE, close() is called instead. Ahhh... very good then! I think you may proceed with your cleanup but with caution ;-) Abdel.
Re: Fix bug 3092.
Peter Kümmel <[EMAIL PROTECTED]> writes: | Bo Peng wrote: | > What I meant is something like follows. The patch does not work | > correctly yet, but shows my point. | > | > 1. src/bufferlist.C: remove quitWriteBuffer and quitWriteAll. | > | > 2. LFUN_LYX_QUIT triggers LFUN_WINDOW_CLOSE | > | > 3. LFUN_WINDOW_CLOSE triggers LFUN_BUFFER_CLOSE one by one | > | > 4. closeEvent triggers LFUN_LYX_QUIT. | > | > That is to say, no one takes the shortcut quitWriteAll. lyx-close | > experts (Peter, Enrico, Abdel), what do you think? Anything seriously | > wrong here? | | I think, the most important thing when quitting is that there | is one place where all prepare-to-quit code is located. | | Does you patch support this? | How many ways of quitting exit? | 1. menu exit -> LFUN_LYX_QUIT | 2. last window closed -> LFUN_WINDOW_CLOSE -> LFUN_LYX_QUIT and the third: 3. direct LFUN_LYX_QUIT (short-cut, lyx-server...) But I think you are right. All "quitting" should end up in LDUN_LYX_QUIT and IMHO that should handle closing of the rest of the windows and buffers. -- Lgb
Re: Fix bug 3092.
I agree this has nothing to do here but we have to ask this question to the user nevertheless, don't we? So where is this part of the code transferred? quitWriteBuffer() and close() are almost identical. By calling LFUN_BUFFER_CLOSE, close() is called instead. So this means that the user clicked [Cancel] here... But I fail to see where is the code for that. The close() function in bufferlist.C. Bo
Re: Fix bug 3092.
Abdelrazak Younes wrote: > Peter Kümmel wrote: >> Bo Peng wrote: >>> What I meant is something like follows. The patch does not work >>> correctly yet, but shows my point. >>> >>> 1. src/bufferlist.C: remove quitWriteBuffer and quitWriteAll. >>> >>> 2. LFUN_LYX_QUIT triggers LFUN_WINDOW_CLOSE >>> >>> 3. LFUN_WINDOW_CLOSE triggers LFUN_BUFFER_CLOSE one by one >>> >>> 4. closeEvent triggers LFUN_LYX_QUIT. >>> >>> That is to say, no one takes the shortcut quitWriteAll. lyx-close >>> experts (Peter, Enrico, Abdel), what do you think? Anything seriously >>> wrong here? >> >> I think, the most important thing when quitting is that there >> is one place where all prepare-to-quit code is located. >> >> Does you patch support this? >> How many ways of quitting exit? >> 1. menu exit -> LFUN_LYX_QUIT >> 2. last window closed -> LFUN_WINDOW_CLOSE -> LFUN_LYX_QUIT >> >> two only? have I forgotten one? > > You already forgot you Mac experience? It's amazing to see how human > beings manage to erase bad experiences from their memories :-) NO, NO, I haven't forgotten it ;) The problem on the mac was that there is a auto generated menu entry which doesn't triggers LFUN_LYX_QUIT, so it is item 1. > > IIRC there's another way on Mac. Isn't this fixed now? > > Abdel. > Peter
Re: Fix bug 3092.
Peter Kümmel wrote: Bo Peng wrote: What I meant is something like follows. The patch does not work correctly yet, but shows my point. 1. src/bufferlist.C: remove quitWriteBuffer and quitWriteAll. 2. LFUN_LYX_QUIT triggers LFUN_WINDOW_CLOSE 3. LFUN_WINDOW_CLOSE triggers LFUN_BUFFER_CLOSE one by one 4. closeEvent triggers LFUN_LYX_QUIT. That is to say, no one takes the shortcut quitWriteAll. lyx-close experts (Peter, Enrico, Abdel), what do you think? Anything seriously wrong here? I think, the most important thing when quitting is that there is one place where all prepare-to-quit code is located. Does you patch support this? How many ways of quitting exit? 1. menu exit -> LFUN_LYX_QUIT 2. last window closed -> LFUN_WINDOW_CLOSE -> LFUN_LYX_QUIT two only? have I forgotten one? You already forgot you Mac experience? It's amazing to see how human beings manage to erase bad experiences from their memories :-) IIRC there's another way on Mac. Abdel.
Re: Fix bug 3092.
Bo Peng wrote:
> What I meant is something like follows. The patch does not work
> correctly yet, but shows my point.
>
> 1. src/bufferlist.C: remove quitWriteBuffer and quitWriteAll.
>
> 2. LFUN_LYX_QUIT triggers LFUN_WINDOW_CLOSE
>
> 3. LFUN_WINDOW_CLOSE triggers LFUN_BUFFER_CLOSE one by one
>
> 4. closeEvent triggers LFUN_LYX_QUIT.
>
> That is to say, no one takes the shortcut quitWriteAll. lyx-close
> experts (Peter, Enrico, Abdel), what do you think? Anything seriously
> wrong here?
I think, the most important thing when quitting is that there
is one place where all prepare-to-quit code is located.
Does you patch support this?
How many ways of quitting exit?
1. menu exit -> LFUN_LYX_QUIT
2. last window closed -> LFUN_WINDOW_CLOSE -> LFUN_LYX_QUIT
two only? have I forgotten one?
>
> Bo
>
>
>
> Index: src/bufferlist.C
> ===
> --- src/bufferlist.C(revision 16658)
> +++ src/bufferlist.C(working copy)
> @@ -96,76 +96,6 @@
> }
>
>
> -bool BufferList::quitWriteBuffer(Buffer * buf)
> -{
> -BOOST_ASSERT(buf);
> -
> -docstring file;
> -if (buf->isUnnamed())
> -file = from_utf8(onlyFilename(buf->fileName()));
> -else
> -file = makeDisplayPath(buf->fileName(), 30);
> -
> -docstring const text =
> -bformat(_("The document %1$s has unsaved changes.\n\n"
> - "Do you want to save the document or discard the
> changes?"),
> - file);
> -int const ret = Alert::prompt(_("Save changed document?"),
> -text, 0, 2, _("&Save"), _("&Discard"), _("&Cancel"));
> -
> -if (ret == 0) {
> -// FIXME: WriteAs can be asynch !
> -// but not right now...maybe we should remove that
> -
> -bool succeeded;
> -
> -if (buf->isUnnamed())
> -succeeded = writeAs(buf);
> -else
> -succeeded = menuWrite(buf);
> -
> -if (!succeeded)
> -return false;
> -} else if (ret == 1) {
> -// if we crash after this we could
> -// have no autosave file but I guess
> -// this is really inprobable (Jug)
> -if (buf->isUnnamed())
> -removeAutosaveFile(buf->fileName());
> -
> -} else {
> -return false;
> -}
> -
> -return true;
> -}
> -
> -
> -bool BufferList::quitWriteAll()
> -{
> -BufferStorage::iterator it = bstore.begin();
> -BufferStorage::iterator end = bstore.end();
> -for (; it != end; ++it) {
> -if ((*it)->isClean())
> -continue;
> -
> -if (!quitWriteBuffer(*it))
> -return false;
> -}
> -// now, all buffers have been written sucessfully
> -// save file names to .lyx/session
> -it = bstore.begin();
> -for (; it != end; ++it) {
> -// if master/slave are both open, do not save slave since it
> -// will be automatically loaded when the master is loaded
> -if ((*it)->getMasterBuffer() == (*it))
> -
> LyX::ref().session().lastOpened().add(FileName((*it)->fileName()));
> -}
> -
> -return true;
> -}
> -
> -
> void BufferList::release(Buffer * buf)
> {
> BOOST_ASSERT(buf);
> Index: src/bufferlist.h
> ===
> --- src/bufferlist.h(revision 16658)
> +++ src/bufferlist.h(working copy)
> @@ -42,9 +42,6 @@
> iterator end();
> const_iterator end() const;
>
> -/// write all buffers, asking the user, returns false if cancelled
> -bool quitWriteAll();
> -
> /// create a new buffer
> Buffer * newBuffer(std::string const & s, bool ronly = false);
>
> @@ -104,8 +101,6 @@
> void setCurrentAuthor(docstring const & name, docstring const & email);
>
> private:
> -/// ask to save a buffer on quit, returns false if should cancel
> -bool quitWriteBuffer(Buffer * buf);
>
> typedef std::vector BufferStorage;
>
> Index: src/lyxfunc.C
> ===
> --- src/lyxfunc.C(revision 16658)
> +++ src/lyxfunc.C(working copy)
> @@ -1074,7 +1074,8 @@
> case LFUN_LYX_QUIT:
> // quitting is triggered by the gui code
> // (leaving the event loop).
> -if (theBufferList().quitWriteAll())
> +dispatch(FuncRequest(LFUN_WINDOW_CLOSE));
> +if (theBufferList().empty())
> theApp()->gui().closeAllViews();
> break;
>
> @@ -1703,13 +1704,19 @@
> case LFUN_WINDOW_CLOSE:
> BOOST_ASSERT(lyx_view_);
> BOOST_ASSERT(theApp());
> -// update bookmark pit of the current buffer before window
> close
> -for (size_t i = 0; i <
> LyX::ref().session().bookmarks().size(); ++i)
> -gotoBookmark(i+1, false, false);
> -// ask the user for saving changes or cancel quit
> -if (!theBufferList().quitWriteAll())
> -
Re: Fix bug 3092.
Bo Peng wrote:
What I meant is something like follows. The patch does not work
correctly yet, but shows my point.
1. src/bufferlist.C: remove quitWriteBuffer and quitWriteAll.
This part is good. The BufferList should not care about quitting and
asking for user interaction.
2. LFUN_LYX_QUIT triggers LFUN_WINDOW_CLOSE
Good in theory but you have to make sure that it works equally well on
the three supported platform.
3. LFUN_WINDOW_CLOSE triggers LFUN_BUFFER_CLOSE one by one
Be careful here, we should do that only if it is the last window.
4. closeEvent triggers LFUN_LYX_QUIT.
Only if it is the last window.
That is to say, no one takes the shortcut quitWriteAll. lyx-close
experts (Peter, Enrico, Abdel), what do you think? Anything seriously
wrong here?
See above. More comments below:
Index: src/bufferlist.C
===
--- src/bufferlist.C(revision 16658)
+++ src/bufferlist.C(working copy)
@@ -96,76 +96,6 @@
}
-bool BufferList::quitWriteBuffer(Buffer * buf)
-{
-BOOST_ASSERT(buf);
-
-docstring file;
-if (buf->isUnnamed())
-file = from_utf8(onlyFilename(buf->fileName()));
-else
-file = makeDisplayPath(buf->fileName(), 30);
-
-docstring const text =
-bformat(_("The document %1$s has unsaved changes.\n\n"
- "Do you want to save the document or discard the
changes?"),
- file);
-int const ret = Alert::prompt(_("Save changed document?"),
-text, 0, 2, _("&Save"), _("&Discard"), _("&Cancel"));
I agree this has nothing to do here but we have to ask this question to
the user nevertheless, don't we? So where is this part of the code
transferred?
[...]
-bool BufferList::quitWriteAll()
-{
-BufferStorage::iterator it = bstore.begin();
-BufferStorage::iterator end = bstore.end();
-for (; it != end; ++it) {
-if ((*it)->isClean())
-continue;
-
-if (!quitWriteBuffer(*it))
-return false;
-}
-// now, all buffers have been written sucessfully
-// save file names to .lyx/session
-it = bstore.begin();
-for (; it != end; ++it) {
-// if master/slave are both open, do not save slave since it
-// will be automatically loaded when the master is loaded
-if ((*it)->getMasterBuffer() == (*it))
Same question here. Where is that transferred? I am probably missing
something.
Index: src/lyxfunc.C
===
--- src/lyxfunc.C(revision 16658)
+++ src/lyxfunc.C(working copy)
@@ -1074,7 +1074,8 @@
case LFUN_LYX_QUIT:
// quitting is triggered by the gui code
// (leaving the event loop).
-if (theBufferList().quitWriteAll())
+dispatch(FuncRequest(LFUN_WINDOW_CLOSE));
Here you have to call LFUN_WINDOW_CLOSE for every window. The list is
given to you by the Gui class.
+if (theBufferList().empty())
theApp()->gui().closeAllViews();
This Gui::closeAllViews() is redundant here if you have already closed
all the windows (view == window).
break;
@@ -1703,13 +1704,19 @@
case LFUN_WINDOW_CLOSE:
BOOST_ASSERT(lyx_view_);
BOOST_ASSERT(theApp());
-// update bookmark pit of the current buffer before window
close
-for (size_t i = 0; i <
LyX::ref().session().bookmarks().size(); ++i)
-gotoBookmark(i+1, false, false);
-// ask the user for saving changes or cancel quit
-if (!theBufferList().quitWriteAll())
-break;
-lyx_view_->close();
+// close buffer one by one
Be careful here, only if it is the last window.
+while (true) {
+string const file = lyx_view_->buffer()->fileName();
+dispatch(FuncRequest(LFUN_BUFFER_CLOSE));
+// all files closed
+if (theBufferList().empty()) {
+lyx_view_->close();
+break;
+// buffer close failed
+if (file == lyx_view_->buffer()->fileName())
+break;
+}
+}
return;
case LFUN_BOOKMARK_GOTO:
Index: src/frontends/qt4/GuiView.C
===
--- src/frontends/qt4/GuiView.C(revision 16658)
+++ src/frontends/qt4/GuiView.C(working copy)
@@ -237,7 +237,8 @@
// we may have been called through the close window button
// which bypasses the LFUN machinery.
if (!quitting_by_menu_) {
-if (!theBufferList().quitWriteAll()) {
+dispatch(FuncRequest(LFUN_LYX_QUIT));
Good in theory but you have to make sure that it works equally well on
the three supported platform.
+if (!theBufferList().empty()) {
So this means that the user clicke
Re: Fix bug 3092.
I'll have a look at your patch but at first glance I think you missed the mutiple-view case entirely. I know. That is only a proof of concept patch. Thank you for you attention, I was just irritated that I had to hang my session stuff everywhere instead of doing it on a per-buffer basis. Bo
Re: Fix bug 3092.
Bo Peng wrote: I think I am no lyx-close expert, but in principle your point is correct. Only, please make sure that it works in practice ;-) I know, but I am not familiar with the quit logic, and there are so many ways to quit lyx, plus some mac specialties ... Yes. This is very hairy stuff, don't underestimate the difficulty. I'll have a look at your patch but at first glance I think you missed the mutiple-view case entirely. Abdel.
Re: Fix bug 3092.
I think I am no lyx-close expert, but in principle your point is correct. Only, please make sure that it works in practice ;-) I know, but I am not familiar with the quit logic, and there are so many ways to quit lyx, plus some mac specialties ... I guess my biggest concern is that why we have quitWriteBuffer in the first place. Bo
Re: Fix bug 3092.
On Fri, Jan 12, 2007 at 09:22:00PM +1800, Bo Peng wrote: > What I meant is something like follows. The patch does not work > correctly yet, but shows my point. > > 1. src/bufferlist.C: remove quitWriteBuffer and quitWriteAll. > > 2. LFUN_LYX_QUIT triggers LFUN_WINDOW_CLOSE > > 3. LFUN_WINDOW_CLOSE triggers LFUN_BUFFER_CLOSE one by one > > 4. closeEvent triggers LFUN_LYX_QUIT. > > That is to say, no one takes the shortcut quitWriteAll. lyx-close > experts (Peter, Enrico, Abdel), what do you think? Anything seriously > wrong here? I think I am no lyx-close expert, but in principle your point is correct. Only, please make sure that it works in practice ;-) -- Enrico
Re: Fix bug 3092.
What I meant is something like follows. The patch does not work
correctly yet, but shows my point.
1. src/bufferlist.C: remove quitWriteBuffer and quitWriteAll.
2. LFUN_LYX_QUIT triggers LFUN_WINDOW_CLOSE
3. LFUN_WINDOW_CLOSE triggers LFUN_BUFFER_CLOSE one by one
4. closeEvent triggers LFUN_LYX_QUIT.
That is to say, no one takes the shortcut quitWriteAll. lyx-close
experts (Peter, Enrico, Abdel), what do you think? Anything seriously
wrong here?
Bo
Index: src/bufferlist.C
===
--- src/bufferlist.C(revision 16658)
+++ src/bufferlist.C(working copy)
@@ -96,76 +96,6 @@
}
-bool BufferList::quitWriteBuffer(Buffer * buf)
-{
- BOOST_ASSERT(buf);
-
- docstring file;
- if (buf->isUnnamed())
- file = from_utf8(onlyFilename(buf->fileName()));
- else
- file = makeDisplayPath(buf->fileName(), 30);
-
- docstring const text =
- bformat(_("The document %1$s has unsaved changes.\n\n"
- "Do you want to save the document or discard
the changes?"),
- file);
- int const ret = Alert::prompt(_("Save changed document?"),
- text, 0, 2, _("&Save"), _("&Discard"), _("&Cancel"));
-
- if (ret == 0) {
- // FIXME: WriteAs can be asynch !
- // but not right now...maybe we should remove that
-
- bool succeeded;
-
- if (buf->isUnnamed())
- succeeded = writeAs(buf);
- else
- succeeded = menuWrite(buf);
-
- if (!succeeded)
- return false;
- } else if (ret == 1) {
- // if we crash after this we could
- // have no autosave file but I guess
- // this is really inprobable (Jug)
- if (buf->isUnnamed())
- removeAutosaveFile(buf->fileName());
-
- } else {
- return false;
- }
-
- return true;
-}
-
-
-bool BufferList::quitWriteAll()
-{
- BufferStorage::iterator it = bstore.begin();
- BufferStorage::iterator end = bstore.end();
- for (; it != end; ++it) {
- if ((*it)->isClean())
- continue;
-
- if (!quitWriteBuffer(*it))
- return false;
- }
- // now, all buffers have been written sucessfully
- // save file names to .lyx/session
- it = bstore.begin();
- for (; it != end; ++it) {
- // if master/slave are both open, do not save slave since it
- // will be automatically loaded when the master is loaded
- if ((*it)->getMasterBuffer() == (*it))
-
LyX::ref().session().lastOpened().add(FileName((*it)->fileName()));
- }
-
- return true;
-}
-
-
void BufferList::release(Buffer * buf)
{
BOOST_ASSERT(buf);
Index: src/bufferlist.h
===
--- src/bufferlist.h(revision 16658)
+++ src/bufferlist.h(working copy)
@@ -42,9 +42,6 @@
iterator end();
const_iterator end() const;
- /// write all buffers, asking the user, returns false if cancelled
- bool quitWriteAll();
-
/// create a new buffer
Buffer * newBuffer(std::string const & s, bool ronly = false);
@@ -104,8 +101,6 @@
void setCurrentAuthor(docstring const & name, docstring const & email);
private:
- /// ask to save a buffer on quit, returns false if should cancel
- bool quitWriteBuffer(Buffer * buf);
typedef std::vector BufferStorage;
Index: src/lyxfunc.C
===
--- src/lyxfunc.C (revision 16658)
+++ src/lyxfunc.C (working copy)
@@ -1074,7 +1074,8 @@
case LFUN_LYX_QUIT:
// quitting is triggered by the gui code
// (leaving the event loop).
- if (theBufferList().quitWriteAll())
+ dispatch(FuncRequest(LFUN_WINDOW_CLOSE));
+ if (theBufferList().empty())
theApp()->gui().closeAllViews();
break;
@@ -1703,13 +1704,19 @@
case LFUN_WINDOW_CLOSE:
BOOST_ASSERT(lyx_view_);
BOOST_ASSERT(theApp());
- // update bookmark pit of the current buffer before
window close
- for (size_t i = 0; i <
LyX::ref().session().bookmarks().size(); ++i)
- gotoBookmark(i+1, false, false);
- // ask the user for saving changes or cancel quit
- if (!theBufferList().quitWriteAll())
- break;
- lyx_view_->close(
Re: Fix bug 3092.
> We should definitely do that. I fear another quit cleanup saga... For example, if you look at bufferlist.C, we have close, quiteWriteBuffer, and quiteWriteAll. As far as I can tell, close = quiteWriteBuffer, and quiteWriteAll should be removed (call LFUN_BUFFER_CLOSE one by one). Agreed? Bo
Re: Fix bug 3092.
On Thu, Jan 11, 2007 at 04:43:37PM +0100, Jean-Marc Lasgouttes wrote: > > "Bo" == Bo Peng <[EMAIL PROTECTED]> writes: > > Bo> You can see that I update bookmark when buffer switch, buffer > Bo> close, windows close, but I still can not update bookmark when > Bo> File->exit is triggered. > > Bo> WHY CANNOT WE take the time to fire LFUN_BUFFER_CLOSE, and > Bo> LFUN_WINDOW_CLOSE when lyx exists? When lyx takes shortcuts to > Bo> quit, proper cleanup in these CLOSE events may be bypassed. > > We should definitely do that. I fear another quit cleanup saga... -- Enrico
Re: Fix bug 3092.
It looks more complicated than I thought it would be, but I guess it is OK. I though it would be a five minutes jobs, but it took me two hours. Both id and pit can change so I have to use and update both of them when necessary. :-( The patch is being cleaned, and will be in in a few minutes. Cheers, Bo
Re: Fix bug 3092.
> "Bo" == Bo Peng <[EMAIL PROTECTED]> writes: Bo> You can see that I update bookmark when buffer switch, buffer Bo> close, windows close, but I still can not update bookmark when Bo> File->exit is triggered. Bo> WHY CANNOT WE take the time to fire LFUN_BUFFER_CLOSE, and Bo> LFUN_WINDOW_CLOSE when lyx exists? When lyx takes shortcuts to Bo> quit, proper cleanup in these CLOSE events may be bypassed. We should definitely do that. JMarc
Re: Fix bug 3092.
> "Bo" == Bo Peng <[EMAIL PROTECTED]> writes: Bo> On 1/11/07, Bo Peng <[EMAIL PROTECTED]> wrote: >> This patch uses both pit and id to locate a bookmark. Bo> If there is no objection, it is going in soon. I will fix the file-> exit/bookmark update part later. It looks more complicated than I thought it would be, but I guess it is OK. JMarc
Re: Fix bug 3092.
On 1/11/07, Bo Peng <[EMAIL PROTECTED]> wrote: This patch uses both pit and id to locate a bookmark. If there is no objection, it is going in soon. I will fix the file->exit/bookmark update part later. Cheers, Bo
Re: Fix bug 3092.
Given that quitWriteAll() is called in all the cases above, maybe you can stick there your code? Without looking at the code, maybe, 1. quiteWriteAll() may not have a cursor. (Can be called in non-GUI mode?) 2. I will have to switch to a buffer, set cursor to get the pid. It is much easier to do this at buffer switch and buffer close. I mean, at the buffer level. Bo
Re: Fix bug 3092.
On Thu, Jan 11, 2007 at 02:29:27PM +1800, Bo Peng wrote: > > Maybe I don't know what you exactly mean, but whatever the method > > you use to quit (menu or click on the X in the right upper corner), > > GuiView::closeEvent() is called. There you can already find some > > code for saving geometry and cursor position of opened files. > > What I meant was that GuiView::closeEvent should trigger > LFUN_WINDOW_CLOSE, and LFUN_BUFFER_CLOSE and let them handle file > save, close and clean up. In this way, GUI level cleanup can be done > once in one place. > > To be more specific, I update bookmark in LFUN_BUFFER_CLOSE. This is > the only logical place to 'finally check bookmark pit before a buffer > is closed'. However, I also need to update bookmarks in > LFUN_WINDOW_CLOSE and maybe GuiView::closeEvent. > > It does not make sense to me at all to call 'quitAndCloseAll' > (something like that), instead of sending LFUN_BUFFER_CLOSE to each > opened buffer. Given that quitWriteAll() is called in all the cases above, maybe you can stick there your code? -- Enrico
Re: Fix bug 3092.
Maybe I don't know what you exactly mean, but whatever the method you use to quit (menu or click on the X in the right upper corner), GuiView::closeEvent() is called. There you can already find some code for saving geometry and cursor position of opened files. What I meant was that GuiView::closeEvent should trigger LFUN_WINDOW_CLOSE, and LFUN_BUFFER_CLOSE and let them handle file save, close and clean up. In this way, GUI level cleanup can be done once in one place. To be more specific, I update bookmark in LFUN_BUFFER_CLOSE. This is the only logical place to 'finally check bookmark pit before a buffer is closed'. However, I also need to update bookmarks in LFUN_WINDOW_CLOSE and maybe GuiView::closeEvent. It does not make sense to me at all to call 'quitAndCloseAll' (something like that), instead of sending LFUN_BUFFER_CLOSE to each opened buffer. Bo
Re: Fix bug 3092.
On Thu, Jan 11, 2007 at 12:18:19PM +1800, Bo Peng wrote: > WHY CANNOT WE take the time to fire LFUN_BUFFER_CLOSE, and > LFUN_WINDOW_CLOSE when lyx exists? When lyx takes shortcuts to quit, > proper cleanup in these CLOSE events may be bypassed. Maybe I don't know what you exactly mean, but whatever the method you use to quit (menu or click on the X in the right upper corner), GuiView::closeEvent() is called. There you can already find some code for saving geometry and cursor position of opened files. -- Enrico
Fix bug 3092.
This patch uses both pit and id to locate a bookmark.
1. both pit and id are used to locate a position. Pit is saved by session.
2. moveToPosition now returns id and pit of the new position and
update bookmark if necessary. These happens:
* when id is invalid (Uwe's case), pit is used, and bookmark id is updated.
* when a paragraph is added/removed, id can still locate the
paragraph but pit needs to be updated.
3. when a buffer/window is closed, its bookmarks are updated (by going
to the bookmarks and get their pit.) This avoids the following
problem: set bookmark; go to the begining of the buffer add a few
paragraphs (id ok, pit changed); close buffer; open the buffer using
the bookmark, bookmark is shifted because id is now invalid, pit is
shifted.
You can see that I update bookmark when buffer switch, buffer close,
windows close, but I still can not update bookmark when File->exit is
triggered.
WHY CANNOT WE take the time to fire LFUN_BUFFER_CLOSE, and
LFUN_WINDOW_CLOSE when lyx exists? When lyx takes shortcuts to quit,
proper cleanup in these CLOSE events may be bypassed.
Bo
Index: src/session.C
===
--- src/session.C (revision 16638)
+++ src/session.C (working copy)
@@ -244,12 +244,12 @@
try {
// read bookmarks
- // id, pos, file\n
- unsigned int id;
+ // pit, pos, file\n
+ pit_type pit;
pos_type pos;
string fname;
istringstream itmp(tmp);
- itmp >> id;
+ itmp >> pit;
itmp.ignore(2); // ignore ", "
itmp >> pos;
itmp.ignore(2); // ignore ", "
@@ -261,7 +261,7 @@
if (fs::exists(file.toFilesystemEncoding()) &&
!fs::is_directory(file.toFilesystemEncoding()) &&
bookmarks.size() < max_bookmarks)
-bookmarks.push_back(Bookmark(file, id, pos));
+bookmarks.push_back(Bookmark(file, pit, 0, pos));
else
lyxerr[Debug::INIT] << "LyX: Warning: Ignore bookmark of file: " << fname << endl;
} catch (...) {
@@ -275,22 +275,22 @@
{
os << '\n' << sec_bookmarks << '\n';
for (size_t i = 0; i < bookmarks.size(); ++i) {
- os << bookmarks[i].par_id << ", "
+ os << bookmarks[i].par_pit << ", "
<< bookmarks[i].par_pos << ", "
<< bookmarks[i].filename << '\n';
}
}
-void BookmarksSection::save(FileName const & fname, int par_id, pos_type par_pos, bool persistent)
+void BookmarksSection::save(FileName const & fname, pit_type par_pit, int par_id, pos_type par_pos, bool persistent)
{
if (persistent) {
- bookmarks.push_back(Bookmark(fname, par_id, par_pos));
+ bookmarks.push_back(Bookmark(fname, par_pit, par_id, par_pos));
if (bookmarks.size() > max_bookmarks)
bookmarks.pop_back();
}
else
- temp_bookmark = Bookmark(fname, par_id, par_pos);
+ temp_bookmark = Bookmark(fname, par_pit, par_id, par_pos);
}
Index: src/lyxfunc.C
===
--- src/lyxfunc.C (revision 16638)
+++ src/lyxfunc.C (working copy)
@@ -242,6 +242,42 @@
}
+void LyXFunc::gotoBookmark(unsigned int idx, bool openFile, bool switchToBuffer)
+{
+ BOOST_ASSERT(lyx_view_);
+ if (!LyX::ref().session().bookmarks().isValid(idx))
+ return;
+ BookmarksSection::Bookmark const & bm = LyX::ref().session().bookmarks().bookmark(idx);
+ BOOST_ASSERT(!bm.filename.empty());
+ string const file = bm.filename.absFilename();
+ // if the file is not opened, open it.
+ if (!theBufferList().exists(file)) {
+ if (openFile)
+ dispatch(FuncRequest(LFUN_FILE_OPEN, file));
+ else
+ return;
+ }
+ // open may fail, so we need to test it again
+ if (theBufferList().exists(file)) {
+ // if the current buffer is not that one, switch to it.
+ if (lyx_view_->buffer()->fileName() != file) {
+ if (switchToBuffer)
+dispatch(FuncRequest(LFUN_BUFFER_SWITCH, file));
+ else
+return;
+ }
+ // moveToPosition use par_id, and par_pit and return new par_id.
+ pit_type new_pit;
+ int new_id;
+ boost::tie(new_pit, new_id) = view()->moveToPosition(bm.par_pit, bm.par_id, bm.par_pos);
+ // if par_id or pit has been changed, reset par_id
+ // see http://bugzilla.lyx.org/show_bug.cgi?id=3092
+ if (bm.par_pit != new_pit || bm.par_id != new_id)
+ const_cast(bm).setPos(new_pit, new_id);
+ }
+}
+
+
void LyXFunc::processKeySym(LyXKeySymPtr keysym, key_modifier::state state)
{
lyxerr[Debug::KEY] << "KeySym is " << keysym->getSymbolName() << endl;
@@ -1126,6 +1162,9 @@
// --- buffers
case LFUN_BUFFER_SWITCH:
BOOST_ASSERT(lyx_view_);
+ // update bookmark pit.
+ for (size_t i = 0; i < LyX::ref().session().bookmarks().size(); ++i)
+gotoBookmark(i+1, false, false);
lyx_view_->setBuffer(theBufferList().getBuffer(argument));
break;
@@ -1664,33 +1703,19 @@
case LFUN_WINDOW_CLOSE:
BOOST_ASSERT(lyx_view_);
BOOST_ASSERT(theApp());
+ // update bookmark pit.
+ for (size_t i = 0; i < LyX::ref().session().bookmarks().size(); ++i)
+gotoBookmark(i+1, false,
