"Bo Peng" <[EMAIL PROTECTED]> writes:
| > > If not for anything else that the code gets easier to read.
| > > (I do not quite see how removal of id plays into this.)
| > > (And no; operator[pit] on paragraphs won't solve it, just make the
| > > cursor placement even more coarse grained)
|
| Now, a separate
|
Can you please stop adding more and more features to the patch at
every step I just have to begin review again... more comments more
back and forth and then a new patch with new features and round and
round we go...
| Index: src/BufferView_pimpl.C
| ===================================================================
| --- src/BufferView_pimpl.C (revision 13495)
| +++ src/BufferView_pimpl.C (working copy)
| @@ -331,6 +349,9 @@
| // to this buffer later on.
| buffer_->saveCursor(cursor_.selectionBegin(),
| cursor_.selectionEnd());
| + // current buffer is going to be switched-off, save cursor pos
| + LyX::ref().session().saveFilePosition(buffer_->fileName(),
| + boost::tie(cursor_.pit(), cursor_.pos()) );
| }
|
| // If we are closing current buffer, switch to the first in
| @@ -804,13 +825,41 @@
| owner_->message(bformat(_("Moved to bookmark %1$d"), i));
| }
|
| -
| bool BufferView::Pimpl::isSavedPosition(unsigned int i)
| {
| return i < saved_positions_num && !saved_positions[i].filename.empty();
| }
Note that we use two blank lines between funciton definitions.
| +void BufferView::Pimpl::scrollToPosition(Buffer* buffer, lyx::pit_type pit,
lyx::pos_type pos)
| +{
| + // check pit
| + if ( static_cast<size_t>(pit) >= buffer->paragraphs().size() )
| + return;
| +
| + ParIterator it = buffer->par_iterator_begin();
| + ParIterator const end = buffer->par_iterator_end();
| + for (; it != end; ++it)
| + if (it.pit() == pit) {
| + bv_->setCursor(makeDocIterator(it, pos));
| + bv_->update(Update::FitCursor);
| + return;
| + }
| +}
IMO too much was moved out into this function. The finding part is not
separated from the policy part.
| +string const sec_lastfiles = "[recent files]";
| +string const sec_lastfilepos = "[cursor positions]";
| +string const sec_lastopened = "[last opened files]";
| +string const sec_bookmarks = "[bookmarks]";
| +string const sec_misc = "[misc info]";
Please no misc section...
(and you know how I feel about INI files...)
| Index: src/lyxfunc.C
| ===================================================================
| --- src/lyxfunc.C (revision 13495)
| +++ src/lyxfunc.C (working copy)
| @@ -1880,6 +1891,9 @@
|
| void LyXFunc::closeBuffer()
| {
| + // save current cursor position
| + LyX::ref().session().saveFilePosition(owner->buffer()->fileName(),
| + boost::tie(view()->cursor().pit(), view()->cursor().pos()) );
This looks wrong. boost::make_tuple should be used here.
| Index: src/frontends/gtk/GView.C
| ===================================================================
| --- src/frontends/gtk/GView.C (revision 13495)
| +++ src/frontends/gtk/GView.C (working copy)
| @@ -95,7 +99,17 @@
| boost::bind(&GMiniBuffer::editMode, minibuffer_.get()));
| view_state_changed.connect(boost::bind(&GView::showViewState, this));
| signal_focus_in_event().connect(sigc::mem_fun(*this,
&GView::onFocusIn));
| - set_default_size(750, 550);
| + //
| + int width = 750;
| + int height = 550;
| + // try to grab width/height saved from last session
| + string val = LyX::ref().session().loadMiscInfo("WindowWidth");
| + if (val != "")
| + width = convert<unsigned int>(val);
| + val = LyX::ref().session().loadMiscInfo("WindowHeight");
| + if (val != "")
| + height = convert<unsigned int>(val);
| + set_default_size(width, height);
| // Make sure the buttons are disabled if needed.
| updateToolbars();
| string const iconName =
| @@ -117,7 +131,13 @@
Please wait with misc info and window info storing.
--
Lgb
