From: SourceForge.net <no...@so...> - 2007-03-01 23:23:49
|
Patches item #1672086, was opened at 2007-03-01 18:23 Message generated for change (Tracker Item Submitted) made by Item Submitter You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300588&aid=1672086&group_id=588 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: texteditor Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Joseph Erickson (firstclown) Assigned to: Nobody/Anonymous (nobody) Summary: Cleanup for Clear Undo on Save Initial Comment: Moved the code to do the actual clear to UndoManager.bufferSaved() per Kazutoshi Satoda's suggestion. Also refactored the bufferSaved method to differentiate between reseting the start and end on a dirty and actions to be taken when a buffer is finished saving. Also removed now unneeded getUndoManager method. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300588&aid=1672086&group_id=588 |
From: SourceForge.net <no...@so...> - 2007-03-02 05:25:14
|
Patches item #1672086, was opened at 2007-03-01 15:23 Message generated for change (Comment added) made by vanza You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300588&aid=1672086&group_id=588 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: texteditor Group: None >Status: Closed >Resolution: Accepted Priority: 5 Private: No Submitted By: Joseph Erickson (firstclown) Assigned to: Nobody/Anonymous (nobody) Summary: Cleanup for Clear Undo on Save Initial Comment: Moved the code to do the actual clear to UndoManager.bufferSaved() per Kazutoshi Satoda's suggestion. Also refactored the bufferSaved method to differentiate between reseting the start and end on a dirty and actions to be taken when a buffer is finished saving. Also removed now unneeded getUndoManager method. ---------------------------------------------------------------------- >Comment By: Marcelo Vanzin (vanza) Date: 2007-03-01 21:25 Message: Logged In: YES user_id=75113 Originator: NO Applied, rev #9059. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300588&aid=1672086&group_id=588 |
From: SourceForge.net <no...@so...> - 2008-06-19 22:17:48
|
Patches item #1672086, was opened at 2007-03-02 00:23 Message generated for change (Comment added) made by kpouer You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300588&aid=1672086&group_id=588 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: texteditor Group: None Status: Closed Resolution: Accepted Priority: 5 Private: No Submitted By: Joseph Erickson (firstclown) Assigned to: Nobody/Anonymous (nobody) Summary: Cleanup for Clear Undo on Save Initial Comment: Moved the code to do the actual clear to UndoManager.bufferSaved() per Kazutoshi Satoda's suggestion. Also refactored the bufferSaved method to differentiate between reseting the start and end on a dirty and actions to be taken when a buffer is finished saving. Also removed now unneeded getUndoManager method. ---------------------------------------------------------------------- >Comment By: Matthieu Casanova (kpouer) Date: 2008-06-20 00:17 Message: Logged In: YES user_id=285591 Originator: NO Hi, I am late but what was the reason of this patch ? It introduce a regression since the UndoManager must not depend on jEdit class. Can it be rollbacked ? ---------------------------------------------------------------------- Comment By: Marcelo Vanzin (vanza) Date: 2007-03-02 06:25 Message: Logged In: YES user_id=75113 Originator: NO Applied, rev #9059. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300588&aid=1672086&group_id=588 |
From: Kazutoshi S. <k_s...@f2...> - 2008-06-19 23:31:37
|
Hi Matthieu, >>Comment By: Matthieu Casanova (kpouer) (snip) > Hi, I am late but what was the reason of this patch ? Sorry, there was a discussion in some private mails sent only to Joseph and Marcelo. I attached the summary of it at the bottom of this mail. I wasn't sure that it should be on the list at that time. But I'm now absolutely sure it should be. > It introduce a regression since the UndoManager must not depend on jEdit > class. > Can it be rollbacked ? I see the problem. But I don't want rollbacking it for reasons said in the mailing. I'll make a patch to solve it, do some test, and commit. Could you please wait for a while? It will one or a few days. The mailing started at 2007/03/01: Kazutoshi Satoda wrote: > va...@us... wrote: >> Revision: 9037 >> http://svn.sourceforge.net/jedit/?rev=9037&view=rev >> Author: vanza >> Date: 2007-02-27 21:18:25 -0800 (Tue, 27 Feb 2007) >> >> Log Message: >> ----------- >> SF Patch #1666690: No Undo past file save option > (snip) >> Modified: jEdit/trunk/org/gjt/sp/jedit/bufferio/BufferSaveRequest.java >> =================================================================== >> --- jEdit/trunk/org/gjt/sp/jedit/bufferio/BufferSaveRequest.java 2007-02-27 14:22:46 UTC (rev 9036) >> +++ jEdit/trunk/org/gjt/sp/jedit/bufferio/BufferSaveRequest.java 2007-02-28 05:18:25 UTC (rev 9037) > (snip) >> @@ -137,6 +137,9 @@ >> finally >> { >> buffer.readUnlock(); >> + if(jEdit.getBooleanProperty("resetUndoOnSave")){ >> + buffer.getUndoManager().clear(); >> + } >> } >> } >> finally > (snip) > > This code invokes buffer.getUndoManager().clear() even if the save request > failed (with something like a low level I/O error, an encoding error, ...). > User will get nothing but loses undo. Is this intended? > > Should I use devel-list or commits-list for questions like this? Joseph Erickson wrote: > Completely unintended. It makes sense now that you say it though. > > Where in the code is a successful save certain? Right after > write(buffer,out); in the try block? Is it later on when the markers > file is cleaned up? When vfs.saveComplete is called? There's a lot of > try-catch-finallys in there and it's hard for me to tell where things > are happening. > > Let me know and I'll submit another patch. Kazutoshi Satoda wrote: > I propose to place it in Buffer.finishSaving(), next to > undoMgr.bufferSaved() in it. Or, placing in UndoManager.bufferSaved() > may be more correct. In latter case, you should take care of that > UndoManager.bufferSaved() is also called from Buffer.setDirty(). > In both way, new method getUndoManager() will be no longer needed. Joseph Erickson wrote: > Thank you for your help. I refactored bufferSaved() to handle this and > moved the dirty flag handling code into a new method. > > http://sourceforge.net/tracker/index.php?func=detail&aid=1672086&group_id=588&atid=300588 -- k_satoda |
From: Matthieu C. <cho...@gm...> - 2008-06-20 07:06:57
|
Hi Kazutoshi, thanks for your fast answer, I understand now, I will do nothing and wait for your patch, it is not urgent, and it is not easy to know what part of the code should depend on jEdit. Basically the buffer package must not depend on jEdit class. I wrote a doc : CORE_GUIDELINE.TXT that contains those informations, it may not be up to date but it is better than nothing. Of course anybody can put in it informations they find important to know about the core. And I'll change the in the build.xml the method how the standalone textArea is built in order to help us to know if we broke something Matthieu On Fri, Jun 20, 2008 at 1:31 AM, Kazutoshi Satoda <k_s...@f2...> wrote: > Hi Matthieu, > >>> Comment By: Matthieu Casanova (kpouer) > > (snip) >> >> Hi, I am late but what was the reason of this patch ? > > Sorry, there was a discussion in some private mails sent only to Joseph > and Marcelo. I attached the summary of it at the bottom of this mail. > I wasn't sure that it should be on the list at that time. But I'm now > absolutely sure it should be. > >> It introduce a regression since the UndoManager must not depend on jEdit >> class. >> Can it be rollbacked ? > > I see the problem. But I don't want rollbacking it for reasons said in > the mailing. I'll make a patch to solve it, do some test, and commit. > > Could you please wait for a while? It will one or a few days. > > > > The mailing started at 2007/03/01: > > Kazutoshi Satoda wrote: >> >> va...@us... wrote: >>> >>> Revision: 9037 >>> http://svn.sourceforge.net/jedit/?rev=9037&view=rev >>> Author: vanza >>> Date: 2007-02-27 21:18:25 -0800 (Tue, 27 Feb 2007) >>> >>> Log Message: >>> ----------- >>> SF Patch #1666690: No Undo past file save option >> >> (snip) >>> >>> Modified: jEdit/trunk/org/gjt/sp/jedit/bufferio/BufferSaveRequest.java >>> =================================================================== >>> --- jEdit/trunk/org/gjt/sp/jedit/bufferio/BufferSaveRequest.java >>> 2007-02-27 14:22:46 UTC (rev 9036) >>> +++ jEdit/trunk/org/gjt/sp/jedit/bufferio/BufferSaveRequest.java >>> 2007-02-28 05:18:25 UTC (rev 9037) >> >> (snip) >>> >>> @@ -137,6 +137,9 @@ >>> finally >>> { >>> buffer.readUnlock(); >>> + >>> if(jEdit.getBooleanProperty("resetUndoOnSave")){ >>> + >>> buffer.getUndoManager().clear(); >>> + } >>> } >>> } >>> finally >> >> (snip) >> >> This code invokes buffer.getUndoManager().clear() even if the save request >> failed (with something like a low level I/O error, an encoding error, >> ...). >> User will get nothing but loses undo. Is this intended? >> >> Should I use devel-list or commits-list for questions like this? > > Joseph Erickson wrote: >> >> Completely unintended. It makes sense now that you say it though. >> >> Where in the code is a successful save certain? Right after >> write(buffer,out); in the try block? Is it later on when the markers >> file is cleaned up? When vfs.saveComplete is called? There's a lot of >> try-catch-finallys in there and it's hard for me to tell where things >> are happening. >> >> Let me know and I'll submit another patch. > > Kazutoshi Satoda wrote: >> >> I propose to place it in Buffer.finishSaving(), next to >> undoMgr.bufferSaved() in it. Or, placing in UndoManager.bufferSaved() >> may be more correct. In latter case, you should take care of that >> UndoManager.bufferSaved() is also called from Buffer.setDirty(). >> In both way, new method getUndoManager() will be no longer needed. > > Joseph Erickson wrote: >> >> Thank you for your help. I refactored bufferSaved() to handle this and >> moved the dirty flag handling code into a new method. >> >> >> http://sourceforge.net/tracker/index.php?func=detail&aid=1672086&group_id=588&atid=300588 > > -- > k_satoda > |
From: SourceForge.net <no...@so...> - 2008-06-20 00:07:10
|
Patches item #1672086, was opened at 2007-03-02 08:23 Message generated for change (Comment added) made by k_satoda You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300588&aid=1672086&group_id=588 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: texteditor Group: None Status: Closed Resolution: Accepted Priority: 5 Private: No Submitted By: Joseph Erickson (firstclown) Assigned to: Nobody/Anonymous (nobody) Summary: Cleanup for Clear Undo on Save Initial Comment: Moved the code to do the actual clear to UndoManager.bufferSaved() per Kazutoshi Satoda's suggestion. Also refactored the bufferSaved method to differentiate between reseting the start and end on a dirty and actions to be taken when a buffer is finished saving. Also removed now unneeded getUndoManager method. ---------------------------------------------------------------------- >Comment By: Kazutoshi Satoda (k_satoda) Date: 2008-06-20 09:07 Message: Logged In: YES user_id=1483238 Originator: NO I see the problem, but I don't want to rollback it because it contains some fix for other actual problems. See the summary of mailing in this post. http://www.nabble.com//forum/Permalink.jtp?root=18019443&post=18020223 I made another patch to fix the dependency. I'll commit this after some tests. Comments are welcome. File Added: UndoManager_do_not_depend_on_jEdit.patch ---------------------------------------------------------------------- Comment By: Matthieu Casanova (kpouer) Date: 2008-06-20 07:17 Message: Logged In: YES user_id=285591 Originator: NO Hi, I am late but what was the reason of this patch ? It introduce a regression since the UndoManager must not depend on jEdit class. Can it be rollbacked ? ---------------------------------------------------------------------- Comment By: Marcelo Vanzin (vanza) Date: 2007-03-02 14:25 Message: Logged In: YES user_id=75113 Originator: NO Applied, rev #9059. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300588&aid=1672086&group_id=588 |
From: SourceForge.net <no...@so...> - 2008-06-22 06:07:44
|
Patches item #1672086, was opened at 2007-03-02 08:23 Message generated for change (Comment added) made by k_satoda You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300588&aid=1672086&group_id=588 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: texteditor Group: None Status: Closed Resolution: Accepted Priority: 5 Private: No Submitted By: Joseph Erickson (firstclown) Assigned to: Nobody/Anonymous (nobody) Summary: Cleanup for Clear Undo on Save Initial Comment: Moved the code to do the actual clear to UndoManager.bufferSaved() per Kazutoshi Satoda's suggestion. Also refactored the bufferSaved method to differentiate between reseting the start and end on a dirty and actions to be taken when a buffer is finished saving. Also removed now unneeded getUndoManager method. ---------------------------------------------------------------------- >Comment By: Kazutoshi Satoda (k_satoda) Date: 2008-06-21 21:21 Message: Logged In: YES user_id=1483238 Originator: NO Committed UndoManager_do_not_depend_on_jEdit.patch in r12867. ---------------------------------------------------------------------- Comment By: Kazutoshi Satoda (k_satoda) Date: 2008-06-20 09:07 Message: Logged In: YES user_id=1483238 Originator: NO I see the problem, but I don't want to rollback it because it contains some fix for other actual problems. See the summary of mailing in this post. http://www.nabble.com//forum/Permalink.jtp?root=18019443&post=18020223 I made another patch to fix the dependency. I'll commit this after some tests. Comments are welcome. File Added: UndoManager_do_not_depend_on_jEdit.patch ---------------------------------------------------------------------- Comment By: Matthieu Casanova (kpouer) Date: 2008-06-20 07:17 Message: Logged In: YES user_id=285591 Originator: NO Hi, I am late but what was the reason of this patch ? It introduce a regression since the UndoManager must not depend on jEdit class. Can it be rollbacked ? ---------------------------------------------------------------------- Comment By: Marcelo Vanzin (vanza) Date: 2007-03-02 14:25 Message: Logged In: YES user_id=75113 Originator: NO Applied, rev #9059. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300588&aid=1672086&group_id=588 |