Re: Undo.cpp reports an error on trunk
Le 18/07/2012 16:35, Richard Heck a écrit : On 07/18/2012 05:46 AM, Jean-Marc Lasgouttes wrote: Le 13/07/2012 19:21, Richard Heck a écrit : This is why I propose to add a parameter to reload telling whether undo should be discarded. I just committed that change. This is an obvious candidate for branch (no risk, IMO). OK. I did that at 824fe17511. JMarc
Re: Undo.cpp reports an error on trunk
On 07/18/2012 03:08 PM, Jean-Marc Lasgouttes wrote: Le 18/07/12 17:43, Richard Heck a écrit : Looks like you never backported this particular fix. If you decide to do it, I'll backport my patch too. Which thing didn't I backport? This one, I think. I guess that should go in, so I've done it. Richard
Re: Undo.cpp reports an error on trunk
Le 18/07/12 17:43, Richard Heck a écrit : Looks like you never backported this particular fix. If you decide to do it, I'll backport my patch too. Which thing didn't I backport? This one, I think. JMarc commit 9283cbdbcb3b33da28d2ecca6cd5665f39c7b388 Author: Richard Heck Date: Fri Jan 13 03:31:01 2012 + Fix crash reported on list when renaming a child buffer and then trying to compile. There are really two problems here. One is that the renamed buffer needs to be reloaded. All kinds of files, etc, may not exist any more, if we've been saved to a new directory; our children e.g. may not be in the right place. And, in this case, we may no longer be a child of our old parent. Reloading will fix all of that. On reload, though, we need to clear our parent, since we may not have one any more. It will get reset if need be.
Re: Undo.cpp reports an error on trunk
On 07/18/2012 10:48 AM, Jean-Marc Lasgouttes wrote: Le 18/07/2012 16:35, Richard Heck a écrit : On 07/18/2012 05:46 AM, Jean-Marc Lasgouttes wrote: Le 13/07/2012 19:21, Richard Heck a écrit : This is why I propose to add a parameter to reload telling whether undo should be discarded. I just committed that change. This is an obvious candidate for branch (no risk, IMO). OK. Looks like you never backported this particular fix. If you decide to do it, I'll backport my patch too. Which thing didn't I backport? rh
Re: Undo.cpp reports an error on trunk
Le 18/07/2012 16:35, Richard Heck a écrit : On 07/18/2012 05:46 AM, Jean-Marc Lasgouttes wrote: Le 13/07/2012 19:21, Richard Heck a écrit : This is why I propose to add a parameter to reload telling whether undo should be discarded. I just committed that change. This is an obvious candidate for branch (no risk, IMO). OK. Looks like you never backported this particular fix. If you decide to do it, I'll backport my patch too. JMarc
Re: Undo.cpp reports an error on trunk
On 07/18/2012 05:46 AM, Jean-Marc Lasgouttes wrote: Le 13/07/2012 19:21, Richard Heck a écrit : This is why I propose to add a parameter to reload telling whether undo should be discarded. I just committed that change. This is an obvious candidate for branch (no risk, IMO). OK. rh
Re: Undo.cpp reports an error on trunk
On 07/18/2012 06:08 AM, Jean-Marc Lasgouttes wrote: Le 13/07/2012 19:21, Richard Heck a écrit : OTOH, it looks like doing a reload in after save-as is a bit harsh. Is the goal only to sanitize IncludeInset? It certainly wouldn't surprise me if there were something simpler we could do. It was definitely a big hammer, that commit. The primary goal was to deal with the fact that children, parents, etc, may no longer be where there are supposed to be. But the Buffer contains all kinds of pointers (e.g., in the TOC, in the reference cache, etc) that may point to children and the like. So I'm guessing there are other issues. Does it mean that LyX just crashes if I remove a file that is supposed to be here? How rude... I can't remember exactly what the crash was. But suppose you save the file to a new location, then you close what used to be a child buffer. The original file thinks that Buffer is still a child, and it didn't get notified of anything because it's not the parent any longer. Etc. Richard
Re: Undo.cpp reports an error on trunk
Le 13/07/2012 19:21, Richard Heck a écrit : OTOH, it looks like doing a reload in after save-as is a bit harsh. Is the goal only to sanitize IncludeInset? It certainly wouldn't surprise me if there were something simpler we could do. It was definitely a big hammer, that commit. The primary goal was to deal with the fact that children, parents, etc, may no longer be where there are supposed to be. But the Buffer contains all kinds of pointers (e.g., in the TOC, in the reference cache, etc) that may point to children and the like. So I'm guessing there are other issues. Does it mean that LyX just crashes if I remove a file that is supposed to be here? How rude... JMarc
Re: Undo.cpp reports an error on trunk
Le 13/07/2012 19:21, Richard Heck a écrit : This is why I propose to add a parameter to reload telling whether undo should be discarded. I just committed that change. This is an obvious candidate for branch (no risk, IMO). JMarc
Re: Undo.cpp reports an error on trunk
Jean-Marc Lasgouttes wrote: >> You mean to preserve undo stack for each reload of file or just for save-as >> scenario? We also reload automatically after commit/checkout VCS operation >> which can change read-only status. > > This is why I propose to add a parameter to reload telling whether undo > should be discarded. That looks fine. > OTOH, it looks like doing a reload in after save-as is a bit harsh. Is the > goal only to sanitize IncludeInset? Paths to images also change, though don't know whether reload is needed. Pavel
Re: Undo.cpp reports an error on trunk
On 07/13/2012 03:47 AM, Jean-Marc Lasgouttes wrote: Le 13/07/2012 00:24, Pavel Sanda a écrit : Richard Heck wrote: This leads to the harmless message above, but more importantly, it means that the undo stack is lost every time a file is saved-as. This looks like a regression to me. Richard, would it seem right to add a parameter to Buffer::reload asking to preserve the undo stack in this case? You mean to preserve undo stack for each reload of file or just for save-as scenario? We also reload automatically after commit/checkout VCS operation which can change read-only status. This is why I propose to add a parameter to reload telling whether undo should be discarded. OTOH, it looks like doing a reload in after save-as is a bit harsh. Is the goal only to sanitize IncludeInset? It certainly wouldn't surprise me if there were something simpler we could do. It was definitely a big hammer, that commit. The primary goal was to deal with the fact that children, parents, etc, may no longer be where there are supposed to be. But the Buffer contains all kinds of pointers (e.g., in the TOC, in the reference cache, etc) that may point to children and the like. So I'm guessing there are other issues. I have been pondering for some time whether the updateBuffer mechanism could be extended with an enum telling what to update, in order to be able to run it for specific tasks. For example updateBuffer(EXTERNAL_FILES) could check the validity of all external files we hold. I had other applications in mind, but I can't remember them ATM. That's quite a good idea, I think. I'm sure we'd discover uses for this. I can well remember putting comments in the code that said things like, "What we actually want here is such-and-such, and the way to get it is to update the Buffer" Richard
Re: Undo.cpp reports an error on trunk
Le 12/07/2012 18:14, Richard Heck a écrit : Richard, would it seem right to add a parameter to Buffer::reload asking to preserve the undo stack in this case? You would probably know better than I do. Are there pointers on the undo stack that will be invalidated once we reload? The undo stack holds a pointer to the buffer (which is OK) and the undo elements hold StableIterators, which do not contain any meaningful pointers. So this should be safe enough IMO. I will experiment. JMarc
Re: Undo.cpp reports an error on trunk
Le 13/07/2012 00:24, Pavel Sanda a écrit : Richard Heck wrote: This leads to the harmless message above, but more importantly, it means that the undo stack is lost every time a file is saved-as. This looks like a regression to me. Richard, would it seem right to add a parameter to Buffer::reload asking to preserve the undo stack in this case? You mean to preserve undo stack for each reload of file or just for save-as scenario? We also reload automatically after commit/checkout VCS operation which can change read-only status. This is why I propose to add a parameter to reload telling whether undo should be discarded. OTOH, it looks like doing a reload in after save-as is a bit harsh. Is the goal only to sanitize IncludeInset? I have been pondering for some time whether the updateBuffer mechanism could be extended with an enum telling what to update, in order to be able to run it for specific tasks. For example updateBuffer(EXTERNAL_FILES) could check the validity of all external files we hold. I had other applications in mind, but I can't remember them ATM. JMarc
Re: Undo.cpp reports an error on trunk
Richard Heck wrote: >> This leads to the harmless message above, but more importantly, it means >> that the undo stack is lost every time a file is saved-as. This looks like >> a regression to me. >> >> Richard, would it seem right to add a parameter to Buffer::reload asking >> to preserve the undo stack in this case? You mean to preserve undo stack for each reload of file or just for save-as scenario? We also reload automatically after commit/checkout VCS operation which can change read-only status. Pavel
Re: Undo.cpp reports an error on trunk
On 07/12/2012 07:31 AM, Jean-Marc Lasgouttes wrote: Le 11/07/2012 23:05, Scott Kostyshak a écrit : git bisect leads me here: bbbc2b654175edbccb1ce54ae6dbb15f8a042360 which is related to this ticket: http://www.lyx.org/trac/ticket/8159 I am not surprised that this change leads to this kind of problems (which are harmless AFAICS). What happens is that, when a new file is saved, it has to be renamed and, since commit 9283cbdb from Richard, this implies a Buffer::reload, which in turn clears the undo stack. This leads to the harmless message above, but more importantly, it means that the undo stack is lost every time a file is saved-as. This looks like a regression to me. Richard, would it seem right to add a parameter to Buffer::reload asking to preserve the undo stack in this case? You would probably know better than I do. Are there pointers on the undo stack that will be invalidated once we reload? I'd have done something less drastic than reload the file there if I could have thought of anything. But there are so many parameters that depend upon the file's location that it didn't seem anything less would do. Richard
Re: Undo.cpp reports an error on trunk
Le 11/07/2012 23:05, Scott Kostyshak a écrit : git bisect leads me here: bbbc2b654175edbccb1ce54ae6dbb15f8a042360 which is related to this ticket: http://www.lyx.org/trac/ticket/8159 I am not surprised that this change leads to this kind of problems (which are harmless AFAICS). What happens is that, when a new file is saved, it has to be renamed and, since commit 9283cbdb from Richard, this implies a Buffer::reload, which in turn clears the undo stack. This leads to the harmless message above, but more importantly, it means that the undo stack is lost every time a file is saved-as. This looks like a regression to me. Richard, would it seem right to add a parameter to Buffer::reload asking to preserve the undo stack in this case? JMarc
Undo.cpp reports an error on trunk
I can reproduce as follows: 1. Start a fresh LyX session. 2. Open a new document. 3. Save (or Save As). I get the following messages: Saving document ~/newfile1.lyx... Saving document ~/newfile1.lyx... done. Document ~/newfile1.lyx reloaded. (buffer-write: Ctrl+S, F2)Undo.cpp (515): There is no undo group to end here On a different note, shouldn't there be a line break in the last line after (buffer-write: Ctrl+S, F2)? git bisect leads me here: bbbc2b654175edbccb1ce54ae6dbb15f8a042360 which is related to this ticket: http://www.lyx.org/trac/ticket/8159 Scott