Author Topic: Autosave enhancement  (Read 30579 times)

Offline Bat

  • Multiple posting newcomer
  • *
  • Posts: 48
Autosave enhancement
« on: November 16, 2013, 12:10:18 am »
I've just send a patch :
https://developer.berlios.de/patch/index.php?func=detailpatch&patch_id=3524&group_id=5358
 goal : add integrated restore on autosave feature

There some slight modifications to introduce necessary entry point
Note : lots of modification are only visible for ".save" version of autosave

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Autosave enhancement
« Reply #1 on: November 16, 2013, 12:26:25 am »
Can you explain what is the purpose of this patch?
What problem are you trying to solve with it?
For me it seems that you're trying to replace a VCS system with it.
Is this the case?

Looking at the patch it seems you're using _("...") for things that are not translatable, which is a mistake!
For example all the file types should be surrounded by _T or wxT and not by _().

Another problem I see is that you've couple the SDK for cbEditor and the AutoSave plugin. I'm not sure this is a good idea.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline Bat

  • Multiple posting newcomer
  • *
  • Posts: 48
Re: Autosave enhancement
« Reply #2 on: November 16, 2013, 11:48:49 am »
The prupose of this match, is mainly to integrate recover of autosaved file, in case of C::B crash
Ok, crash never appends, but just in case  ;)
It's not same purpose of CVS, as it recover only "unsaved" file. As soon as a file is saved on disk, autosave is deleted. And there CVS take relay

At workspace/project/editor open, if an autosaved file is younger than original file, there were probably some problem (crash, no save of file other than voluntary say "no" to save file, ...)

For _("..."), I will correct this (I ignore / and haven't search / this difference)

Quote
Another problem I see is that you've couple the SDK for cbEditor and the AutoSave plugin. I'm not sure this is a good idea.

Humm, really not wanted. I've added some particular entry point to have a better autosave (without mess in "modified" flag), but I thought Autosave is not needed for cbEditor to work.
To be decoupled, I must have possibility to start C::B with autosave plugin not present ? Is it the good test ?

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Autosave enhancement
« Reply #3 on: November 16, 2013, 12:18:11 pm »
The prupose of this match, is mainly to integrate recover of autosaved file, in case of C::B crash
I still don't understand. What you want is C::B to automatically copy the file from .save to the original on the next start?
This patch adds tons of complexity to the code, so you'll have to justify it, before it goes in SVN.

And yes, C::B must be able to work without the autosave plugin as before.
« Last Edit: November 16, 2013, 01:31:19 pm by oBFusCATed »
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: Autosave enhancement
« Reply #4 on: November 16, 2013, 01:01:37 pm »
I'm not quite happy with the patch for a couple of reasons.

The autosave plugin is already something that's not-really-necessary, and it was only ever intended as a workaround to the actual problem, years ago. The whole idea behind it was that it is slighly less annoying when Code::Blocks crashes and the last 2 hours of your work aren't gone -- you only lose maybe the last 2-3 mins of your work.
The main objective is, however, that Code::Blocks does not crash in the first place, and we're reasonably close to that objective (I've not seen a crash on my machine for years).

Having to restore from a save file is something that should rarely happen if ever (I remember having to do it a single time, years ago). It is not something most people will regularly find useful being automated.

More importantly, however...

The code alters the SDK to make the plugin work with its new functionality, and the "complete save" feature uses a boolean as a lock. Of course a boolean is sufficient since autosave only runs every few minutes, so there will never be any concurrency, right? Except when there is concurrency because the user opens or saves a file or saves the current project... which of course happens in regular intervals.
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline Bat

  • Multiple posting newcomer
  • *
  • Posts: 48
Re: Autosave enhancement
« Reply #5 on: November 16, 2013, 02:46:24 pm »
Quote
I still don't understand. What you want is C::B to automatically copy the file from .save to the original on the next start?

Yes but not "silently". When something (workspace/project/editor) is opened, if a .save file exists, it warn user that an autosaved file exist and ask if it should be used or not. This type of feature is present on some other editors, code or generalist (I agree mainly Microsoft ones ...).

Quote
This patch adds tons of complexity to the code, so you'll have to justify it, before it goes in SVN.

Yes, I've opened this topic to share about it because I know it add lot of things/breaks in sdk
Perharps at end of discussion, it's concluded it's not good idea ...

Quote
The main objective is, however, that Code::Blocks does not crash in the first place, and we're reasonably close to that objective (I've not seen a crash on my machine for years).

I'm totally agree with objective. But in my usage for 2 years, I found some random crashes or "hangs" that lead to some losts. I'm convinced that some (and even all) are plugins related. If there is no problem, it don't change at all usage of C::B

Quote
Having to restore from a save file is something that should rarely happen if ever (I remember having to do it a single time, years ago). It is not something most people will regularly find useful being automated.

The goal is clearly that this plugin never have work to do. It's like all bug reporting feature.
If poeple know which file should be restored it's ok.
Files poeple are mainly working on are obvious
A minor modification (comment one line) done one hour ago can easily not recover.
A change in project compilation flag can also be forgoten, as one hour after I don't remember that I have ad this DEFINE that enable a small minor change ...
For me it's the most important part, warning that an autosaved file exist for what we open.

Precision : I'm (also) an embedded developper (quite small chips) and it's a part of my usage. There are more projects options (related to compiler option) tweeking in this usage

So patch permit also to load this autosaved one "in place" of original, aka : get things back just before previous crash. File on disk is always original. In memory we have modified one. And at this point user can do what he would do (save, throw away). I've done it because I found this useful, be it can be a bad idea

Quote
The code alters the SDK to make the plugin work with its new functionality, and the "complete save" feature uses a boolean as a lock.

Which one do you refer ? As far as I know, I add boolean for "workspace is load", I do that because there is only one workspace loaded at a time ?
I add a boolean in each editor instance and each project instance to says "this file is a recovered copy", because I haven't find any solution for saying "this file is the same as loaded from disk - from .save one, but it really a modified file - so if we close it we should ask if it must be save - to original one".
But I'm new to this part of C::B code so I can have missed lot of things !

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Autosave enhancement
« Reply #6 on: November 16, 2013, 03:13:24 pm »
Maybe I'm baised, but I don't see a need for this feature.
I'm using autosave in save-to-original file mode and I think that the mode where the file is saved with another name is useless and should be removed.
The reason is that with my workflow - I don't want to restore the version from 5 minutes ago, I want the last version just before the crash.
CB saves your source code before compilation, so the original file might have new content relative to the .save file.
I don't want to compare the two versions and then choose which I want to keep. I always know that my latest work is on the disk in the original file.

If I want older revision I use my VCS to go back to it. With the modern DVCS-es you're free to commit whenever you like without possibility for breaking something for other people.

(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline Bat

  • Multiple posting newcomer
  • *
  • Posts: 48
Re: Autosave enhancement
« Reply #7 on: November 16, 2013, 07:17:19 pm »
This patch don't modify "save to original" mode. It just modify save to .save and save to .save (logrotate)

New version of patch :
https://developer.berlios.de/patch/index.php?func=detailpatch&patch_id=3524&group_id=5358

-correction of _(" => _T(" where needed
-correction of one test missing to impact only ".save" and ".save" (logrotate)

For coupling between Autosave and cbEditor, I've searched, but I should miss something. If I delete autosave.dll C::B start correctly (no error), without autosave feature (seem obvious). If I disable autosave plugin, same behaviour. Could you point me an example of the coupling ?
[Note : if I disable and enable in same session of C::B neither plugin, there is an error as "xxx.zip" is already in VFS]



Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Autosave enhancement
« Reply #8 on: November 16, 2013, 07:41:14 pm »
Could you point me an example of the coupling ?
You're adding code and APIs in cbEditor which are only usable by this feature of the autosave plugin.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline Folco

  • Regular
  • ***
  • Posts: 343
    • Folco's blog (68k lover)
Re: Autosave enhancement
« Reply #9 on: November 16, 2013, 09:45:09 pm »
As far as I remember, AutoCAD works like that : in case of crash, it shows you a dialog to recover files that were opened and not saved when the last crash occured.
Kernel Extremist - PedroM power ©

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Autosave enhancement
« Reply #10 on: November 16, 2013, 10:01:04 pm »
Falco:
You can't compare AutoCAD or any similar user program to an IDE.
 In a IDE the files are saved before every compilation and this is one of the most common operations users execute in an IDE.
I think most of the users of C::B are working with saved files (sources) most of the time.
Also, most of C::B's users probably use some kind of VCS, so they have some sort of a backup already.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: Autosave enhancement
« Reply #11 on: November 17, 2013, 12:47:08 am »
I'm using autosave in save-to-original file mode and I think that the mode where the file is saved with another name is useless and should be removed.
The reason is that with my workflow - I don't want to restore the version from 5 minutes ago, I want the last version just before the crash.
That's kind of true, and not true. I had added several modes because different people have different working patterns.

Sometimes you are just editing things and do not intend to save them to disk. Or, you only want to have consistent snapshots (no half-edited stuff) saved. Except of course when there's a crash, then you can open the half-edited file, complete the edit, and save it proper.

Using the "save-to-original-name" overwrites the original file without asking (actually it does a write-to-temp-swap, but that's irrespective). You may have to restore the file from version control only because Code::Blocks overwrote it when you didn't want that to happen. Maybe acceptable to you, but other people will think differently.
On the other hand, using the save-to-savefile mode, in the very rare event of a crash, you have two file versions, the one you explicitly saved (that is, one that you decided was "authorative") and one that is half-edited. Plus, you have one extra copy, which doesn't hurt. Harddisks fail rarely, but they do. In case your harddisk has an unrecoverable sector just at that very moment (a coincidence like a lotto win, but still), you still have at least another copy that's a few minutes old.
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: Autosave enhancement
« Reply #12 on: November 17, 2013, 01:00:48 am »
But in my usage for 2 years, I found some random crashes or "hangs" that lead to some losts. I'm convinced that some (and even all) are plugins related.
I'm convinced of that too, with a minimum set of plugins that I'm using (mostly because I don't need them) I never see any crashes. Code completion used to be the source of all crashes once upon a time, but this is fixed for a long time, I'm using CC for ages now and never see a problem.

Quote
If poeple know which file should be restored it's ok.
Files poeple are mainly working on are obvious
A minor modification (comment one line) done one hour ago can easily not recover.
I think you have to open them side by side manually anyway, since it's not a trivial thing (nor a thing that I'd like a computer to do). But yes, I can somehow see what you're saying.

Quote
Which one do you refer ? As far as I know, I add boolean for "workspace is load", I do that because there is only one workspace loaded at a time ?
I only scanned the code quickly, but what I saw was a bool variable called "lock" which was set and verified at various places, presumably protecting the process of saving/loading project files and such.
That's generally OK, but it doesn't look like it would work. The user can open or close or save projects and workspaces or files at any time, asynchronously to the autosave plugin's timer. Which means that the user could close a project in the workspace while you're loading or saving it, etc.
If something needs to be locked, it has to be done properly, not by setting a bool value. Otherwise you create a race condition.

That said, I personally don't deem project saving something that is really urgent for autosave at all. Changes are not that frequent and not that large.
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline Alpha

  • Developer
  • Lives here!
  • *****
  • Posts: 1513
Re: Autosave enhancement
« Reply #13 on: November 17, 2013, 02:16:01 am »
If it is pure crash recovery that you are looking for, an option would be to have the plugin save/update a config file (listing all files that were auto-saved) in the AppData folder each time it auto-saves files.  When the Code::Blocks exits, have the plugin delete this config file.
Now, when the plugin is loaded (on startup), if the config file exists, Code::Blocks probably crashed, so you can prompt the user with options to use auto-saved versions of files (listed in the config file).

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
Re: Autosave enhancement
« Reply #14 on: November 17, 2013, 08:13:09 am »
If it is pure crash recovery that you are looking for, an option would be to have the plugin save/update a config file (listing all files that were auto-saved) in the AppData folder each time it auto-saves files.  When the Code::Blocks exits, have the plugin delete this config file.
Now, when the plugin is loaded (on startup), if the config file exists, Code::Blocks probably crashed, so you can prompt the user with options to use auto-saved versions of files (listed in the config file).
This attempt will not work, if a project is on a network share and is used from different systems.
If we really need such a fle, it should be side-by -side with the projectfile, like the *.layout file in my opinion.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Autosave enhancement
« Reply #15 on: November 17, 2013, 11:37:48 am »
That said, I personally don't deem project saving something that is really urgent for autosave at all. Changes are not that frequent and not that large.
I'm on the opposite opinion here :)
For me autosave is more usefull for saving projects and workspaces then source files.
I think so because there is no visual indicator in C::B to show/warn they are modified and so it is impossible to know if you've saved them or not.
So a crash might mean that you've lost some set up work you have to redo, which is annoying.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline Bat

  • Multiple posting newcomer
  • *
  • Posts: 48
Re: Autosave enhancement
« Reply #16 on: November 17, 2013, 12:43:46 pm »
Quote
I think so because there is no visual indicator in C::B to show/warn they are modified and so it is impossible to know if you've saved them or not.
So a crash might mean that you've lost some set up work you have to redo, which is annoying.

I'm agree with :
-no "visual indicator" aspect for project & workspace
-work is regularly saved each time we "build" (here no need)
-it will not get back modified file at last second before crash but one or 2 mintue ago (as plugin configuration)

Quote
I saw was a bool variable called "lock" which was set and verified at various places, presumably protecting the process of saving/loading project files and such.

Ok, see it.
It's usage is a "workaround only" for callback of Worskpace opening that is catched by
Code
cbEVT_WORKSPACE_LOADING_COMPLETE
In my mind, this event is aimed at end of Workspace loading and not during Workspace opening so "IsLoadingWorkspace" is false at this time and in plugin, I need to know when opening a file or a project if this is during Workspace loading, Project loading or file loading.
During Workspace loading, there is a summary of all not saved project - with selection possibility. Not a message by project basis.

=>Good option is perharps to create a
Code
cbEVT_WORKSPACE_OPEN
as, after that, I must also create
Code
cbEVT_WORKSPACE_CLOSED
where there is no entry point at all. With this, I can remove all this lock stuff

Are timer totally asynchronous of Manager (other threads) ? (I haven't check) ? Here there is potential problem even with actual autosave, as it's possible to close a project during autosave (and destroy project pointer). And even more with my patch.

Quote
You're adding code and APIs in cbEditor which are only usable by this feature of the autosave plugin.

I'm sorry, I but I have other question ...
Even if only autosave use it, any other plugin can also use it, if they need, no ? (I'm agree that I'm not sure to find another plugin that need to alter file at load time)
cbEditor don't need at all autosave plugin to work, flag is set to false by default and other parameters are all optional

To understand better, if I take another example :
Code
void SetErrorLine(int line);
is code in cbEditor that is only useful to compiler plugin
Code
void SetDebugLine(int line);
only useful to debugger plugin

At side from this, what is the (best) solution ?

My need :
get back a cbEditor at same state (modified flag is important) than during last autosave. This procedure is trigged during workspace loading, project loading or editor loading

First problem : load from a file and save to another (without actually do the save)
-I can externally from cbEditor load a file. But I will have the .save in m_Filename, so bad when user save editor
-I (perharps) can externally duplicate load process. It's ugly and will fail at first cbEditor modification

Second problem :
When my file is loaded, I set "modified" flag to true via SetModified(). I'm on cbEVT_EDITOR_OPEN event and there is plugin called before autosave (don't hurt), and after autosave. 2 or 3 of theses plugins after reset "modified" flag to false. So I can't assure that modified flag will really be set to true.
Here :
-should "modification" be made on all theses plugins ?
-should I create another event cbEVT_EDITOR_AFTER_OPEN ?
-other