Re: [Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Georg Baum]

Martin Vermeer wrote:

 On Wed, 2006-03-22 at 23:19 -0600, Bo Peng wrote:
 Dear list,
 
 It does not look so difficult to implement this feature so I tried to
 implement it by myself. Attached is my patch. Since I rarely submit a
 patch here so I am afraid that there might be many problems (format,
 logic, bugs). Please have a look and give me some feedbacks.
 
 Format looks OK. The logic is a bit ad-hoc, but that comes with the
 territory.
 
 Others may give detailed feedback on the code.
 
 Many files have been modified:
 
 src/BufferView_pimpl.C
 When a new buffer is loaded, it tries to get last cursor position from
 ref().lastfiles(). When a file is going to be closed, current location
 is saved as bookmark 0, and to ref().lastfiles(). A side effect of
 this is that a file can be re-opened (and to the same location) from
 bookmark 0.

I think that this is a very sensible approach. I would avoid to store this
information in the lyx file itself (privacy). Currently ezxchanging .lyx
files is safe because they don't carry aany hidden information, and I would
like it to stay like that.

 src/lastfiles.C:
 I do not want to change the Files structure so I use a map to store
 position information. lastfile format is changed to id, pos
 filename if position information is available, and filename if not.
 
 Would it be easier to always have the id, pos filename format? And id
 = pos = 0 if no valid info exists, so current behaviour is reproduced?

I agree.

 1. Will invalid paragraph id, pos crash lyx? I mean, if a file is
 saved (with id, pos saved locally), and is modified externally (get a
 revised copy from others), id, pos will no longer be valid and goto
 bookmark 0 will fail.

This should be explicitly handled after the file is read: Check for invalid
bookmarks, and delete them.

 This is a file format change and lyx2lyx must be made to handle it.

Why? I dont' see a format change.

 2. Save position does not seem to work when I quit lyx by closing the
 lyx window. Only close-buffer will do. Where should I handle the close
 event?
 
 Don't bother. That's not the way to leave LyX.

I disagree. I once had to tutor a lab course where a program was used that
did strange things if it was closed via thw close button. It was very hard
to get the students used to that. IMO the close button should t=do exactly
the same as File-Quit. Everything else would confuse users.


Georg

Re: [Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

  src/lastfiles.C:
  I do not want to change the Files structure so I use a map to store
  position information. lastfile format is changed to id, pos
  filename if position information is available, and filename if not.

 Would it be easier to always have the id, pos filename format? And id
 = pos = 0 if no valid info exists, so current behaviour is reproduced?

With that I want to have a smooth transition to people that already
have a .lyx/lastfile. I is enough though, to always write in name
format, but also handle name when read.


  1. Will invalid paragraph id, pos crash lyx? I mean, if a file is
  saved (with id, pos saved locally), and is modified externally (get a
  revised copy from others), id, pos will no longer be valid and goto
  bookmark 0 will fail.

 This is a file format change and lyx2lyx must be made to handle it.

Is $HOME/.lyx/lastfile also handled by lyx2lyx?

 +// not found, return invalid id number
 +   else {
 +   id = -1;
 +   pos = 0;
 +   }

 Please don't do this ;-)

 It won't be needed if we change and handle the new file format.

There is a problem with my current approach. LastFiles::readFile only
read num_files entries. If an older file is re-opened, the position
information will not be available. That is why I currently use this
'info not found' approach. It should be better to load all position
information regardless of num_files.


  2. Save position does not seem to work when I quit lyx by closing the
  lyx window. Only close-buffer will do. Where should I handle the close
  event?

 Don't bother. That's not the way to leave LyX.

Many people do this anyway.

  3. Is saved_positions[0] really unused?

 No, it's used in at least two other places.

 1) To save the main document position to return to when opening a child
 2) To save the return (ref) position when jumping to a label.

 Neither interferes with your use, I think.

That means the first menu entry will/should be disabled after such operations.

Bo

Re: [Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

An updated patch that loads all position information while keeping
only num_files Files entries.

Question: is 0 a valid paragraph ID? I currently use -1 to indicate invalid ID.

Bo
Index: src/BufferView_pimpl.C
===
--- src/BufferView_pimpl.C	(revision 13463)
+++ src/BufferView_pimpl.C	(working copy)
@@ -293,6 +293,14 @@
 
 	setBuffer(b);
 	bv_-showErrorList(_(Parse));
+
+	// load position when the file was last closed
+	int id;
+	lyx::pos_type pos; 
+	LyX::ref().lastfiles().loadFilePosition(s, id, pos);
+	// if id is valid ...
+	if( id = 0 )
+		saved_positions[0] = Position(s, id, pos);
 
 	if (tolastfiles)
 		LyX::ref().lastfiles().newFile(b-fileName());
@@ -767,7 +775,14 @@
 	saved_positions[i] = Position(buffer_-fileName(),
   cursor_.paragraph().id(),
   cursor_.pos());
-	if (i  0)
+	// if i == 0, (called when this buffer is closed), 
+	// save current position to lastfile
+	if (i == 0) {
+		LyX::ref().lastfiles().saveFilePosition(buffer_-fileName(),
+cursor_.paragraph().id(), 
+cursor_.pos());
+}
+	else
 		owner_-message(bformat(_(Saved bookmark %1$d), i));
 }
 
Index: src/lastfiles.C
===
--- src/lastfiles.C	(revision 13463)
+++ src/lastfiles.C	(working copy)
@@ -19,6 +19,11 @@
 #include fstream
 #include iterator
 
+// file_pos is a map to save position of cursor when a file is closed.
+#include map
+#include sstream
+#include utility
+
 namespace fs = boost::filesystem;
 
 using std::copy;
@@ -26,10 +31,16 @@
 using std::find;
 using std::getline;
 using std::string;
+using std::map;
+using std::pair;
 using std::ifstream;
 using std::ofstream;
 using std::ostream_iterator;
 
+// store file position information to a map to avoid changing the 
+// dequeue structure Files
+typedef mapstring, pairint, lyx::pos_type  file_pos;
+file_pos filePositions;
 
 LastFiles::LastFiles(string const  filename, bool st, unsigned int num)
 	: dostat(st)
@@ -59,10 +70,31 @@
 	ifstream ifs(filename.c_str());
 	string tmp;
 
-	while (getline(ifs, tmp)  files.size()  num_files) {
+	while (getline(ifs, tmp)) {
+		// id, position filename\n
+		if (tmp[0] == ''  tmp.find('', 1) != string::npos ) {
+			std::istringstream itmp(tmp);
+			string fname;
+			int id;
+			lyx::pos_type pos;
+			itmp.ignore(1);  // ignore 
+			itmp  id;
+			itmp.ignore(2);  // ignore , 
+			itmp  pos;
+			itmp.ignore(2);  // ingore  
+			itmp  fname;
+			filePositions[fname] = pairint, lyx::pos_type(id, pos);
+			tmp = fname;
+		}
+		// 'tmp' is now the last part of id, pos file or a line 
+		// without . The latter case provides a smooth transition 
+		// for people who have the old lastfile format
 		if (dostat  !fs::exists(tmp))
-continue;
-		files.push_back(tmp);
+			continue;
+		// while loading all position information, 
+		// keep num_files files entries
+		if (files.size()  num_files )
+			files.push_back(tmp);
 	}
 }
 
@@ -71,8 +103,19 @@
 {
 	ofstream ofs(filename.c_str());
 	if (ofs) {
-		copy(files.begin(), files.end(),
-		 ostream_iteratorstring(ofs, \n));
+		for (const_iterator file=files.begin(); file != files.end(); ++file) {
+			// has position information, save in the format of 
+			// id, pos filename
+			file_pos::iterator entry = filePositions.find(*file);
+			if (entry != filePositions.end() )
+ofs(*entry).second.first  ,   
+	(*entry).second.second *file  endl;
+			// this will only happen during the transition from the
+			// old to new lastfile format
+			else
+ofs  -1, 0   *file  endl;
+		}
+		ofs.close();
 	} else
 		lyxerr  LyX: Warning: unable to save LastFiles: 
 		filename  endl;
@@ -90,7 +133,27 @@
 		files.pop_back();
 }
 
+void LastFiles::saveFilePosition(string const fname, int id, lyx::pos_type pos ) const
+{
+	filePositions[fname] = pairint, lyx::pos_type(id, pos);
+}
 
+void LastFiles::loadFilePosition(string const fname, int id, lyx::pos_type pos ) const
+{
+	file_pos::iterator entry = filePositions.find(fname);
+	// has position information, return it.
+	if( entry != filePositions.end() ) {
+		id = (*entry).second.first;
+		pos = (*entry).second.second;
+	}
+	// not found, return invalid id number. This will happen 
+	// in the cases when a new file (not in lastfile) is opened.
+	else {
+		id = -1;
+		pos = 0;
+	}
+}
+
 string const LastFiles::operator[](unsigned int i) const
 {
 	if (i  files.size())
Index: src/lyxfunc.C
===
--- src/lyxfunc.C	(revision 13463)
+++ src/lyxfunc.C	(working copy)
@@ -1880,6 +1880,8 @@
 
 void LyXFunc::closeBuffer()
 {
+	// save current position to lastfile
+	view()-savePosition(0);
 	if (bufferlist.close(owner-buffer(), true)  !quitting) {
 		if (bufferlist.empty()) {
 			// need this otherwise SEGV may occur while
Index: src/lastfiles.h
===
--- 

Re: [Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Bo Peng [EMAIL PROTECTED] writes:

| Dear list,
| 
| It does not look so difficult to implement this feature so I tried to
| implement it by myself. Attached is my patch. Since I rarely submit a
| patch here so I am afraid that there might be many problems (format,
| logic, bugs). Please have a look and give me some feedbacks.
| 
| Many files have been modified:
| 
| src/BufferView_pimpl.C
| When a new buffer is loaded, it tries to get last cursor position from
| ref().lastfiles(). When a file is going to be closed, current location
| is saved as bookmark 0, and to ref().lastfiles(). A side effect of
| this is that a file can be re-opened (and to the same location) from
| bookmark 0.

I am not sure if I like lastfiles to be the files that store this
info. One of the reason is that we only store a very limited number of
files there... lastpostion info should hold more files.

I'd prefere if there was a separate file for this information.

bookmark0? where is that stored? if it in the lyx document, then
please no.

Also I think bookmarks and lastplace should be kept a part and not be
connected at all.

| src/lastfiles.C:
| I do not want to change the Files structure so I use a map to store
| position information. lastfile format is changed to id, pos
| filename if position information is available, and filename if not.
| The biggest changes are to read/write lastfile. I hope that I have not
| broken anything here.

With a new file with its own format, this is not a problem. Also
please have a look at how emacs does this. (.emacs-places)
 
| src/lyxfunc.C:
|save cursor position when the buffer is closed.
| 
| lib/ui/stdmenus.ui:
|add a menu item with an awkard name.

What do you need the menuitem for?
 
| I am worrying about two problem though:
| 
| 1. Will invalid paragraph id, pos crash lyx? I mean, if a file is
| saved (with id, pos saved locally), and is modified externally (get a
| revised copy from others), id, pos will no longer be valid and goto
| bookmark 0 will fail.

Please do not use paragraph id it is not stable. Perhaps use the
cursor stack info instead.
 
| 2. Save position does not seem to work when I quit lyx by closing the
| lyx window. Only close-buffer will do. Where should I handle the close
| event?

That would be a more general bug.

| 3. Is saved_positions[0] really unused?

Where is it found?

-- 
Lgb

Re: [Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Bo Peng [EMAIL PROTECTED] writes:

| An updated patch that loads all position information while keeping
| only num_files Files entries.
| 
| Question: is 0 a valid paragraph ID? I currently use -1 to indicate
| invalid ID.

As said, I'd prefere a separate file in .lyx/; but even if I didn't:

| Index: src/BufferView_pimpl.C
| ===
| --- src/BufferView_pimpl.C(revision 13463)
| +++ src/BufferView_pimpl.C(working copy)
| @@ -293,6 +293,14 @@
|  
|   setBuffer(b);
|   bv_-showErrorList(_(Parse));
| +
| + // load position when the file was last closed
| + int id;
| + lyx::pos_type pos; 
| + LyX::ref().lastfiles().loadFilePosition(s, id, pos);

I must admit that I prefere to not use in/out parameters.
I like tupples though :-)  (boost::tuple)

| + // if id is valid ...
| + if( id = 0 )
| + saved_positions[0] = Position(s, id, pos);

I think we should just have a switch. LyX wide per user where (lyxrc)
the decides where the lastposition info should be used or not.

| + if (i == 0) {
| + LyX::ref().lastfiles().saveFilePosition(buffer_-fileName(),
| + cursor_.paragraph().id(), 
| + cursor_.pos());

Using paragraph id is icky. Please use offsets into the paragraphlist
instead. You should be able to get that from the cursor.

| Index: src/lastfiles.C
| ===
| --- src/lastfiles.C   (revision 13463)
| +++ src/lastfiles.C   (working copy)
| @@ -26,10 +31,16 @@
|  using std::find;
|  using std::getline;
|  using std::string;
| +using std::map;
| +using std::pair;
|  using std::ifstream;
|  using std::ofstream;
|  using std::ostream_iterator;
|  
| +// store file position information to a map to avoid changing the 
| +// dequeue structure Files
| +typedef mapstring, pairint, lyx::pos_type  file_pos;
| +file_pos filePositions;

This is a no-go. This information must be in the LastFiles object
(class).

| Index: src/lastfiles.h
| ===
| --- src/lastfiles.h   (revision 13463)
| +++ src/lastfiles.h   (working copy)
| @@ -56,6 +59,18 @@
|   @param file the file we write the lastfiles list to.
|   */
|   void writeFile(std::string const  file) const;
| + /** add cursor position to the fname entry in the lastfile
| + @param fname file entry for which to save position information
| + @param id id of the paragraph of the location when the file is 
closed.
| + @param pos position of the cursor when the file is closed.
| + */
| + void LastFiles::saveFilePosition(std::string const fname, int id, 
lyx::pos_type pos) const;
| + /** load saved cursor position to the fname entry in the lastfile
| + @param fname file entry for which to load position information
| + @param id id of the paragraph of the location when the file is 
closed.
| + @param pos position of the cursor when the file is closed.
| + */
| + void LastFiles::loadFilePosition(std::string const fname, int id, 
lyx::pos_type pos ) const;
|   /** Return file #n# in the lastfiles list.
|   @param n number in the list to get
|   */

Using ClassName::Func in the class decl. is a syntax error. Just use
'Func' instead.


-- 
Lgb

Re: [Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Georg Baum]

Martin Vermeer wrote:

> On Wed, 2006-03-22 at 23:19 -0600, Bo Peng wrote:
>> Dear list,
>> 
>> It does not look so difficult to implement this feature so I tried to
>> implement it by myself. Attached is my patch. Since I rarely submit a
>> patch here so I am afraid that there might be many problems (format,
>> logic, bugs). Please have a look and give me some feedbacks.
> 
> Format looks OK. The logic is a bit ad-hoc, but that comes with the
> territory.
> 
> Others may give detailed feedback on the code.
> 
>> Many files have been modified:
>> 
>> src/BufferView_pimpl.C
>> When a new buffer is loaded, it tries to get last cursor position from
>> ref().lastfiles(). When a file is going to be closed, current location
>> is saved as bookmark 0, and to ref().lastfiles(). A side effect of
>> this is that a file can be re-opened (and to the same location) from
>> bookmark 0.

I think that this is a very sensible approach. I would avoid to store this
information in the lyx file itself (privacy). Currently ezxchanging .lyx
files is safe because they don't carry aany hidden information, and I would
like it to stay like that.

>> src/lastfiles.C:
>> I do not want to change the Files structure so I use a map to store
>> position information. lastfile format is changed to "
>> filename" if position information is available, and "filename" if not.
> 
> Would it be easier to always have the  filename format? And id
> = pos = 0 if no valid info exists, so current behaviour is reproduced?

I agree.

>> 1. Will invalid paragraph id, pos crash lyx? I mean, if a file is
>> saved (with id, pos saved locally), and is modified externally (get a
>> revised copy from others), id, pos will no longer be valid and "goto
>> bookmark 0" will fail.

This should be explicitly handled after the file is read: Check for invalid
bookmarks, and delete them.

> This is a file format change and lyx2lyx must be made to handle it.

Why? I dont' see a format change.

>> 2. Save position does not seem to work when I quit lyx by closing the
>> lyx window. Only close-buffer will do. Where should I handle the close
>> event?
> 
> Don't bother. That's not the way to leave LyX.

I disagree. I once had to tutor a lab course where a program was used that
did strange things if it was closed via thw close button. It was very hard
to get the students used to that. IMO the close button should t=do exactly
the same as File->Quit. Everything else would confuse users.


Georg

Re: [Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

> > src/lastfiles.C:
> > I do not want to change the Files structure so I use a map to store
> > position information. lastfile format is changed to "
> > filename" if position information is available, and "filename" if not.
>
> Would it be easier to always have the  filename format? And id
> = pos = 0 if no valid info exists, so current behaviour is reproduced?

With that I want to have a smooth transition to people that already
have a .lyx/lastfile. I is enough though, to always write in <>name
format, but also handle "name" when read.


> > 1. Will invalid paragraph id, pos crash lyx? I mean, if a file is
> > saved (with id, pos saved locally), and is modified externally (get a
> > revised copy from others), id, pos will no longer be valid and "goto
> > bookmark 0" will fail.
>
> This is a file format change and lyx2lyx must be made to handle it.

Is $HOME/.lyx/lastfile also handled by lyx2lyx?

> +// not found, return invalid id number
> +   else {
> +   id = -1;
> +   pos = 0;
> +   }
>
> Please don't do this ;-)
>
> It won't be needed if we change and handle the new file format.

There is a problem with my current approach. LastFiles::readFile only
read num_files entries. If an older file is re-opened, the position
information will not be available. That is why I currently use this
'info not found' approach. It should be better to load all position
information regardless of num_files.


> > 2. Save position does not seem to work when I quit lyx by closing the
> > lyx window. Only close-buffer will do. Where should I handle the close
> > event?
>
> Don't bother. That's not the way to leave LyX.

Many people do this anyway.

> > 3. Is saved_positions[0] really unused?
>
> No, it's used in at least two other places.
>
> 1) To save the main document position to return to when opening a child
> 2) To save the return (ref) position when jumping to a label.
>
> Neither interferes with your use, I think.

That means the first menu entry will/should be disabled after such operations.

Bo

Re: [Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

An updated patch that loads all position information while keeping
only num_files Files entries.

Question: is 0 a valid paragraph ID? I currently use -1 to indicate invalid ID.

Bo
Index: src/BufferView_pimpl.C
===
--- src/BufferView_pimpl.C	(revision 13463)
+++ src/BufferView_pimpl.C	(working copy)
@@ -293,6 +293,14 @@
 
 	setBuffer(b);
 	bv_->showErrorList(_("Parse"));
+
+	// load position when the file was last closed
+	int id;
+	lyx::pos_type pos; 
+	LyX::ref().lastfiles().loadFilePosition(s, id, pos);
+	// if id is valid ...
+	if( id >= 0 )
+		saved_positions[0] = Position(s, id, pos);
 
 	if (tolastfiles)
 		LyX::ref().lastfiles().newFile(b->fileName());
@@ -767,7 +775,14 @@
 	saved_positions[i] = Position(buffer_->fileName(),
   cursor_.paragraph().id(),
   cursor_.pos());
-	if (i > 0)
+	// if i == 0, (called when this buffer is closed), 
+	// save current position to lastfile
+	if (i == 0) {
+		LyX::ref().lastfiles().saveFilePosition(buffer_->fileName(),
+cursor_.paragraph().id(), 
+cursor_.pos());
+}
+	else
 		owner_->message(bformat(_("Saved bookmark %1$d"), i));
 }
 
Index: src/lastfiles.C
===
--- src/lastfiles.C	(revision 13463)
+++ src/lastfiles.C	(working copy)
@@ -19,6 +19,11 @@
 #include 
 #include 
 
+// file_pos is a map to save position of cursor when a file is closed.
+#include 
+#include 
+#include 
+
 namespace fs = boost::filesystem;
 
 using std::copy;
@@ -26,10 +31,16 @@
 using std::find;
 using std::getline;
 using std::string;
+using std::map;
+using std::pair;
 using std::ifstream;
 using std::ofstream;
 using std::ostream_iterator;
 
+// store file position information to a map to avoid changing the 
+// dequeue structure Files
+typedef map > file_pos;
+file_pos filePositions;
 
 LastFiles::LastFiles(string const & filename, bool st, unsigned int num)
 	: dostat(st)
@@ -59,10 +70,31 @@
 	ifstream ifs(filename.c_str());
 	string tmp;
 
-	while (getline(ifs, tmp) && files.size() < num_files) {
+	while (getline(ifs, tmp)) {
+		//  filename\n
+		if (tmp[0] == '<' && tmp.find('>', 1) != string::npos ) {
+			std::istringstream itmp(tmp);
+			string fname;
+			int id;
+			lyx::pos_type pos;
+			itmp.ignore(1);  // ignore "<"
+			itmp >> id;
+			itmp.ignore(2);  // ignore ", "
+			itmp >> pos;
+			itmp.ignore(2);  // ingore "> "
+			itmp >> fname;
+			filePositions[fname] = pair(id, pos);
+			tmp = fname;
+		}
+		// 'tmp' is now the last part of " file" or a line 
+		// without <>. The latter case provides a smooth transition 
+		// for people who have the old lastfile format
 		if (dostat && !fs::exists(tmp))
-continue;
-		files.push_back(tmp);
+			continue;
+		// while loading all position information, 
+		// keep num_files files entries
+		if (files.size() < num_files )
+			files.push_back(tmp);
 	}
 }
 
@@ -71,8 +103,19 @@
 {
 	ofstream ofs(filename.c_str());
 	if (ofs) {
-		copy(files.begin(), files.end(),
-		 ostream_iterator(ofs, "\n"));
+		for (const_iterator file=files.begin(); file != files.end(); ++file) {
+			// has position information, save in the format of 
+			//  filename
+			file_pos::iterator entry = filePositions.find(*file);
+			if (entry != filePositions.end() )
+ofs << "<" << (*entry).second.first << ", " << 
+	(*entry).second.second << "> " << *file << endl;
+			// this will only happen during the transition from the
+			// old to new lastfile format
+			else
+ofs << "<-1, 0> " << *file << endl;
+		}
+		ofs.close();
 	} else
 		lyxerr << "LyX: Warning: unable to save LastFiles: "
 		   << filename << endl;
@@ -90,7 +133,27 @@
 		files.pop_back();
 }
 
+void LastFiles::saveFilePosition(string const& fname, int id, lyx::pos_type pos ) const
+{
+	filePositions[fname] = pair(id, pos);
+}
 
+void LastFiles::loadFilePosition(string const& fname, int& id, lyx::pos_type& pos ) const
+{
+	file_pos::iterator entry = filePositions.find(fname);
+	// has position information, return it.
+	if( entry != filePositions.end() ) {
+		id = (*entry).second.first;
+		pos = (*entry).second.second;
+	}
+	// not found, return invalid id number. This will happen 
+	// in the cases when a new file (not in lastfile) is opened.
+	else {
+		id = -1;
+		pos = 0;
+	}
+}
+
 string const LastFiles::operator[](unsigned int i) const
 {
 	if (i < files.size())
Index: src/lyxfunc.C
===
--- src/lyxfunc.C	(revision 13463)
+++ src/lyxfunc.C	(working copy)
@@ -1880,6 +1880,8 @@
 
 void LyXFunc::closeBuffer()
 {
+	// save current position to lastfile
+	view()->savePosition(0);
 	if (bufferlist.close(owner->buffer(), true) && !quitting) {
 		if (bufferlist.empty()) {
 			// need this otherwise SEGV may occur while
Index: src/lastfiles.h

Re: [Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

"Bo Peng" <[EMAIL PROTECTED]> writes:

| Dear list,
| 
| It does not look so difficult to implement this feature so I tried to
| implement it by myself. Attached is my patch. Since I rarely submit a
| patch here so I am afraid that there might be many problems (format,
| logic, bugs). Please have a look and give me some feedbacks.
| 
| Many files have been modified:
| 
| src/BufferView_pimpl.C
| When a new buffer is loaded, it tries to get last cursor position from
| ref().lastfiles(). When a file is going to be closed, current location
| is saved as bookmark 0, and to ref().lastfiles(). A side effect of
| this is that a file can be re-opened (and to the same location) from
| bookmark 0.

I am not sure if I like lastfiles to be the files that store this
info. One of the reason is that we only store a very limited number of
files there... lastpostion info should hold more files.

I'd prefere if there was a separate file for this information.

bookmark0? where is that stored? if it in the lyx document, then
please no.

Also I think bookmarks and lastplace should be kept a part and not be
connected at all.

| src/lastfiles.C:
| I do not want to change the Files structure so I use a map to store
| position information. lastfile format is changed to "
| filename" if position information is available, and "filename" if not.
| The biggest changes are to read/write lastfile. I hope that I have not
| broken anything here.

With a new file with its own format, this is not a problem. Also
please have a look at how emacs does this. (.emacs-places)
 
| src/lyxfunc.C:
|save cursor position when the buffer is closed.
| 
| lib/ui/stdmenus.ui:
|add a menu item with an awkard name.

What do you need the menuitem for?
 
| I am worrying about two problem though:
| 
| 1. Will invalid paragraph id, pos crash lyx? I mean, if a file is
| saved (with id, pos saved locally), and is modified externally (get a
| revised copy from others), id, pos will no longer be valid and "goto
| bookmark 0" will fail.

Please do not use paragraph id it is not stable. Perhaps use the
cursor stack info instead.
 
| 2. Save position does not seem to work when I quit lyx by closing the
| lyx window. Only close-buffer will do. Where should I handle the close
| event?

That would be a more general bug.

| 3. Is saved_positions[0] really unused?

Where is it found?

-- 
Lgb

Re: [Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

"Bo Peng" <[EMAIL PROTECTED]> writes:

| An updated patch that loads all position information while keeping
| only num_files Files entries.
| 
| Question: is 0 a valid paragraph ID? I currently use -1 to indicate
| invalid ID.

As said, I'd prefere a separate file in .lyx/; but even if I didn't:

| Index: src/BufferView_pimpl.C
| ===
| --- src/BufferView_pimpl.C(revision 13463)
| +++ src/BufferView_pimpl.C(working copy)
| @@ -293,6 +293,14 @@
|  
|   setBuffer(b);
|   bv_->showErrorList(_("Parse"));
| +
| + // load position when the file was last closed
| + int id;
| + lyx::pos_type pos; 
| + LyX::ref().lastfiles().loadFilePosition(s, id, pos);

I must admit that I prefere to not use in/out parameters.
I like tupples though :-)  (boost::tuple)

| + // if id is valid ...
| + if( id >= 0 )
| + saved_positions[0] = Position(s, id, pos);

I think we should just have a switch. LyX wide per user where (lyxrc)
the decides where the lastposition info should be used or not.

| + if (i == 0) {
| + LyX::ref().lastfiles().saveFilePosition(buffer_->fileName(),
| + cursor_.paragraph().id(), 
| + cursor_.pos());

Using paragraph id is icky. Please use offsets into the paragraphlist
instead. You should be able to get that from the cursor.

| Index: src/lastfiles.C
| ===
| --- src/lastfiles.C   (revision 13463)
| +++ src/lastfiles.C   (working copy)
| @@ -26,10 +31,16 @@
|  using std::find;
|  using std::getline;
|  using std::string;
| +using std::map;
| +using std::pair;
|  using std::ifstream;
|  using std::ofstream;
|  using std::ostream_iterator;
|  
| +// store file position information to a map to avoid changing the 
| +// dequeue structure Files
| +typedef map > file_pos;
| +file_pos filePositions;

This is a no-go. This information must be in the LastFiles object
(class).

| Index: src/lastfiles.h
| ===
| --- src/lastfiles.h   (revision 13463)
| +++ src/lastfiles.h   (working copy)
| @@ -56,6 +59,18 @@
|   @param file the file we write the lastfiles list to.
|   */
|   void writeFile(std::string const & file) const;
| + /** add cursor position to the fname entry in the lastfile
| + @param fname file entry for which to save position information
| + @param id id of the paragraph of the location when the file is 
closed.
| + @param pos position of the cursor when the file is closed.
| + */
| + void LastFiles::saveFilePosition(std::string const& fname, int id, 
lyx::pos_type pos) const;
| + /** load saved cursor position to the fname entry in the lastfile
| + @param fname file entry for which to load position information
| + @param id id of the paragraph of the location when the file is 
closed.
| + @param pos position of the cursor when the file is closed.
| + */
| + void LastFiles::loadFilePosition(std::string const& fname, int& id, 
lyx::pos_type& pos ) const;
|   /** Return file #n# in the lastfiles list.
|   @param n number in the list to get
|   */

Using "ClassName::Func" in the class decl. is a syntax error. Just use
'Func' instead.


-- 
Lgb

[Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

Dear list,

It does not look so difficult to implement this feature so I tried to
implement it by myself. Attached is my patch. Since I rarely submit a
patch here so I am afraid that there might be many problems (format,
logic, bugs). Please have a look and give me some feedbacks.

Many files have been modified:

src/BufferView_pimpl.C
When a new buffer is loaded, it tries to get last cursor position from
ref().lastfiles(). When a file is going to be closed, current location
is saved as bookmark 0, and to ref().lastfiles(). A side effect of
this is that a file can be re-opened (and to the same location) from
bookmark 0.

src/lastfiles.C:
I do not want to change the Files structure so I use a map to store
position information. lastfile format is changed to id, pos
filename if position information is available, and filename if not.
The biggest changes are to read/write lastfile. I hope that I have not
broken anything here.

src/lyxfunc.C:
   save cursor position when the buffer is closed.

lib/ui/stdmenus.ui:
   add a menu item with an awkard name.

I am worrying about two problem though:

1. Will invalid paragraph id, pos crash lyx? I mean, if a file is
saved (with id, pos saved locally), and is modified externally (get a
revised copy from others), id, pos will no longer be valid and goto
bookmark 0 will fail.

2. Save position does not seem to work when I quit lyx by closing the
lyx window. Only close-buffer will do. Where should I handle the close
event?

3. Is saved_positions[0] really unused?

Cheers,
Bo
Index: src/BufferView_pimpl.C
===
--- src/BufferView_pimpl.C	(revision 13455)
+++ src/BufferView_pimpl.C	(working copy)
@@ -293,6 +293,14 @@
 
 	setBuffer(b);
 	bv_-showErrorList(_(Parse));
+
+	// load position when the file was last closed
+	int id;
+	lyx::pos_type pos; 
+	LyX::ref().lastfiles().loadFilePosition(s, id, pos);
+	// if id is valid ...
+	if( id = 0 )
+		saved_positions[0] = Position(s, id, pos);
 
 	if (tolastfiles)
 		LyX::ref().lastfiles().newFile(b-fileName());
@@ -767,7 +775,14 @@
 	saved_positions[i] = Position(buffer_-fileName(),
   cursor_.paragraph().id(),
   cursor_.pos());
-	if (i  0)
+	// if i == 0, (called when this buffer is closed), 
+	// save current position to lastfile
+	if (i == 0) {
+		LyX::ref().lastfiles().saveFilePosition(buffer_-fileName(),
+cursor_.paragraph().id(), 
+cursor_.pos());
+}
+	else
 		owner_-message(bformat(_(Saved bookmark %1$d), i));
 }
 
Index: src/lastfiles.C
===
--- src/lastfiles.C	(revision 13455)
+++ src/lastfiles.C	(working copy)
@@ -19,6 +19,11 @@
 #include fstream
 #include iterator
 
+// file_pos is a map to save position of cursor when a file is closed.
+#include map
+#include sstream
+#include utility
+
 namespace fs = boost::filesystem;
 
 using std::copy;
@@ -26,10 +31,16 @@
 using std::find;
 using std::getline;
 using std::string;
+using std::map;
+using std::pair;
 using std::ifstream;
 using std::ofstream;
 using std::ostream_iterator;
 
+// store file position information to a map to avoid changing the 
+// dequeue structure Files
+typedef mapstring, pairint, lyx::pos_type  file_pos;
+file_pos filePositions;
 
 LastFiles::LastFiles(string const  filename, bool st, unsigned int num)
 	: dostat(st)
@@ -60,6 +71,22 @@
 	string tmp;
 
 	while (getline(ifs, tmp)  files.size()  num_files) {
+		// id, position filename\n
+		if (tmp[0] == ''  tmp.find('', 1) != string::npos ) {
+			std::istringstream itmp(tmp);
+			string fname;
+			int id;
+			lyx::pos_type pos;
+			itmp.ignore(1);  // ignore ''
+			itmp  id;
+			itmp.ignore(2);  // ignore , 
+			itmp  pos;
+			itmp.ignore(2);  // ingore  ' '
+			itmp  fname;
+			filePositions[fname] = pairint, lyx::pos_type(id, pos);
+			tmp = fname;
+		}
+		// the last part of  file or or lines without .
 		if (dostat  !fs::exists(tmp))
 continue;
 		files.push_back(tmp);
@@ -71,8 +98,18 @@
 {
 	ofstream ofs(filename.c_str());
 	if (ofs) {
-		copy(files.begin(), files.end(),
-		 ostream_iteratorstring(ofs, \n));
+		for (const_iterator file=files.begin(); file != files.end(); ++file) {
+			// has position information, save in the format of 
+// id, pos filename
+			file_pos::iterator entry = filePositions.find(*file);
+			if (entry != filePositions.end() )
+ofs(*entry).second.first  ,   
+	(*entry).second.second *file  endl;
+			// if not, simply save filename
+			else
+ofs  *file  endl;
+		}
+		ofs.close();
 	} else
 		lyxerr  LyX: Warning: unable to save LastFiles: 
 		filename  endl;
@@ -90,7 +127,26 @@
 		files.pop_back();
 }
 
+void LastFiles::saveFilePosition(string const fname, int id, lyx::pos_type pos ) const
+{
+	filePositions[fname] = pairint, lyx::pos_type(id, pos);
+}
 
+void LastFiles::loadFilePosition(string const fname, int id, lyx::pos_type pos ) const
+{
+	

Re: [Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Martin Vermeer]

On Wed, 2006-03-22 at 23:19 -0600, Bo Peng wrote:
 Dear list,
 
 It does not look so difficult to implement this feature so I tried to
 implement it by myself. Attached is my patch. Since I rarely submit a
 patch here so I am afraid that there might be many problems (format,
 logic, bugs). Please have a look and give me some feedbacks.

Format looks OK. The logic is a bit ad-hoc, but that comes with the
territory.

Others may give detailed feedback on the code.

 Many files have been modified:
 
 src/BufferView_pimpl.C
 When a new buffer is loaded, it tries to get last cursor position from
 ref().lastfiles(). When a file is going to be closed, current location
 is saved as bookmark 0, and to ref().lastfiles(). A side effect of
 this is that a file can be re-opened (and to the same location) from
 bookmark 0.
 
 src/lastfiles.C:
 I do not want to change the Files structure so I use a map to store
 position information. lastfile format is changed to id, pos
 filename if position information is available, and filename if not.

Would it be easier to always have the id, pos filename format? And id
= pos = 0 if no valid info exists, so current behaviour is reproduced?

 The biggest changes are to read/write lastfile. I hope that I have not
 broken anything here.
 
 src/lyxfunc.C:
save cursor position when the buffer is closed.
 
 lib/ui/stdmenus.ui:
add a menu item with an awkard name.
 
 I am worrying about two problem though:
 
 1. Will invalid paragraph id, pos crash lyx? I mean, if a file is
 saved (with id, pos saved locally), and is modified externally (get a
 revised copy from others), id, pos will no longer be valid and goto
 bookmark 0 will fail.

This is a file format change and lyx2lyx must be made to handle it.

+// not found, return invalid id number
+   else {
+   id = -1;
+   pos = 0;
+   }

Please don't do this ;-)

It won't be needed if we change and handle the new file format.

 2. Save position does not seem to work when I quit lyx by closing the
 lyx window. Only close-buffer will do. Where should I handle the close
 event?

Don't bother. That's not the way to leave LyX.

 3. Is saved_positions[0] really unused?

No, it's used in at least two other places.

1) To save the main document position to return to when opening a child
2) To save the return (ref) position when jumping to a label.

Neither interferes with your use, I think.

So you're not the first to have this brilliant idea :-)

 Cheers,
 Bo

Welcome to the club!

Regards Martin



signature.asc
Description: This is a digitally signed message part

[Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

Dear list,

It does not look so difficult to implement this feature so I tried to
implement it by myself. Attached is my patch. Since I rarely submit a
patch here so I am afraid that there might be many problems (format,
logic, bugs). Please have a look and give me some feedbacks.

Many files have been modified:

src/BufferView_pimpl.C
When a new buffer is loaded, it tries to get last cursor position from
ref().lastfiles(). When a file is going to be closed, current location
is saved as bookmark 0, and to ref().lastfiles(). A side effect of
this is that a file can be re-opened (and to the same location) from
bookmark 0.

src/lastfiles.C:
I do not want to change the Files structure so I use a map to store
position information. lastfile format is changed to "
filename" if position information is available, and "filename" if not.
The biggest changes are to read/write lastfile. I hope that I have not
broken anything here.

src/lyxfunc.C:
   save cursor position when the buffer is closed.

lib/ui/stdmenus.ui:
   add a menu item with an awkard name.

I am worrying about two problem though:

1. Will invalid paragraph id, pos crash lyx? I mean, if a file is
saved (with id, pos saved locally), and is modified externally (get a
revised copy from others), id, pos will no longer be valid and "goto
bookmark 0" will fail.

2. Save position does not seem to work when I quit lyx by closing the
lyx window. Only close-buffer will do. Where should I handle the close
event?

3. Is saved_positions[0] really unused?

Cheers,
Bo
Index: src/BufferView_pimpl.C
===
--- src/BufferView_pimpl.C	(revision 13455)
+++ src/BufferView_pimpl.C	(working copy)
@@ -293,6 +293,14 @@
 
 	setBuffer(b);
 	bv_->showErrorList(_("Parse"));
+
+	// load position when the file was last closed
+	int id;
+	lyx::pos_type pos; 
+	LyX::ref().lastfiles().loadFilePosition(s, id, pos);
+	// if id is valid ...
+	if( id >= 0 )
+		saved_positions[0] = Position(s, id, pos);
 
 	if (tolastfiles)
 		LyX::ref().lastfiles().newFile(b->fileName());
@@ -767,7 +775,14 @@
 	saved_positions[i] = Position(buffer_->fileName(),
   cursor_.paragraph().id(),
   cursor_.pos());
-	if (i > 0)
+	// if i == 0, (called when this buffer is closed), 
+	// save current position to lastfile
+	if (i == 0) {
+		LyX::ref().lastfiles().saveFilePosition(buffer_->fileName(),
+cursor_.paragraph().id(), 
+cursor_.pos());
+}
+	else
 		owner_->message(bformat(_("Saved bookmark %1$d"), i));
 }
 
Index: src/lastfiles.C
===
--- src/lastfiles.C	(revision 13455)
+++ src/lastfiles.C	(working copy)
@@ -19,6 +19,11 @@
 #include 
 #include 
 
+// file_pos is a map to save position of cursor when a file is closed.
+#include 
+#include 
+#include 
+
 namespace fs = boost::filesystem;
 
 using std::copy;
@@ -26,10 +31,16 @@
 using std::find;
 using std::getline;
 using std::string;
+using std::map;
+using std::pair;
 using std::ifstream;
 using std::ofstream;
 using std::ostream_iterator;
 
+// store file position information to a map to avoid changing the 
+// dequeue structure Files
+typedef map > file_pos;
+file_pos filePositions;
 
 LastFiles::LastFiles(string const & filename, bool st, unsigned int num)
 	: dostat(st)
@@ -60,6 +71,22 @@
 	string tmp;
 
 	while (getline(ifs, tmp) && files.size() < num_files) {
+		//  filename\n
+		if (tmp[0] == '<' && tmp.find('>', 1) != string::npos ) {
+			std::istringstream itmp(tmp);
+			string fname;
+			int id;
+			lyx::pos_type pos;
+			itmp.ignore(1);  // ignore '<'
+			itmp >> id;
+			itmp.ignore(2);  // ignore ", "
+			itmp >> pos;
+			itmp.ignore(2);  // ingore  '> '
+			itmp >> fname;
+			filePositions[fname] = pair(id, pos);
+			tmp = fname;
+		}
+		// the last part of "<> file" or or lines without <>.
 		if (dostat && !fs::exists(tmp))
 continue;
 		files.push_back(tmp);
@@ -71,8 +98,18 @@
 {
 	ofstream ofs(filename.c_str());
 	if (ofs) {
-		copy(files.begin(), files.end(),
-		 ostream_iterator(ofs, "\n"));
+		for (const_iterator file=files.begin(); file != files.end(); ++file) {
+			// has position information, save in the format of 
+//  filename
+			file_pos::iterator entry = filePositions.find(*file);
+			if (entry != filePositions.end() )
+ofs << "<" << (*entry).second.first << ", " << 
+	(*entry).second.second << "> " << *file << endl;
+			// if not, simply save filename
+			else
+ofs << *file << endl;
+		}
+		ofs.close();
 	} else
 		lyxerr << "LyX: Warning: unable to save LastFiles: "
 		   << filename << endl;
@@ -90,7 +127,26 @@
 		files.pop_back();
 }
 
+void LastFiles::saveFilePosition(string const& fname, int id, lyx::pos_type pos ) const
+{
+	filePositions[fname] = pair(id, pos);
+}
 
+void LastFiles::loadFilePosition(string 

Re: [Pre-Patch] Was: Feature request: Remember the editing location when a file is closed.

[Martin Vermeer]

On Wed, 2006-03-22 at 23:19 -0600, Bo Peng wrote:
> Dear list,
> 
> It does not look so difficult to implement this feature so I tried to
> implement it by myself. Attached is my patch. Since I rarely submit a
> patch here so I am afraid that there might be many problems (format,
> logic, bugs). Please have a look and give me some feedbacks.

Format looks OK. The logic is a bit ad-hoc, but that comes with the
territory.

Others may give detailed feedback on the code.

> Many files have been modified:
> 
> src/BufferView_pimpl.C
> When a new buffer is loaded, it tries to get last cursor position from
> ref().lastfiles(). When a file is going to be closed, current location
> is saved as bookmark 0, and to ref().lastfiles(). A side effect of
> this is that a file can be re-opened (and to the same location) from
> bookmark 0.
> 
> src/lastfiles.C:
> I do not want to change the Files structure so I use a map to store
> position information. lastfile format is changed to "
> filename" if position information is available, and "filename" if not.

Would it be easier to always have the  filename format? And id
= pos = 0 if no valid info exists, so current behaviour is reproduced?

> The biggest changes are to read/write lastfile. I hope that I have not
> broken anything here.
> 
> src/lyxfunc.C:
>save cursor position when the buffer is closed.
> 
> lib/ui/stdmenus.ui:
>add a menu item with an awkard name.
> 
> I am worrying about two problem though:
> 
> 1. Will invalid paragraph id, pos crash lyx? I mean, if a file is
> saved (with id, pos saved locally), and is modified externally (get a
> revised copy from others), id, pos will no longer be valid and "goto
> bookmark 0" will fail.

This is a file format change and lyx2lyx must be made to handle it.

+// not found, return invalid id number
+   else {
+   id = -1;
+   pos = 0;
+   }

Please don't do this ;-)

It won't be needed if we change and handle the new file format.

> 2. Save position does not seem to work when I quit lyx by closing the
> lyx window. Only close-buffer will do. Where should I handle the close
> event?

Don't bother. That's not the way to leave LyX.

> 3. Is saved_positions[0] really unused?

No, it's used in at least two other places.

1) To save the main document position to return to when opening a child
2) To save the return (ref) position when jumping to a label.

Neither interferes with your use, I think.

So you're not the first to have this brilliant idea :-)

> Cheers,
> Bo

Welcome to the club!

Regards Martin



signature.asc
Description: This is a digitally signed message part