Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Angus Leeming]

Lars Gullik Bjønnes [EMAIL PROTECTED] writes:
 Bo Peng [EMAIL PROTECTED] writes:
 | +void FilePositions::readFile(string const  filename)
 | +{
 | +   // we will not complain if we can't find filename nor will
 | +   // we issue a warning. (Lgb)
 | +   ifstream ifs(filename.c_str());
 | +   string tmp;
 | +
 | +   while (getline(ifs, tmp)  file_positions.size()  num_positions ) {
 | +   // pos, filename\n
 | +   FilePos pos;
 | +   string fname;
 | +   istringstream itmp(tmp);
 | +   itmp  pos;
 | +   itmp.ignore(2);  // ingore ,

 Typo in comment. Also are you not ignoring , ?

If this is meant to be code to read a comma-separated list, then it's not
robust. This file can be edited by hand...

Bo, have a look at Boost.Tokenizer. It's ideally suited to, well, tokenizing ;-)
There are examples of it in use in the src/support directory.

Regards,
Angus

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Jean-Marc Lasgouttes]

 Lars == Lars Gullik Bjønnes [EMAIL PROTECTED] writes:

Lars Just as a test... load a document move the cursor save. Exit
Lars lyx. Start lyx. Load a different document. Load the first doc.
Lars Where is the cursor placed now?

To make things fail you have to edit the file before saving. Something
like

1/ enter
 aa

 bb

2/ insert a paragraph after first
 aa

 cc

 bb

3/ save with cursor on the second paragraph.

I bet that restore will not work anymore if it is based on paragraph
id. OTOH, a simple par/pos will not work either.

I suspect the only solution is to output the cursor stack in a format
like

cursor_size pit1 pos1 pit2 pos2 ...

[the cursor_size part could be skipped if we test for line end]

The code to read and write this could be put as  and  operators
for StableDocIterator. Actually these operators are already declared,
but only  is implmented, and in a form that does not allow
re-reading.

JMarc

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Angus Leeming [EMAIL PROTECTED] writes:

| Lars Gullik Bjønnes [EMAIL PROTECTED] writes:
|  Bo Peng [EMAIL PROTECTED] writes:
|  | +void FilePositions::readFile(string const  filename)
|  | +{
|  | + // we will not complain if we can't find filename nor will
|  | + // we issue a warning. (Lgb)
|  | + ifstream ifs(filename.c_str());
|  | + string tmp;
|  | +
|  | + while (getline(ifs, tmp)  file_positions.size()  num_positions ) {
|  | + // pos, filename\n
|  | + FilePos pos;
|  | + string fname;
|  | + istringstream itmp(tmp);
|  | + itmp  pos;
|  | + itmp.ignore(2);  // ingore ,
| 
|  Typo in comment. Also are you not ignoring , ?
| 
| If this is meant to be code to read a comma-separated list, then it's not
| robust. This file can be edited by hand...

If you edit it by hand you are on your own.

But sure if we can make it more robust.

| Bo, have a look at Boost.Tokenizer. It's ideally suited to, well,
| tokenizing ;-)

Perhaps boost::regex will be even easier.

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Angus Leeming]

Lars Gullik Bjønnes [EMAIL PROTECTED] writes:
 | If this is meant to be code to read a comma-separated list, then it's not
 | robust. This file can be edited by hand...
 
 If you edit it by hand you are on your own.
 But sure if we can make it more robust.

 | Bo, have a look at Boost.Tokenizer. It's ideally suited to, well,
 | tokenizing 
 Perhaps boost::regex will be even easier.

I don't think so.

#include iostream
#include boost/tokenizer.hpp
#include string

int main()
{
   std::string const data = This is,  a test;
   typedef boost::tokenizerboost::char_separatorchar  Tokenizer;
   // Tokenize on spaces and commas.
   // Discard empty tokens.
   boost::char_separatorchar sep( ,);

   Tokenizer tokens(data, sep);
   Tokenizer::iterator const end = tokens.end();
   for (Tokenizer::iterator it = tokens.begin(); it != end; ++it)
 std::cout*tok_iter   ;
   std::cout  \n;
   return EXIT_SUCCESS;
}

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Angus Leeming [EMAIL PROTECTED] writes:

| Lars Gullik Bjønnes [EMAIL PROTECTED] writes:
|  | If this is meant to be code to read a comma-separated list, then it's not
|  | robust. This file can be edited by hand...
|  
|  If you edit it by hand you are on your own.
|  But sure if we can make it more robust.
| 
|  | Bo, have a look at Boost.Tokenizer. It's ideally suited to, well,
|  | tokenizing 
|  Perhaps boost::regex will be even easier.
| 
| I don't think so.
| 
| #include iostream
| #include boost/tokenizer.hpp
| #include string
| 
| int main()
| {
|std::string const data = This is,  a test;
|typedef boost::tokenizerboost::char_separatorchar  Tokenizer;
|// Tokenize on spaces and commas.
|// Discard empty tokens.
|boost::char_separatorchar sep( ,);
| 
|Tokenizer tokens(data, sep);
|Tokenizer::iterator const end = tokens.end();
|for (Tokenizer::iterator it = tokens.begin(); it != end; ++it)
|  std::cout*tok_iter   ;
|std::cout  \n;
|return EXIT_SUCCESS;
| }

I am still not sure.

#include iostream
#include boost/regex.hpp
#include string

int main()
{
boost::regex reg(([^,]+),[ ]+(.*));
std::string const data = This is,  a test;

boost::smatch m;
if (boost::regex_search(data, m, reg)) {
std::coutm[1]   ;
std::coutm[2]   ;
std::cout  \n;
return EXIT_SUCCESS;
}

return EXIT_FAILURE;
}

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Angus Leeming]

Lars Gullik Bjønnes [EMAIL PROTECTED] writes:
 I am still not sure.
 boost::regex reg(([^,]+),[ ]+(.*));

Does your regex work over multiple lines, for example?

Regexes are expensive to construct and a pain to maintain.
As case in point, what is that (.*) group doing? If you
want to handle multiple tokens then the regex above just
doesn't cut it.

Use the right tool for the job; regexes aren't good
tools for parsing straightforward grammars. In fact,
parsers written using them (eg reLyX) are a PITA.

Angus

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Angus Leeming [EMAIL PROTECTED] writes:

| Lars Gullik Bjønnes [EMAIL PROTECTED] writes:
|  I am still not sure.
|  boost::regex reg(([^,]+),[ ]+(.*));
| 
| Does your regex work over multiple lines, for example?

It does not have to.

| Regexes are expensive to construct and a pain to maintain.
| As case in point, what is that (.*) group doing? If you
| want to handle multiple tokens then the regex above just
| doesn't cut it.

We are talking of a specific file format, not some general parser
stuff.

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Jean-Marc Lasgouttes]

 Lars == Lars Gullik Bjønnes [EMAIL PROTECTED] writes:

Lars Angus Leeming [EMAIL PROTECTED] writes: | Lars Gullik Bjønnes
| Does your regex work over multiple lines, for example?

Lars It does not have to.

I even think it should not.

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

 We have 'lastfiles' and 'lastpos' as configurable filenames, I am not
 sure that they should be. IMHO we should just hardcode them. Opinions
 on that?

I just tried to make features of lastpos parallel to lastfiles. I do
not think these filenames need to be configured. lastfile::dostat may
not be needed either. Sure, people may lose lastfile entries under
some special circumstances but we donot have to worry too much about
those cases.

I think lastpos is a better name to this feature, should I change
everything from fileposition to lastpos?

 | 1. Alt-F4 and close-button do not trigger LDUNC_QUIT or bufferClose()
 | so position will not be saved. Note that Alt-F4 is marked as a
 | shortcut for File-Exit but it behaves differently than File-Exit.

 This not triggering, is it the same on all frontends?

I do not know. I am using RHEL4 + KDE 3.3. I think Alt-F4 is
intercepted by KDE and passed to the application as the same event as
close-button.

 | 2. If a filename is passed to lyx as 'lyx file.lyx', the cursor will
 | be moved by my patch, but then moved back by some unknown mechanism
 | (seems to be some GUI stuff). This is a big drawback since this is the
 | main way I start lyx.

 Ok, we need to investigate a bit.
 Could it be that a -setCursor is also called later, so that your
 setting is overwritten?

I found that loadLyXFile is called in frontend/qt2/lyx)gui.C but can
not trace further. I think some other event is called after it.

 Also, is the clearSelection ever needed? This is upon file load. Can a
 selection ever be set?

Maybe not. I tried to mimic restorePosition, and I kept this line for
safety reasons.

 As said I don't like this mix of features, but as it is strictly an
 internal thing, let's handle that one later.

Using bookmark[0] has the advantage that other bookmarks[0] usages
will also be saved. They are often related to buffer change (?).

 space before '' please
 ditto
 no space before ''
 space before ''
 ditto

Over the years, I have formed a bad habit of relying on bcpp to format
my code, so I do not have to worry about indent too much during
coding. I know bcpp is not powerful enough for fine formatting, but
there are tools over there. Have you thought of recommending one
program with a specified set of options to be applied to lyx source
code automatically? Then new comers like me do not have to worry about
such details.

Regarding the robustness you mentioned later. I think regex and
tokenizer (I use both myself) are overkill for such a simple case. My
coding style is a try/catch block around the existing code and ignore
wrong lines silently. I overheard, however, that exceptions is/was not
used by lyx...

Talking about robustness, I remember that I once used a 1.4.x layout
file for lyx 1.3.7. Lyx crashed silently. I do think we should have
better handling of errors in 1.4.x.

Cheers,
Bo

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Juergen Spitzmueller]

Bo Peng wrote:
 I think lastpos is a better name to this feature

As long as it cannot be confused with cursor.lastpos()
(I didn't read your patch, so the comment might be irrelevant).

Jürgen

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Abdelrazak Younes]

Bo Peng a écrit :

We have 'lastfiles' and 'lastpos' as configurable filenames, I am not
sure that they should be. IMHO we should just hardcode them. Opinions
on that?


I just tried to make features of lastpos parallel to lastfiles. I do
not think these filenames need to be configured. lastfile::dostat may
not be needed either. Sure, people may lose lastfile entries under
some special circumstances but we donot have to worry too much about
those cases.


IMHO this is session management and all session management should go 
into a single file with a wide spread format (say XML or INI like).
Where you're at it Bo, could you implement the (optional) reloading of 
all file that where opened on last exit? ;-)


Abdel.

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

On 3/24/06, Abdelrazak Younes [EMAIL PROTECTED] wrote:
 could you implement the (optional) reloading of
 all file that where opened on last exit? ;-)

That is a separate feature that is definitely nice to have. It is
possible though, to make $HOME/.lyx/lastfilepos something like
*, 32, file1
*, 12, file2
45, file3

and, of course optionally, load file1 and file2 automatically when lyx starts.

Bo

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Georg Baum]

Abdelrazak Younes wrote:

 IMHO this is session management and all session management should go
 into a single file with a wide spread format (say XML or INI like).

I disagree a bit. Being able to open a file at the last position is not
session management IMHO. session management is recreate the exact same
state as LyX was in before shutdown, and it should be implemented using
the qt/gtk API.

 Where you're at it Bo, could you implement the (optional) reloading of
 all file that where opened on last exit? ;-)

This is indeed session management.


Georg

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Abdelrazak Younes]

Georg Baum a écrit :

Abdelrazak Younes wrote:


IMHO this is session management and all session management should go
into a single file with a wide spread format (say XML or INI like).


I disagree a bit. Being able to open a file at the last position is not
session management IMHO. session management is recreate the exact same
state as LyX was in before shutdown, and it should be implemented using
the qt/gtk API.


Indeed, from the Qt doc:

void QApplication::saveState ( QSessionManager  manager )  [virtual]
This function deals with session management. It is invoked when the 
session manager wants the application to preserve its state for a future 
session.
For example, a text editor would create a temporary file that includes 
the current contents of its edit buffers, the location of the cursor and 
other aspects of the current editing session.



Where you're at it Bo, could you implement the (optional) reloading of
all file that where opened on last exit? ;-)


This is indeed session management. 

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

  Where you're at it Bo, could you implement the (optional) reloading of
  all file that where opened on last exit? ;-)

 This is indeed session management.

If we only need to open files at their original positions, we do not
really need qt api etc. I can expand filepositions to something like

$HOME/.lyx/session

[closedfiles]
file1

[lastpos]
23, file1
129, file2

My patch will handle lastpos. A simple patch during lyx startup can
load all files.

Bo

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[John C. Spray]

On Fri, 2006-03-24 at 15:29 +0100, Georg Baum wrote:
 I disagree a bit. Being able to open a file at the last position is not
 session management IMHO. session management is recreate the exact same
 state as LyX was in before shutdown, and it should be implemented using
 the qt/gtk API.

Do you mean things like Was the math panel open? and Where was the
math panel?.  Seems like a good idea, although perhaps more basic
things like allowing hiding/showing of toolbars from the View menu would
be better done first.

John

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Georg Baum]

John C. Spray wrote:

 Do you mean things like Was the math panel open? and Where was the
 math panel?.  Seems like a good idea, although perhaps more basic
 things like allowing hiding/showing of toolbars from the View menu would
 be better done first.

No. What I mean are more basic things: documents that were open, cursor
position in each document and unsaved changes in each document.
Things like window size, math panel position etc. would be a plus, but I
think that these could be handled by the toolkit.


Georg

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Angus Leeming]

Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes:
> "Bo Peng" <[EMAIL PROTECTED]> writes:
> | +void FilePositions::readFile(string const & filename)
> | +{
> | +   // we will not complain if we can't find filename nor will
> | +   // we issue a warning. (Lgb)
> | +   ifstream ifs(filename.c_str());
> | +   string tmp;
> | +
> | +   while (getline(ifs, tmp) && file_positions.size() < num_positions ) {
> | +   // pos, filename\n
> | +   FilePos pos;
> | +   string fname;
> | +   istringstream itmp(tmp);
> | +   itmp >> pos;
> | +   itmp.ignore(2);  // ingore ","

> Typo in comment. Also are you not ignoring ", "?

If this is meant to be code to read a comma-separated list, then it's not
robust. This file can be edited by hand...

Bo, have a look at Boost.Tokenizer. It's ideally suited to, well, tokenizing ;-)
There are examples of it in use in the src/support directory.

Regards,
Angus

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Jean-Marc Lasgouttes]

> "Lars" == Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes:

Lars> Just as a test... load a document move the cursor save. Exit
Lars> lyx. Start lyx. Load a different document. Load the first doc.
Lars> Where is the cursor placed now?

To make things fail you have to edit the file before saving. Something
like

1/ enter
 aa

 bb

2/ insert a paragraph after first
 aa

 cc

 bb

3/ save with cursor on the second paragraph.

I bet that restore will not work anymore if it is based on paragraph
id. OTOH, a simple par/pos will not work either.

I suspect the only solution is to output the cursor stack in a format
like

 ...

[the  part could be skipped if we test for line end]

The code to read and write this could be put as << and >> operators
for StableDocIterator. Actually these operators are already declared,
but only << is implmented, and in a form that does not allow
re-reading.

JMarc

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Angus Leeming <[EMAIL PROTECTED]> writes:

| Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes:
| > "Bo Peng" <[EMAIL PROTECTED]> writes:
| > | +void FilePositions::readFile(string const & filename)
| > | +{
| > | + // we will not complain if we can't find filename nor will
| > | + // we issue a warning. (Lgb)
| > | + ifstream ifs(filename.c_str());
| > | + string tmp;
| > | +
| > | + while (getline(ifs, tmp) && file_positions.size() < num_positions ) {
| > | + // pos, filename\n
| > | + FilePos pos;
| > | + string fname;
| > | + istringstream itmp(tmp);
| > | + itmp >> pos;
| > | + itmp.ignore(2);  // ingore ","
| 
| > Typo in comment. Also are you not ignoring ", "?
| 
| If this is meant to be code to read a comma-separated list, then it's not
| robust. This file can be edited by hand...

If you edit it by hand you are on your own.

But sure if we can make it more robust.

| Bo, have a look at Boost.Tokenizer. It's ideally suited to, well,
| tokenizing ;-)

Perhaps boost::regex will be even easier.

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Angus Leeming]

Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes:
> | If this is meant to be code to read a comma-separated list, then it's not
> | robust. This file can be edited by hand...
> 
> If you edit it by hand you are on your own.
> But sure if we can make it more robust.

> | Bo, have a look at Boost.Tokenizer. It's ideally suited to, well,
> | tokenizing 
> Perhaps boost::regex will be even easier.

I don't think so.

#include 
#include 
#include 

int main()
{
   std::string const data = "This is,  a test";
   typedef boost::tokenizer Tokenizer;
   // Tokenize on spaces and commas.
   // Discard empty tokens.
   boost::char_separator sep(" ,");

   Tokenizer tokens(data, sep);
   Tokenizer::iterator const end = tokens.end();
   for (Tokenizer::iterator it = tokens.begin(); it != end; ++it)
 std::cout << "<" << *tok_iter << "> ";
   std::cout << "\n";
   return EXIT_SUCCESS;
}

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Angus Leeming <[EMAIL PROTECTED]> writes:

| Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes:
| > | If this is meant to be code to read a comma-separated list, then it's not
| > | robust. This file can be edited by hand...
| > 
| > If you edit it by hand you are on your own.
| > But sure if we can make it more robust.
| 
| > | Bo, have a look at Boost.Tokenizer. It's ideally suited to, well,
| > | tokenizing 
| > Perhaps boost::regex will be even easier.
| 
| I don't think so.
| 
| #include 
| #include 
| #include 
| 
| int main()
| {
|std::string const data = "This is,  a test";
|typedef boost::tokenizer Tokenizer;
|// Tokenize on spaces and commas.
|// Discard empty tokens.
|boost::char_separator sep(" ,");
| 
|Tokenizer tokens(data, sep);
|Tokenizer::iterator const end = tokens.end();
|for (Tokenizer::iterator it = tokens.begin(); it != end; ++it)
|  std::cout << "<" << *tok_iter << "> ";
|std::cout << "\n";
|return EXIT_SUCCESS;
| }

I am still not sure.

#include 
#include 
#include 

int main()
{
boost::regex reg("([^,]+),[ ]+(.*)");
std::string const data = "This is,  a test";

boost::smatch m;
if (boost::regex_search(data, m, reg)) {
std::cout << "<" << m[1] << "> ";
std::cout << "<" << m[2] << "> ";
std::cout << "\n";
return EXIT_SUCCESS;
}

return EXIT_FAILURE;
}

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Angus Leeming]

Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes:
> I am still not sure.
> boost::regex reg("([^,]+),[ ]+(.*)");

Does your regex work over multiple lines, for example?

Regexes are expensive to construct and a pain to maintain.
As case in point, what is that (.*) group doing? If you
want to handle multiple tokens then the regex above just
doesn't cut it.

Use the right tool for the job; regexes aren't good
tools for parsing straightforward grammars. In fact,
parsers written using them (eg reLyX) are a PITA.

Angus

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Angus Leeming <[EMAIL PROTECTED]> writes:

| Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes:
| > I am still not sure.
| > boost::regex reg("([^,]+),[ ]+(.*)");
| 
| Does your regex work over multiple lines, for example?

It does not have to.

| Regexes are expensive to construct and a pain to maintain.
| As case in point, what is that (.*) group doing? If you
| want to handle multiple tokens then the regex above just
| doesn't cut it.

We are talking of a specific file format, not some general parser
stuff.

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Jean-Marc Lasgouttes]

> "Lars" == Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes:

Lars> Angus Leeming <[EMAIL PROTECTED]> writes: | Lars Gullik Bjønnes
| Does your regex work over multiple lines, for example?

Lars> It does not have to.

I even think it should not.

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

> We have 'lastfiles' and 'lastpos' as configurable filenames, I am not
> sure that they should be. IMHO we should just hardcode them. Opinions
> on that?

I just tried to make features of lastpos parallel to lastfiles. I do
not think these filenames need to be configured. lastfile::dostat may
not be needed either. Sure, people may lose lastfile entries under
some special circumstances but we donot have to worry too much about
those cases.

I think lastpos is a better name to this feature, should I change
everything from fileposition to lastpos?

> | 1. Alt-F4 and close-button do not trigger LDUNC_QUIT or bufferClose()
> | so position will not be saved. Note that Alt-F4 is marked as a
> | shortcut for File->Exit but it behaves differently than File->Exit.
>
> This not triggering, is it the same on all frontends?

I do not know. I am using RHEL4 + KDE 3.3. I think Alt-F4 is
intercepted by KDE and passed to the application as the same event as
close-button.

> | 2. If a filename is passed to lyx as 'lyx file.lyx', the cursor will
> | be moved by my patch, but then moved back by some unknown mechanism
> | (seems to be some GUI stuff). This is a big drawback since this is the
> | main way I start lyx.
>
> Ok, we need to investigate a bit.
> Could it be that a ->setCursor is also called later, so that your
> setting is overwritten?

I found that loadLyXFile is called in frontend/qt2/lyx)gui.C but can
not trace further. I think some other event is called after it.

> Also, is the clearSelection ever needed? This is upon file load. Can a
> selection ever be set?

Maybe not. I tried to mimic restorePosition, and I kept this line for
safety reasons.

> As said I don't like this mix of features, but as it is strictly an
> internal thing, let's handle that one later.

Using bookmark[0] has the advantage that other bookmarks[0] usages
will also be saved. They are often related to buffer change (?).

> space before '&' please
> ditto
> no space before '>'
> space before '&'
> ditto

Over the years, I have formed a bad habit of relying on bcpp to format
my code, so I do not have to worry about indent too much during
coding. I know bcpp is not powerful enough for fine formatting, but
there are tools over there. Have you thought of recommending one
program with a specified set of options to be applied to lyx source
code automatically? Then new comers like me do not have to worry about
such details.

Regarding the robustness you mentioned later. I think regex and
tokenizer (I use both myself) are overkill for such a simple case. My
coding style is a try/catch block around the existing code and ignore
wrong lines silently. I overheard, however, that exceptions is/was not
used by lyx...

Talking about robustness, I remember that I once used a 1.4.x layout
file for lyx 1.3.7. Lyx crashed silently. I do think we should have
better handling of errors in 1.4.x.

Cheers,
Bo

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Juergen Spitzmueller]

Bo Peng wrote:
> I think lastpos is a better name to this feature

As long as it cannot be confused with cursor.lastpos()
(I didn't read your patch, so the comment might be irrelevant).

Jürgen

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Abdelrazak Younes]

Bo Peng a écrit :

We have 'lastfiles' and 'lastpos' as configurable filenames, I am not
sure that they should be. IMHO we should just hardcode them. Opinions
on that?


I just tried to make features of lastpos parallel to lastfiles. I do
not think these filenames need to be configured. lastfile::dostat may
not be needed either. Sure, people may lose lastfile entries under
some special circumstances but we donot have to worry too much about
those cases.


IMHO this is session management and all session management should go 
into a single file with a wide spread format (say XML or INI like).
Where you're at it Bo, could you implement the (optional) reloading of 
all file that where opened on last exit? ;-)


Abdel.

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

On 3/24/06, Abdelrazak Younes <[EMAIL PROTECTED]> wrote:
> could you implement the (optional) reloading of
> all file that where opened on last exit? ;-)

That is a separate feature that is definitely nice to have. It is
possible though, to make $HOME/.lyx/lastfilepos something like
*, 32, file1
*, 12, file2
45, file3

and, of course optionally, load file1 and file2 automatically when lyx starts.

Bo

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Georg Baum]

Abdelrazak Younes wrote:

> IMHO this is session management and all session management should go
> into a single file with a wide spread format (say XML or INI like).

I disagree a bit. Being able to open a file at the last position is not
session management IMHO. session management is "recreate the exact same
state as LyX was in before shutdown", and it should be implemented using
the qt/gtk API.

> Where you're at it Bo, could you implement the (optional) reloading of
> all file that where opened on last exit? ;-)

This is indeed session management.


Georg

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Abdelrazak Younes]

Georg Baum a écrit :

Abdelrazak Younes wrote:


IMHO this is session management and all session management should go
into a single file with a wide spread format (say XML or INI like).


I disagree a bit. Being able to open a file at the last position is not
session management IMHO. session management is "recreate the exact same
state as LyX was in before shutdown", and it should be implemented using
the qt/gtk API.


Indeed, from the Qt doc:

void QApplication::saveState ( QSessionManager & manager )  [virtual]
This function deals with session management. It is invoked when the 
session manager wants the application to preserve its state for a future 
session.
For example, a text editor would create a temporary file that includes 
the current contents of its edit buffers, the location of the cursor and 
other aspects of the current editing session.



Where you're at it Bo, could you implement the (optional) reloading of
all file that where opened on last exit? ;-)


This is indeed session management. 

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

> > Where you're at it Bo, could you implement the (optional) reloading of
> > all file that where opened on last exit? ;-)
>
> This is indeed session management.

If we only need to open files at their original positions, we do not
really need qt api etc. I can expand filepositions to something like

$HOME/.lyx/session

[closedfiles]
file1

[lastpos]
23, file1
129, file2

My patch will handle lastpos. A simple patch during lyx startup can
load all files.

Bo

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[John C. Spray]

On Fri, 2006-03-24 at 15:29 +0100, Georg Baum wrote:
> I disagree a bit. Being able to open a file at the last position is not
> session management IMHO. session management is "recreate the exact same
> state as LyX was in before shutdown", and it should be implemented using
> the qt/gtk API.

Do you mean things like "Was the math panel open?" and "Where was the
math panel?".  Seems like a good idea, although perhaps more basic
things like allowing hiding/showing of toolbars from the View menu would
be better done first.

John

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Georg Baum]

John C. Spray wrote:

> Do you mean things like "Was the math panel open?" and "Where was the
> math panel?".  Seems like a good idea, although perhaps more basic
> things like allowing hiding/showing of toolbars from the View menu would
> be better done first.

No. What I mean are more basic things: documents that were open, cursor
position in each document and unsaved changes in each document.
Things like window size, math panel position etc. would be a plus, but I
think that these could be handled by the toolkit.


Georg

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: Feature request: Remember the editing location when a file is closed.

[Jean-Marc Lasgouttes]

 christian == christian ridderstrom [EMAIL PROTECTED] writes:

christian On Wed, 22 Mar 2006, Bo Peng wrote:
 Dear list,
 
 I am not sure if others will like this idea. When I edit a long
 file, it may be troublesome to locate and continue from where I was
 working on, after the file is re-opened. I would be happy if lyx
 can automatically goes to the page at which lyx was closed.
 
 I can think of two ways to do this:
 
 1. (More intrusive). Remember the location under .lyx and jump to
 it when a lyx file is opened. I think this is what vim is doing.

christian For good or worse, this also means that when working with
christian someone else on the same file (that's sent back and forth,
christian or under version control), you'll open it up where the
christian other person left off...

Not if the data is saved in .lyx/ directory.

 2. (Less intrusive). Save bookmark with the lyx file. A user can
 save bookmark, close the file, and go to the bookmark after the
 file is re-opened.

christian Maybe file a bugzilla report? Or if you're brave, discss
christian which of the options would be best on the user's list :-)

I would not be surprised to see that this bug already exists.

JMarc

Re: Feature request: Remember the editing location when a file is closed.

[Jose' Matos]

On Wednesday 22 March 2006 22:44, [EMAIL PROTECTED] wrote:
 On Wed, 22 Mar 2006, Bo Peng wrote:
  Dear list,
 
  I am not sure if others will like this idea. When I edit a long file,
  it may be troublesome to locate and continue from where I was working
  on, after the file is re-opened. I would be happy if lyx can
  automatically goes to the page at which lyx was closed.
 
  I can think of two ways to do this:
 
  1. (More intrusive). Remember the location under .lyx and jump to it
  when a lyx file is opened. I think this is what vim is doing.

 For good or worse, this also means that when working with someone else on
 the same file (that's sent back and forth, or under version control),
 you'll open it up where the other person left off...

  Lars suggested that those values could be saved on .lyx directory. That 
would serve both purposes. :-)

  2. (Less intrusive). Save bookmark with the lyx file. A user can save
  bookmark, close the file, and go to the bookmark after the file is
  re-opened.

 Maybe file a bugzilla report? Or if you're brave, discss which of the
 options would be best on the user's list :-)

  This feature is orthogonal to the one above. Should bookmarks be persistent?

 /C

-- 
José Abílio

Re: Feature request: Remember the editing location when a file is closed.

[christian . ridderstrom]

On Thu, 23 Mar 2006, Jean-Marc Lasgouttes wrote:

  I can think of two ways to do this:
  
  1. (More intrusive). Remember the location under .lyx and jump to
  it when a lyx file is opened. I think this is what vim is doing.
 
 christian For good or worse, this also means that when working with
 christian someone else on the same file (that's sent back and forth,
 christian or under version control), you'll open it up where the
 christian other person left off...
 
 Not if the data is saved in .lyx/ directory.

Oops... I put my reply under the wrong solution. Just in case it's not
obvious to everybody, my comment should refer to when saving the bookmarks 
to the .lyx-file, not the .lyx-directory.

  2. (Less intrusive). Save bookmark with the lyx file. A user can
  save bookmark, close the file, and go to the bookmark after the
  file is re-opened.

/C

-- 
Christian Ridderström, +46-8-768 39 44   http://www.md.kth.se/~chr

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: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Jose' Matos [EMAIL PROTECTED] writes:

| On Wednesday 22 March 2006 22:44, [EMAIL PROTECTED] wrote:
|  On Wed, 22 Mar 2006, Bo Peng wrote:
|   Dear list,
|  
|   I am not sure if others will like this idea. When I edit a long file,
|   it may be troublesome to locate and continue from where I was working
|   on, after the file is re-opened. I would be happy if lyx can
|   automatically goes to the page at which lyx was closed.
|  
|   I can think of two ways to do this:
|  
|   1. (More intrusive). Remember the location under .lyx and jump to it
|   when a lyx file is opened. I think this is what vim is doing.
| 
|  For good or worse, this also means that when working with someone else on
|  the same file (that's sent back and forth, or under version control),
|  you'll open it up where the other person left off...
| 
|   Lars suggested that those values could be saved on .lyx directory. That 
| would serve both purposes. :-)

Both bookmarks and 'old-location' should be saved in .lyx, not in the
lyx file.

-- 
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:

| 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

[PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

Dear list,

This time, a formal patch. Here are the rationals:

 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.

Using a separate file like $HOME/.lyx/filepos means more source code
to change and maintain so I prefer the lastfile interface. In my
implementation, the first several lines are  for lastfiles, and the
rest of the lines will be loaded for file positions.

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

The bookmark is not saved in .lyx file.

 What do you need the menuitem for?

I though a menuitem would be less intrusive, but a rc entry should
serve our purpose better, especially when the menuitem will be used
only once. So, on a second thought, I removed this menu item.

 Please do not use paragraph id it is not stable. Perhaps use the
 cursor stack info instead.

Use pit() instead of id() now.

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

For two items, std::pair is better. I do not really like the get0() interface.

 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.

I do not know how to do it. It should be simple for you guys. ( I have
a FIXME in the patch)

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

I use pit() and paragraphs()[ pit() ].id() etc (see attached patch). I
am not quite confident about this part.

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

Done. With another entry num_positions. An rc entry for it should be
fine but I simply set it to 100.

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

Oops, copy/paste problem. But why did not my gcc complain?

The patch seems to be working. Please test it. Also, I really would
like the close-button to work as File-exit.

Cheers,
Bo
Index: src/BufferView_pimpl.C
===
--- src/BufferView_pimpl.C	(revision 13463)
+++ src/BufferView_pimpl.C	(working copy)
@@ -293,6 +293,20 @@
 
 	setBuffer(b);
 	bv_-showErrorList(_(Parse));
+
+	// scroll to the position when the file was last closed
+	// FIXME: this if(true) should be changed to a rcfile switch
+	// I do not know how to do it now.
+	if( true ) {
+		std::pairlyx::pit_type, lyx::pos_type pos = LyX::ref().lastfiles().loadFilePosition(s);
+		// p.get0 is pit, need to translate it to paragraph id.
+		ParagraphList prghs = b-paragraphs();
+		// be careful since the file may have been externally changed ...
+		if ( pos.first  prghs.size() ) {
+			saved_positions[0] = Position(s, prghs[pos.first].id(), pos.second);
+			restorePosition(0);
+		}
+	}
 
 	if (tolastfiles)
 		LyX::ref().lastfiles().newFile(b-fileName());
@@ -767,7 +781,15 @@
 	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, not that paragraph
+	// index instead of id is saved.
+	if (i == 0) {
+		LyX::ref().lastfiles().saveFilePosition(buffer_-fileName(),
+cursor_.pit(), 
+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,8 @@
 #include fstream
 #include iterator
 
+#include sstream
+
 namespace fs = boost::filesystem;
 
 using std::copy;
@@ -26,13 +28,13 @@
 using std::find;
 using std::getline;
 using std::string;
+using std::map;
 using std::ifstream;
 using std::ofstream;
-using std::ostream_iterator;
+using std::istringstream;
 
-
 LastFiles::LastFiles(string const  filename, bool st, unsigned int num)
-	: dostat(st)
+	: num_positions(100), dostat(st)
 {
 	setNumberOfFiles(num);
 	readFile(filename);
@@ -59,10 +61,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 ) {
+			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;
+			if (file_positions.size()  num_positions )
+file_positions[fname] = FilePos(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;
-		

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Bo Peng [EMAIL PROTECTED] writes:

| Dear list,
| 
| This time, a formal patch. Here are the rationals:
| 
|  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.
| 
| Using a separate file like $HOME/.lyx/filepos means more source code
| to change and maintain so I prefer the lastfile interface.
| In my
| implementation, the first several lines are  for lastfiles, and the
| rest of the lines will be loaded for file positions.

I do not like this one bit.
IMO completely ortogonal feature that happens to share some of the
omplementation. IMHO you have just made a super simple LastFiles a lot
more complex.

|  bookmark0? where is that stored? if it in the lyx document, then
|  please no.
| 
| The bookmark is not saved in .lyx file.

Ok. IMHO this lastpos feature should not be mixed with bookmarks.

|  Please do not use paragraph id it is not stable. Perhaps use the
|  cursor stack info instead.
| 
| Use pit() instead of id() now.

You won't be able to please the cursor accurately then.
 
|  I must admit that I prefere to not use in/out parameters.
|  I like tupples though :-)  (boost::tuple)
| 
| For two items, std::pair is better. I do not really like the
| get0() interface.

boost::tie

| 
|  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.
| 
| I do not know how to do it. It should be simple for you guys. ( I have
| a FIXME in the patch)

To add it to the lyxrc is easy enough... it is getting it into the gui
frontends that is hard...
 
|  Using paragraph id is icky. Please use offsets into the paragraphlist
|  instead. You should be able to get that from the cursor.
| 
| I use pit() and paragraphs()[ pit() ].id() etc (see attached patch). I
| am not quite confident about this part.
| 
|  This is a no-go. This information must be in the LastFiles object
|  (class).
| 
| Done. With another entry num_positions. An rc entry for it should be
| fine but I simply set it to 100.
| 
|  Using ClassName::Func in the class decl. is a syntax error. Just use
|  'Func' instead.
| 
| Oops, copy/paste problem. But why did not my gcc complain?

Too old gcc. I think only gcc  4.1 warns/errors about this.

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Jose' Matos]

On Thursday 23 March 2006 18:39, Lars Gullik Bjønnes wrote:
 Too old gcc. I think only gcc  4.1 warns/errors about this.

 gcc = 4.1

  :-)

 --
 Lgb

-- 
José Abílio

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Bo Peng [EMAIL PROTECTED] writes:

| Index: src/BufferView_pimpl.C
| ===
| --- src/BufferView_pimpl.C(revision 13463)
| +++ src/BufferView_pimpl.C(working copy)
| @@ -293,6 +293,20 @@
|  
|   setBuffer(b);
|   bv_-showErrorList(_(Parse));
| +
| + // scroll to the position when the file was last closed
| + // FIXME: this if(true) should be changed to a rcfile switch
| + // I do not know how to do it now.
| + if( true ) {
| + std::pairlyx::pit_type, lyx::pos_type pos = 
LyX::ref().lastfiles().loadFilePosition(s);

Alternatively, more similar to what you had earlier:

int pit;
int pos;
boost::tie(pit, pos) = LyX::ref().lastfiles().loadFilePosition(s);

| + // p.get0 is pit, need to translate it to paragraph id.

pos.first

| + ParagraphList prghs = b-paragraphs();
| + // be careful since the file may have been externally changed 
...
| + if ( pos.first  prghs.size() ) {
| + saved_positions[0] = Position(s, prghs[pos.first].id(), 
pos.second);
| + restorePosition(0);
| + }
| + }

Can you please drop the bookmark part of the patch for now?
If we decide that we do want bookmark[0] to be the position the cursor
was in when the doc was last saved we can put this in later.
(but... I just realized that you rely on this in your
implementation... hmm)

We actually want to get rid of the whole paragraph.id thing. It is not
stable enough. I have some (vague) plans of introducing a
(universally) unique id to use on paragraphs.

Just as a test... load a document move the cursor save.
Exit lyx. Start lyx. Load a different document. Load the first doc.
Where is the cursor placed now?

| Index: src/lastfiles.C

Doesn't this almost double the size of lastfiles.C?
(I am just trying to prove that code complexity is not reduced it IMNO
increases a lot.)

Note that despite all my negativism I am super happy that you have
picked up this ball. This is a feature I have wanted for a long time,
but have never found the time to implement.

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

 I do not like this one bit.
 IMO completely ortogonal feature that happens to share some of the
 omplementation. IMHO you have just made a super simple LastFiles a lot
 more complex.

It is much more troublesome to have a separate implementation.
Afterall, lastfiles do mean information regarding files we have
edited.

 | Use pit() instead of id() now.

 You won't be able to please the cursor accurately then.

I do not understand why the cursor can not be placed accurately. But
it is good enough to scroll to the beginning of a paragraph so I can
actually ignore the cursor position part.

I also noticed some other problems:

1. Why closeBuffer() is not called when lyx exits through File-Exit?
I think I need to put view()-savePosition(0) somewhere else.

2. When a file is passed through commond line (lyx file.lyx), my patch
does automatically scroll to the  saved location but the file still
appears from the beginning. Why does this happen?

Cheers,
Bo

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

 Alternatively, more similar to what you had earlier:

 int pit;
 int pos;
 boost::tie(pit, pos) = LyX::ref().lastfiles().loadFilePosition(s);

This is good to know.

 Can you please drop the bookmark part of the patch for now?
 If we decide that we do want bookmark[0] to be the position the cursor
 was in when the doc was last saved we can put this in later.
 (but... I just realized that you rely on this in your
 implementation... hmm)

No problem. I can copy part of restorePosition to here ...

 Just as a test... load a document move the cursor save.
 Exit lyx. Start lyx. Load a different document. Load the first doc.
 Where is the cursor placed now?

It will move to the position when the first file was closed. My test
goes well except that lyx file.lyx does not scroll as expected.

 Doesn't this almost double the size of lastfiles.C?
 (I am just trying to prove that code complexity is not reduced it IMNO
 increases a lot.)

I added about 50 lines. Now the code is 155 lines. A separate
fileposition.C will be this long as well, and we need to change the
lyx().lastfile() interface.

Cheers,
Bo

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Bo Peng [EMAIL PROTECTED] writes:

|  I do not like this one bit.
|  IMO completely ortogonal feature that happens to share some of the
|  omplementation. IMHO you have just made a super simple LastFiles a lot
|  more complex.
| 
| It is much more troublesome to have a separate implementation.
| Afterall, lastfiles do mean information regarding files we have
| edited.

Only which files not any information about them.

And I am not going to stop bitching about using a separate file...

|  | Use pit() instead of id() now.
| 
|  You won't be able to please the cursor accurately then.
| 
| I do not understand why the cursor can not be placed accurately. But
| it is good enough to scroll to the beginning of a paragraph so I can
| actually ignore the cursor position part.

Don't you only look-up the outer paragraph list? What if the cursor is
nested (deeply) inside insets?

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Bo Peng [EMAIL PROTECTED] writes:

|  Just as a test... load a document move the cursor save.
|  Exit lyx. Start lyx. Load a different document. Load the first doc.
|  Where is the cursor placed now?
| 
| It will move to the position when the first file was closed. My test
| goes well except that lyx file.lyx does not scroll as expected.

The comment above aimed at the 'use par.id()' for lastpos situation.

pit is a lot more stable.

| 
|  Doesn't this almost double the size of lastfiles.C?
|  (I am just trying to prove that code complexity is not reduced it IMNO
|  increases a lot.)
| 
| I added about 50 lines. Now the code is 155 lines.

And of this some ~40 lines is copyrigh and headers. You have doubled
the code size of the file.

| A separate
| fileposition.C will be this long as well, and we need to change the
| lyx().lastfile() interface.

No. We need to have the last position feature in addition, not mixed
with lastfile.

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

 No. We need to have the last position feature in addition, not mixed
 with lastfile.

OK, since you insist, here comes a much larger patch.

The patch works on my system. However, there are two problems that
make this feature almost useless. Please help me resolve them.

1. Alt-F4 and close-button do not trigger LDUNC_QUIT or bufferClose()
so position will not be saved. Note that Alt-F4 is marked as a
shortcut for File-Exit but it behaves differently than File-Exit.

2. If a filename is passed to lyx as 'lyx file.lyx', the cursor will
be moved by my patch, but then moved back by some unknown mechanism
(seems to be some GUI stuff). This is a big drawback since this is the
main way I start lyx.

Cheers,
Bo
Index: src/lyx_cb.C
===
--- src/lyx_cb.C	(revision 13470)
+++ src/lyx_cb.C	(working copy)
@@ -23,6 +23,7 @@
 #include debug.h
 #include gettext.h
 #include lastfiles.h
+#include filepositions.h
 #include LaTeXFeatures.h
 #include lyx_main.h
 #include lyxlayout.h
@@ -197,6 +198,7 @@
 			return;
 
 		LyX::cref().lastfiles().writeFile(lyxrc.lastfiles);
+		LyX::cref().filepositions().writeFile(lyxrc.filepositions);
 	}
 
 	// Set a flag that we do quitting from the program,
Index: src/BufferView_pimpl.C
===
--- src/BufferView_pimpl.C	(revision 13470)
+++ src/BufferView_pimpl.C	(working copy)
@@ -43,6 +43,7 @@
 #include lyxtext.h
 #include lyxrc.h
 #include lastfiles.h
+#include filepositions.h
 #include metricsinfo.h
 #include paragraph.h
 #include paragraph_funcs.h
@@ -293,6 +294,23 @@
 
 	setBuffer(b);
 	bv_-showErrorList(_(Parse));
+
+	// scroll to the position when the file was last closed
+	if (lyxrc.use_filepos) {
+		lyx::pit_type pit = LyX::ref().filepositions().loadFilePosition(s);
+		// move to the beginning of that paragraph
+		// be careful since the file may have been externally changed ...
+		if ( static_castsize_t(pit)  b-paragraphs().size() ) {
+			cursor_.clearSelection();
+			ParIterator it = b-par_iterator_begin();
+			ParIterator const end = b-par_iterator_end();
+			for (; it != end; ++it)
+if (it.pit() == pit)
+	break;
+			if (it != end)		
+bv_-setCursor(makeDocIterator(it, 0));
+		}
+	}
 
 	if (tolastfiles)
 		LyX::ref().lastfiles().newFile(b-fileName());
@@ -767,7 +785,12 @@
 	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 paragraph pit to filepositions
+	if (i == 0)
+		LyX::ref().filepositions().saveFilePosition(buffer_-fileName(),
+cursor_.pit() );
+	else
 		owner_-message(bformat(_(Saved bookmark %1$d), i));
 }
 
Index: src/lyxfunc.C
===
--- src/lyxfunc.C	(revision 13470)
+++ src/lyxfunc.C	(working copy)
@@ -996,6 +996,11 @@
 			break;
 
 		case LFUN_QUIT:
+			// with this, File-exit will save Position,
+			// however, Alt-F4 and close-button do not go 
+			// through here ...
+			if (view()-available())
+view()-savePosition(0);
 			QuitLyX(argument == force);
 			break;
 
@@ -1880,6 +1885,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
@@ -2008,6 +2015,7 @@
 	case LyXRC::RC_LANGUAGE_PACKAGE:
 	case LyXRC::RC_LANGUAGE_USE_BABEL:
 	case LyXRC::RC_LASTFILES:
+	case LyXRC::RC_FILEPOSITIONS:
 	case LyXRC::RC_MAKE_BACKUP:
 	case LyXRC::RC_MARK_FOREIGN_LANGUAGE:
 	case LyXRC::RC_NUMLASTFILES:
Index: src/Makefile.am
===
--- src/Makefile.am	(revision 13470)
+++ src/Makefile.am	(working copy)
@@ -191,6 +191,8 @@
 	language.h \
 	lastfiles.C \
 	lastfiles.h \
+	filepositions.C \
+	filepositions.h \
 	layout.h \
 	lengthcommon.C \
 	lengthcommon.h \
Index: src/lyxrc.C
===
--- src/lyxrc.C	(revision 13470)
+++ src/lyxrc.C	(working copy)
@@ -26,6 +26,7 @@
 #include format.h
 #include gettext.h
 #include lastfiles.h
+#include filepositions.h
 #include LColor.h
 #include lyxlex.h
 #include lyxfont.h
@@ -105,6 +106,8 @@
 	{ \\language_package, LyXRC::RC_LANGUAGE_PACKAGE },
 	{ \\language_use_babel, LyXRC::RC_LANGUAGE_USE_BABEL },
 	{ \\lastfiles, LyXRC::RC_LASTFILES },
+	{ \\use_filepos, LyXRC::RC_USEFILEPOS },
+	{ \\filepositions, LyXRC::RC_FILEPOSITIONS },
 	{ \\make_backup, LyXRC::RC_MAKE_BACKUP },
 	{ \\mark_foreign_language, LyXRC::RC_MARK_FOREIGN_LANGUAGE },
 	{ \\num_lastfiles, LyXRC::RC_NUMLASTFILES },
@@ -246,6 +249,7 @@
 	ascii_linelen = 65;
 	num_lastfiles = maxlastfiles;
 	check_lastfiles = true;
+	use_filepos = true;
 	make_backup = true;
 	backupdir_path.erase();
 	display_graphics = 

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Bo Peng [EMAIL PROTECTED] writes:

|  No. We need to have the last position feature in addition, not mixed
|  with lastfile.
| 
| OK, since you insist, here comes a much larger patch.

I like this patch a lot better. Thanks for doing this.

One thing we should think about though; not caused by this patch, but
releated to it:

We have 'lastfiles' and 'lastpos' as configurable filenames, I am not
sure that they should be. IMHO we should just hardcode them. Opinions
on that?
 
| The patch works on my system. However, there are two problems that
| make this feature almost useless. Please help me resolve them.
 
| 1. Alt-F4 and close-button do not trigger LDUNC_QUIT or bufferClose()
| so position will not be saved. Note that Alt-F4 is marked as a
| shortcut for File-Exit but it behaves differently than File-Exit.

This not triggering, is it the same on all frontends?
 
| 2. If a filename is passed to lyx as 'lyx file.lyx', the cursor will
| be moved by my patch, but then moved back by some unknown mechanism
| (seems to be some GUI stuff). This is a big drawback since this is the
| main way I start lyx.

Ok, we need to investigate a bit.
Could it be that a -setCursor is also called later, so that your
setting is overwritten?

| Index: src/BufferView_pimpl.C
| ===
| --- src/BufferView_pimpl.C(revision 13470)
| +++ src/BufferView_pimpl.C(working copy)
| @@ -293,6 +294,23 @@
|  
|   setBuffer(b);
|   bv_-showErrorList(_(Parse));
| +
| + // scroll to the position when the file was last closed
| + if (lyxrc.use_filepos) {
| + lyx::pit_type pit = 
LyX::ref().filepositions().loadFilePosition(s);
| + // move to the beginning of that paragraph
| + // be careful since the file may have been externally changed 
...
| + if ( static_castsize_t(pit)  b-paragraphs().size() ) {
| + cursor_.clearSelection();
| + ParIterator it = b-par_iterator_begin();
| + ParIterator const end = b-par_iterator_end();
| + for (; it != end; ++it)
| + if (it.pit() == pit)
| + break;
| + if (it != end)  
| + bv_-setCursor(makeDocIterator(it, 0));

I am not sure that the look is needed (even for safety).
I think that a it = b-paragraphs()[pit] would be enough.
(might have to be written as something else, but eh above would be the
meaning.)

Also, is the clearSelection ever needed? This is upon file load. Can a
selection ever be set?

| @@ -767,7 +785,12 @@
|   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 paragraph pit to filepositions
| + if (i == 0)
| + LyX::ref().filepositions().saveFilePosition(buffer_-fileName(),
| + cursor_.pit() );
| + else
|   owner_-message(bformat(_(Saved bookmark %1$d), i));
|  }

As said I don't like this mix of features, but as it is strictly an
internal thing, let's handle that one later.

(I am dreaming of a .lyx/bookmarks file as well.)

| Index: src/filepositions.C
| ===
| --- src/filepositions.C   (revision 0)
| +++ src/filepositions.C   (revision 0)
| @@ -0,0 +1,86 @@
| +/**
| + * \file filespositions.C
| + * This file is part of LyX, the document processor.
| + * Licence details can be found in the file COPYING.
| + *
| + * \author Bo Peng
| + *
| + * Full author contact details are available in file CREDITS.
| + */
| +
| +#include config.h
| +
| +#include filepositions.h
| +#include debug.h
| +
| +#include boost/filesystem/operations.hpp
| +
| +#include fstream
| +#include sstream
| +
| +using std::getline;
| +using std::string;
| +using std::ifstream;
| +using std::ofstream;
| +using std::endl;
| +using std::istringstream;
| +
| +FilePositions::FilePositions(string const  filename)
| + : num_positions(100)
| +{
| + readFile(filename);
| +}
| +
| +void FilePositions::readFile(string const  filename)
| +{
| + // we will not complain if we can't find filename nor will
| + // we issue a warning. (Lgb)
| + ifstream ifs(filename.c_str());
| + string tmp;
| +
| + while (getline(ifs, tmp)  file_positions.size()  num_positions ) {
| + // pos, filename\n
| + FilePos pos;
| + string fname;
| + istringstream itmp(tmp);
| + itmp  pos;
| + itmp.ignore(2);  // ingore ,

Typo in comment. Also are you not ignoring , ?

| +void FilePositions::saveFilePosition(string const fname, FilePos
| pos )

space before '' please

| +{
| + 

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: Feature request: Remember the editing location when a file is closed.

[Jean-Marc Lasgouttes]

> "christian" == christian ridderstrom <[EMAIL PROTECTED]> writes:

christian> On Wed, 22 Mar 2006, Bo Peng wrote:
>> Dear list,
>> 
>> I am not sure if others will like this idea. When I edit a long
>> file, it may be troublesome to locate and continue from where I was
>> working on, after the file is re-opened. I would be happy if lyx
>> can automatically goes to the page at which lyx was closed.
>> 
>> I can think of two ways to do this:
>> 
>> 1. (More intrusive). Remember the location under .lyx and jump to
>> it when a lyx file is opened. I think this is what vim is doing.

christian> For good or worse, this also means that when working with
christian> someone else on the same file (that's sent back and forth,
christian> or under version control), you'll open it up where the
christian> other person left off...

Not if the data is saved in .lyx/ directory.

>> 2. (Less intrusive). Save bookmark with the lyx file. A user can
>> save bookmark, close the file, and go to the bookmark after the
>> file is re-opened.

christian> Maybe file a bugzilla report? Or if you're brave, discss
christian> which of the options would be best on the user's list :-)

I would not be surprised to see that this bug already exists.

JMarc

Re: Feature request: Remember the editing location when a file is closed.

[Jose' Matos]

On Wednesday 22 March 2006 22:44, [EMAIL PROTECTED] wrote:
> On Wed, 22 Mar 2006, Bo Peng wrote:
> > Dear list,
> >
> > I am not sure if others will like this idea. When I edit a long file,
> > it may be troublesome to locate and continue from where I was working
> > on, after the file is re-opened. I would be happy if lyx can
> > automatically goes to the page at which lyx was closed.
> >
> > I can think of two ways to do this:
> >
> > 1. (More intrusive). Remember the location under .lyx and jump to it
> > when a lyx file is opened. I think this is what vim is doing.
>
> For good or worse, this also means that when working with someone else on
> the same file (that's sent back and forth, or under version control),
> you'll open it up where the other person left off...

  Lars suggested that those values could be saved on .lyx directory. That 
would serve both purposes. :-)

> > 2. (Less intrusive). Save bookmark with the lyx file. A user can save
> > bookmark, close the file, and go to the bookmark after the file is
> > re-opened.
>
> Maybe file a bugzilla report? Or if you're brave, discss which of the
> options would be best on the user's list :-)

  This feature is orthogonal to the one above. Should bookmarks be persistent?

> /C

-- 
José Abílio

Re: Feature request: Remember the editing location when a file is closed.

[christian . ridderstrom]

On Thu, 23 Mar 2006, Jean-Marc Lasgouttes wrote:

> >> I can think of two ways to do this:
> >> 
> >> 1. (More intrusive). Remember the location under .lyx and jump to
> >> it when a lyx file is opened. I think this is what vim is doing.
> 
> christian> For good or worse, this also means that when working with
> christian> someone else on the same file (that's sent back and forth,
> christian> or under version control), you'll open it up where the
> christian> other person left off...
> 
> Not if the data is saved in .lyx/ directory.

Oops... I put my reply under the wrong solution. Just in case it's not
obvious to everybody, my comment should refer to when saving the bookmarks 
to the .lyx-file, not the .lyx-directory.

> >> 2. (Less intrusive). Save bookmark with the lyx file. A user can
> >> save bookmark, close the file, and go to the bookmark after the
> >> file is re-opened.

/C

-- 
Christian Ridderström, +46-8-768 39 44   http://www.md.kth.se/~chr

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: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

Jose' Matos <[EMAIL PROTECTED]> writes:

| On Wednesday 22 March 2006 22:44, [EMAIL PROTECTED] wrote:
| > On Wed, 22 Mar 2006, Bo Peng wrote:
| > > Dear list,
| > >
| > > I am not sure if others will like this idea. When I edit a long file,
| > > it may be troublesome to locate and continue from where I was working
| > > on, after the file is re-opened. I would be happy if lyx can
| > > automatically goes to the page at which lyx was closed.
| > >
| > > I can think of two ways to do this:
| > >
| > > 1. (More intrusive). Remember the location under .lyx and jump to it
| > > when a lyx file is opened. I think this is what vim is doing.
| >
| > For good or worse, this also means that when working with someone else on
| > the same file (that's sent back and forth, or under version control),
| > you'll open it up where the other person left off...
| 
|   Lars suggested that those values could be saved on .lyx directory. That 
| would serve both purposes. :-)

Both bookmarks and 'old-location' should be saved in .lyx, not in the
lyx file.

-- 
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:

| 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

[PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

Dear list,

This time, a formal patch. Here are the rationals:

> 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.

Using a separate file like $HOME/.lyx/filepos means more source code
to change and maintain so I prefer the lastfile interface. In my
implementation, the first several lines are  for lastfiles, and the
rest of the lines will be loaded for file positions.

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

The bookmark is not saved in .lyx file.

> What do you need the menuitem for?

I though a menuitem would be less intrusive, but a rc entry should
serve our purpose better, especially when the menuitem will be used
only once. So, on a second thought, I removed this menu item.

> Please do not use paragraph id it is not stable. Perhaps use the
> cursor stack info instead.

Use pit() instead of id() now.

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

For two items, std::pair is better. I do not really like the get<0>() interface.

> 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.

I do not know how to do it. It should be simple for you guys. ( I have
a FIXME in the patch)

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

I use pit() and paragraphs()[ pit() ].id() etc (see attached patch). I
am not quite confident about this part.

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

Done. With another entry num_positions. An rc entry for it should be
fine but I simply set it to 100.

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

Oops, copy/paste problem. But why did not my gcc complain?

The patch seems to be working. Please test it. Also, I really would
like the close-button to work as File->exit.

Cheers,
Bo
Index: src/BufferView_pimpl.C
===
--- src/BufferView_pimpl.C	(revision 13463)
+++ src/BufferView_pimpl.C	(working copy)
@@ -293,6 +293,20 @@
 
 	setBuffer(b);
 	bv_->showErrorList(_("Parse"));
+
+	// scroll to the position when the file was last closed
+	// FIXME: this "if(true)" should be changed to a rcfile switch
+	// I do not know how to do it now.
+	if( true ) {
+		std::pair pos = LyX::ref().lastfiles().loadFilePosition(s);
+		// p.get<0> is pit, need to translate it to paragraph id.
+		ParagraphList& prghs = b->paragraphs();
+		// be careful since the file may have been externally changed ...
+		if ( pos.first < prghs.size() ) {
+			saved_positions[0] = Position(s, prghs[pos.first].id(), pos.second);
+			restorePosition(0);
+		}
+	}
 
 	if (tolastfiles)
 		LyX::ref().lastfiles().newFile(b->fileName());
@@ -767,7 +781,15 @@
 	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, not that paragraph
+	// index instead of id is saved.
+	if (i == 0) {
+		LyX::ref().lastfiles().saveFilePosition(buffer_->fileName(),
+cursor_.pit(), 
+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,8 @@
 #include 
 #include 
 
+#include 
+
 namespace fs = boost::filesystem;
 
 using std::copy;
@@ -26,13 +28,13 @@
 using std::find;
 using std::getline;
 using std::string;
+using std::map;
 using std::ifstream;
 using std::ofstream;
-using std::ostream_iterator;
+using std::istringstream;
 
-
 LastFiles::LastFiles(string const & filename, bool st, unsigned int num)
-	: dostat(st)
+	: num_positions(100), dostat(st)
 {
 	setNumberOfFiles(num);
 	readFile(filename);
@@ -59,10 +61,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 ) {
+			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;
+			if (file_positions.size() < num_positions )
+file_positions[fname] = FilePos(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 

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

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

| Dear list,
| 
| This time, a formal patch. Here are the rationals:
| 
| > 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.
| 
| Using a separate file like $HOME/.lyx/filepos means more source code
| to change and maintain so I prefer the lastfile interface.
| In my
| implementation, the first several lines are  for lastfiles, and the
| rest of the lines will be loaded for file positions.

I do not like this one bit.
IMO completely ortogonal feature that happens to share some of the
omplementation. IMHO you have just made a super simple LastFiles a lot
more complex.

| > bookmark0? where is that stored? if it in the lyx document, then
| > please no.
| 
| The bookmark is not saved in .lyx file.

Ok. IMHO this lastpos feature should not be mixed with bookmarks.

| > Please do not use paragraph id it is not stable. Perhaps use the
| > cursor stack info instead.
| 
| Use pit() instead of id() now.

You won't be able to please the cursor accurately then.
 
| > I must admit that I prefere to not use in/out parameters.
| > I like tupples though :-)  (boost::tuple)
| 
| For two items, std::pair is better. I do not really like the
| get<0>() interface.

boost::tie

| 
| > 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.
| 
| I do not know how to do it. It should be simple for you guys. ( I have
| a FIXME in the patch)

To add it to the lyxrc is easy enough... it is getting it into the gui
frontends that is hard...
 
| > Using paragraph id is icky. Please use offsets into the paragraphlist
| > instead. You should be able to get that from the cursor.
| 
| I use pit() and paragraphs()[ pit() ].id() etc (see attached patch). I
| am not quite confident about this part.
| 
| > This is a no-go. This information must be in the LastFiles object
| > (class).
| 
| Done. With another entry num_positions. An rc entry for it should be
| fine but I simply set it to 100.
| 
| > Using "ClassName::Func" in the class decl. is a syntax error. Just use
| > 'Func' instead.
| 
| Oops, copy/paste problem. But why did not my gcc complain?

Too old gcc. I think only gcc > 4.1 warns/errors about this.

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Jose' Matos]

On Thursday 23 March 2006 18:39, Lars Gullik Bjønnes wrote:
> Too old gcc. I think only gcc > 4.1 warns/errors about this.

 gcc >= 4.1

  :-)

> --
> Lgb

-- 
José Abílio

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

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

| Index: src/BufferView_pimpl.C
| ===
| --- src/BufferView_pimpl.C(revision 13463)
| +++ src/BufferView_pimpl.C(working copy)
| @@ -293,6 +293,20 @@
|  
|   setBuffer(b);
|   bv_->showErrorList(_("Parse"));
| +
| + // scroll to the position when the file was last closed
| + // FIXME: this "if(true)" should be changed to a rcfile switch
| + // I do not know how to do it now.
| + if( true ) {
| + std::pair pos = 
LyX::ref().lastfiles().loadFilePosition(s);

Alternatively, more similar to what you had earlier:

int pit;
int pos;
boost::tie(pit, pos) = LyX::ref().lastfiles().loadFilePosition(s);

| + // p.get<0> is pit, need to translate it to paragraph id.

pos.first

| + ParagraphList& prghs = b->paragraphs();
| + // be careful since the file may have been externally changed 
...
| + if ( pos.first < prghs.size() ) {
| + saved_positions[0] = Position(s, prghs[pos.first].id(), 
pos.second);
| + restorePosition(0);
| + }
| + }

Can you please drop the bookmark part of the patch for now?
If we decide that we do want bookmark[0] to be the position the cursor
was in when the doc was last saved we can put this in later.
(but... I just realized that you rely on this in your
implementation... hmm)

We actually want to get rid of the whole paragraph.id thing. It is not
stable enough. I have some (vague) plans of introducing a
(universally) unique id to use on paragraphs.

Just as a test... load a document move the cursor save.
Exit lyx. Start lyx. Load a different document. Load the first doc.
Where is the cursor placed now?

| Index: src/lastfiles.C

Doesn't this almost double the size of lastfiles.C?
(I am just trying to prove that code complexity is not reduced it IMNO
increases a lot.)

Note that despite all my negativism I am super happy that you have
picked up this ball. This is a feature I have wanted for a long time,
but have never found the time to implement.

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

> I do not like this one bit.
> IMO completely ortogonal feature that happens to share some of the
> omplementation. IMHO you have just made a super simple LastFiles a lot
> more complex.

It is much more troublesome to have a separate implementation.
Afterall, lastfiles do mean information regarding files we have
edited.

> | Use pit() instead of id() now.
>
> You won't be able to please the cursor accurately then.

I do not understand why the cursor can not be placed accurately. But
it is good enough to scroll to the beginning of a paragraph so I can
actually ignore the cursor position part.

I also noticed some other problems:

1. Why closeBuffer() is not called when lyx exits through File->Exit?
I think I need to put view()->savePosition(0) somewhere else.

2. When a file is passed through commond line (lyx file.lyx), my patch
does automatically scroll to the  saved location but the file still
appears from the beginning. Why does this happen?

Cheers,
Bo

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

> Alternatively, more similar to what you had earlier:
>
> int pit;
> int pos;
> boost::tie(pit, pos) = LyX::ref().lastfiles().loadFilePosition(s);

This is good to know.

> Can you please drop the bookmark part of the patch for now?
> If we decide that we do want bookmark[0] to be the position the cursor
> was in when the doc was last saved we can put this in later.
> (but... I just realized that you rely on this in your
> implementation... hmm)

No problem. I can copy part of restorePosition to here ...

> Just as a test... load a document move the cursor save.
> Exit lyx. Start lyx. Load a different document. Load the first doc.
> Where is the cursor placed now?

It will move to the position when the first file was closed. My test
goes well except that "lyx file.lyx" does not scroll as expected.

> Doesn't this almost double the size of lastfiles.C?
> (I am just trying to prove that code complexity is not reduced it IMNO
> increases a lot.)

I added about 50 lines. Now the code is 155 lines. A separate
fileposition.C will be this long as well, and we need to change the
lyx().lastfile() interface.

Cheers,
Bo

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

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

| > I do not like this one bit.
| > IMO completely ortogonal feature that happens to share some of the
| > omplementation. IMHO you have just made a super simple LastFiles a lot
| > more complex.
| 
| It is much more troublesome to have a separate implementation.
| Afterall, lastfiles do mean information regarding files we have
| edited.

Only which files not any information about them.

And I am not going to stop bitching about using a separate file...

| > | Use pit() instead of id() now.
| >
| > You won't be able to please the cursor accurately then.
| 
| I do not understand why the cursor can not be placed accurately. But
| it is good enough to scroll to the beginning of a paragraph so I can
| actually ignore the cursor position part.

Don't you only look-up the outer paragraph list? What if the cursor is
nested (deeply) inside insets?

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

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

| > Just as a test... load a document move the cursor save.
| > Exit lyx. Start lyx. Load a different document. Load the first doc.
| > Where is the cursor placed now?
| 
| It will move to the position when the first file was closed. My test
| goes well except that "lyx file.lyx" does not scroll as expected.

The comment above aimed at the 'use par.id()' for lastpos situation.

pit is a lot more stable.

| 
| > Doesn't this almost double the size of lastfiles.C?
| > (I am just trying to prove that code complexity is not reduced it IMNO
| > increases a lot.)
| 
| I added about 50 lines. Now the code is 155 lines.

And of this some ~40 lines is copyrigh and headers. You have doubled
the "code size" of the file.

| A separate
| fileposition.C will be this long as well, and we need to change the
| lyx().lastfile() interface.

No. We need to have the "last position" feature in addition, not mixed
with lastfile.

-- 
Lgb

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Bo Peng]

> No. We need to have the "last position" feature in addition, not mixed
> with lastfile.

OK, since you insist, here comes a much larger patch.

The patch works on my system. However, there are two problems that
make this feature almost useless. Please help me resolve them.

1. Alt-F4 and close-button do not trigger LDUNC_QUIT or bufferClose()
so position will not be saved. Note that Alt-F4 is marked as a
shortcut for File->Exit but it behaves differently than File->Exit.

2. If a filename is passed to lyx as 'lyx file.lyx', the cursor will
be moved by my patch, but then moved back by some unknown mechanism
(seems to be some GUI stuff). This is a big drawback since this is the
main way I start lyx.

Cheers,
Bo
Index: src/lyx_cb.C
===
--- src/lyx_cb.C	(revision 13470)
+++ src/lyx_cb.C	(working copy)
@@ -23,6 +23,7 @@
 #include "debug.h"
 #include "gettext.h"
 #include "lastfiles.h"
+#include "filepositions.h"
 #include "LaTeXFeatures.h"
 #include "lyx_main.h"
 #include "lyxlayout.h"
@@ -197,6 +198,7 @@
 			return;
 
 		LyX::cref().lastfiles().writeFile(lyxrc.lastfiles);
+		LyX::cref().filepositions().writeFile(lyxrc.filepositions);
 	}
 
 	// Set a flag that we do quitting from the program,
Index: src/BufferView_pimpl.C
===
--- src/BufferView_pimpl.C	(revision 13470)
+++ src/BufferView_pimpl.C	(working copy)
@@ -43,6 +43,7 @@
 #include "lyxtext.h"
 #include "lyxrc.h"
 #include "lastfiles.h"
+#include "filepositions.h"
 #include "metricsinfo.h"
 #include "paragraph.h"
 #include "paragraph_funcs.h"
@@ -293,6 +294,23 @@
 
 	setBuffer(b);
 	bv_->showErrorList(_("Parse"));
+
+	// scroll to the position when the file was last closed
+	if (lyxrc.use_filepos) {
+		lyx::pit_type pit = LyX::ref().filepositions().loadFilePosition(s);
+		// move to the beginning of that paragraph
+		// be careful since the file may have been externally changed ...
+		if ( static_cast(pit) < b->paragraphs().size() ) {
+			cursor_.clearSelection();
+			ParIterator it = b->par_iterator_begin();
+			ParIterator const end = b->par_iterator_end();
+			for (; it != end; ++it)
+if (it.pit() == pit)
+	break;
+			if (it != end)		
+bv_->setCursor(makeDocIterator(it, 0));
+		}
+	}
 
 	if (tolastfiles)
 		LyX::ref().lastfiles().newFile(b->fileName());
@@ -767,7 +785,12 @@
 	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 paragraph pit to filepositions
+	if (i == 0)
+		LyX::ref().filepositions().saveFilePosition(buffer_->fileName(),
+cursor_.pit() );
+	else
 		owner_->message(bformat(_("Saved bookmark %1$d"), i));
 }
 
Index: src/lyxfunc.C
===
--- src/lyxfunc.C	(revision 13470)
+++ src/lyxfunc.C	(working copy)
@@ -996,6 +996,11 @@
 			break;
 
 		case LFUN_QUIT:
+			// with this, File->exit will save Position,
+			// however, Alt-F4 and close-button do not go 
+			// through here ...
+			if (view()->available())
+view()->savePosition(0);
 			QuitLyX(argument == "force");
 			break;
 
@@ -1880,6 +1885,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
@@ -2008,6 +2015,7 @@
 	case LyXRC::RC_LANGUAGE_PACKAGE:
 	case LyXRC::RC_LANGUAGE_USE_BABEL:
 	case LyXRC::RC_LASTFILES:
+	case LyXRC::RC_FILEPOSITIONS:
 	case LyXRC::RC_MAKE_BACKUP:
 	case LyXRC::RC_MARK_FOREIGN_LANGUAGE:
 	case LyXRC::RC_NUMLASTFILES:
Index: src/Makefile.am
===
--- src/Makefile.am	(revision 13470)
+++ src/Makefile.am	(working copy)
@@ -191,6 +191,8 @@
 	language.h \
 	lastfiles.C \
 	lastfiles.h \
+	filepositions.C \
+	filepositions.h \
 	layout.h \
 	lengthcommon.C \
 	lengthcommon.h \
Index: src/lyxrc.C
===
--- src/lyxrc.C	(revision 13470)
+++ src/lyxrc.C	(working copy)
@@ -26,6 +26,7 @@
 #include "format.h"
 #include "gettext.h"
 #include "lastfiles.h"
+#include "filepositions.h"
 #include "LColor.h"
 #include "lyxlex.h"
 #include "lyxfont.h"
@@ -105,6 +106,8 @@
 	{ "\\language_package", LyXRC::RC_LANGUAGE_PACKAGE },
 	{ "\\language_use_babel", LyXRC::RC_LANGUAGE_USE_BABEL },
 	{ "\\lastfiles", LyXRC::RC_LASTFILES },
+	{ "\\use_filepos", LyXRC::RC_USEFILEPOS },
+	{ "\\filepositions", LyXRC::RC_FILEPOSITIONS },
 	{ "\\make_backup", LyXRC::RC_MAKE_BACKUP },
 	{ "\\mark_foreign_language", LyXRC::RC_MARK_FOREIGN_LANGUAGE },
 	{ "\\num_lastfiles", LyXRC::RC_NUMLASTFILES },
@@ -246,6 +249,7 @@
 	ascii_linelen = 65;
 	num_lastfiles = maxlastfiles;
 	check_lastfiles = true;
+	use_filepos = true;
 	

Re: [PATCH} Was: Feature request: Remember the editing location when a file is closed.

[Lars Gullik Bjønnes]

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

| > No. We need to have the "last position" feature in addition, not mixed
| > with lastfile.
| 
| OK, since you insist, here comes a much larger patch.

I like this patch a lot better. Thanks for doing this.

One thing we should think about though; not caused by this patch, but
releated to it:

We have 'lastfiles' and 'lastpos' as configurable filenames, I am not
sure that they should be. IMHO we should just hardcode them. Opinions
on that?
 
| The patch works on my system. However, there are two problems that
| make this feature almost useless. Please help me resolve them.
 
| 1. Alt-F4 and close-button do not trigger LDUNC_QUIT or bufferClose()
| so position will not be saved. Note that Alt-F4 is marked as a
| shortcut for File->Exit but it behaves differently than File->Exit.

This not triggering, is it the same on all frontends?
 
| 2. If a filename is passed to lyx as 'lyx file.lyx', the cursor will
| be moved by my patch, but then moved back by some unknown mechanism
| (seems to be some GUI stuff). This is a big drawback since this is the
| main way I start lyx.

Ok, we need to investigate a bit.
Could it be that a ->setCursor is also called later, so that your
setting is overwritten?

| Index: src/BufferView_pimpl.C
| ===
| --- src/BufferView_pimpl.C(revision 13470)
| +++ src/BufferView_pimpl.C(working copy)
| @@ -293,6 +294,23 @@
|  
|   setBuffer(b);
|   bv_->showErrorList(_("Parse"));
| +
| + // scroll to the position when the file was last closed
| + if (lyxrc.use_filepos) {
| + lyx::pit_type pit = 
LyX::ref().filepositions().loadFilePosition(s);
| + // move to the beginning of that paragraph
| + // be careful since the file may have been externally changed 
...
| + if ( static_cast(pit) < b->paragraphs().size() ) {
| + cursor_.clearSelection();
| + ParIterator it = b->par_iterator_begin();
| + ParIterator const end = b->par_iterator_end();
| + for (; it != end; ++it)
| + if (it.pit() == pit)
| + break;
| + if (it != end)  
| + bv_->setCursor(makeDocIterator(it, 0));

I am not sure that the look is needed (even for safety).
I think that a "it = b->paragraphs()[pit]" would be enough.
(might have to be written as something else, but eh above would be the
meaning.)

Also, is the clearSelection ever needed? This is upon file load. Can a
selection ever be set?

| @@ -767,7 +785,12 @@
|   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 paragraph pit to filepositions
| + if (i == 0)
| + LyX::ref().filepositions().saveFilePosition(buffer_->fileName(),
| + cursor_.pit() );
| + else
|   owner_->message(bformat(_("Saved bookmark %1$d"), i));
|  }

As said I don't like this mix of features, but as it is strictly an
internal thing, let's handle that one later.

(I am dreaming of a .lyx/bookmarks file as well.)

| Index: src/filepositions.C
| ===
| --- src/filepositions.C   (revision 0)
| +++ src/filepositions.C   (revision 0)
| @@ -0,0 +1,86 @@
| +/**
| + * \file filespositions.C
| + * This file is part of LyX, the document processor.
| + * Licence details can be found in the file COPYING.
| + *
| + * \author Bo Peng
| + *
| + * Full author contact details are available in file CREDITS.
| + */
| +
| +#include 
| +
| +#include "filepositions.h"
| +#include "debug.h"
| +
| +#include 
| +
| +#include 
| +#include 
| +
| +using std::getline;
| +using std::string;
| +using std::ifstream;
| +using std::ofstream;
| +using std::endl;
| +using std::istringstream;
| +
| +FilePositions::FilePositions(string const & filename)
| + : num_positions(100)
| +{
| + readFile(filename);
| +}
| +
| +void FilePositions::readFile(string const & filename)
| +{
| + // we will not complain if we can't find filename nor will
| + // we issue a warning. (Lgb)
| + ifstream ifs(filename.c_str());
| + string tmp;
| +
| + while (getline(ifs, tmp) && file_positions.size() < num_positions ) {
| + // pos, filename\n
| + FilePos pos;
| + string fname;
| + istringstream itmp(tmp);
| + itmp >> pos;
| + itmp.ignore(2);  // ingore ","

Typo in comment. Also are you not ignoring ", "?

| +void FilePositions::saveFilePosition(string const& fname, FilePos
| pos )

space before '&' please

| +{
| + file_positions[fname] 

Feature request: Remember the editing location when a file is closed.

[Bo Peng]

Dear list,

I am not sure if others will like this idea. When I edit a long file,
it may be troublesome to locate and continue from where I was working
on, after the file is re-opened. I would be happy if lyx can
automatically goes to the page at which lyx was closed.

I can think of two ways to do this:

1. (More intrusive). Remember the location under .lyx and jump to it
when a lyx file is opened. I think this is what vim is doing.

2. (Less intrusive). Save bookmark with the lyx file. A user can save
bookmark, close the file, and go to the bookmark after the file is
re-opened.

Cheers,
Bo

Re: Feature request: Remember the editing location when a file is closed.

[christian . ridderstrom]

On Wed, 22 Mar 2006, Bo Peng wrote:

 Dear list,
 
 I am not sure if others will like this idea. When I edit a long file,
 it may be troublesome to locate and continue from where I was working
 on, after the file is re-opened. I would be happy if lyx can
 automatically goes to the page at which lyx was closed.
 
 I can think of two ways to do this:
 
 1. (More intrusive). Remember the location under .lyx and jump to it
 when a lyx file is opened. I think this is what vim is doing.

For good or worse, this also means that when working with someone else on 
the same file (that's sent back and forth, or under version control), 
you'll open it up where the other person left off...

 2. (Less intrusive). Save bookmark with the lyx file. A user can save
 bookmark, close the file, and go to the bookmark after the file is
 re-opened.

Maybe file a bugzilla report? Or if you're brave, discss which of the 
options would be best on the user's list :-)

/C

-- 
Christian Ridderström, +46-8-768 39 44   http://www.md.kth.se/~chr

[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

Feature request: Remember the editing location when a file is closed.

[Bo Peng]

Dear list,

I am not sure if others will like this idea. When I edit a long file,
it may be troublesome to locate and continue from where I was working
on, after the file is re-opened. I would be happy if lyx can
automatically goes to the page at which lyx was closed.

I can think of two ways to do this:

1. (More intrusive). Remember the location under .lyx and jump to it
when a lyx file is opened. I think this is what vim is doing.

2. (Less intrusive). Save bookmark with the lyx file. A user can save
bookmark, close the file, and go to the bookmark after the file is
re-opened.

Cheers,
Bo

Re: Feature request: Remember the editing location when a file is closed.

[christian . ridderstrom]

On Wed, 22 Mar 2006, Bo Peng wrote:

> Dear list,
> 
> I am not sure if others will like this idea. When I edit a long file,
> it may be troublesome to locate and continue from where I was working
> on, after the file is re-opened. I would be happy if lyx can
> automatically goes to the page at which lyx was closed.
> 
> I can think of two ways to do this:
> 
> 1. (More intrusive). Remember the location under .lyx and jump to it
> when a lyx file is opened. I think this is what vim is doing.

For good or worse, this also means that when working with someone else on 
the same file (that's sent back and forth, or under version control), 
you'll open it up where the other person left off...

> 2. (Less intrusive). Save bookmark with the lyx file. A user can save
> bookmark, close the file, and go to the bookmark after the file is
> re-opened.

Maybe file a bugzilla report? Or if you're brave, discss which of the 
options would be best on the user's list :-)

/C

-- 
Christian Ridderström, +46-8-768 39 44   http://www.md.kth.se/~chr

[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