Hi Michael,
Thanks for updating this patch. There are various things I'll want to
change before this can get into the official code base. You don't need
to do any of these yourself unless you feel strongly motivated -- my
approach is always to rewrite patches as I incorporate them. A few main
areas where thought is needed:
1. One thing I don't really like is the manner in which the multiple
instances are kept track of in a central table directory and re-spawned
if necessary: calling a shell with "pidof xournal" won't work on the
win32 platform (I think), and the whole thing seems too complicated to
be 100% reliable. It might be easier to just have each instance create
its file called "....xoj.autosave-(pid).xoj" alongside the file being
edited and trust the user to recover the autosave files manually in case
of a crash (or warn of their existence). There just doesn't seem to be
any clean way to handle PIDs and look up running instances, so a
different mechanism is needed.
Question: how often does xournal crash on you? Are there ever cases
where you would need to rely on the autosave file being located for you,
rather than your being aware of the autosave file's existence and being
able to open it yourself?
The easiest solution would be to let the user open the autosave .xoj
file (instead of the original .xoj), but when opening an autosave file,
prompt the user about whether to rename it back to the original file
name ("recover" the autosave). This avoids a lot of the complexities of
keeping track of multiple autosave files that might lie around, at the
expense of expecting the user to be aware of the existence of the
autosave file and actively wanting to open it.
I'd prefer this, if there is a feeling that it is an acceptable way to
proceed. Otherwise, suggestions are welcome.
(Note: one could also warn the user more actively about the existence of
an autosave file for the .xoj they're about to open, but given that
multiple instances of xournal editing a same file seem more common than
crashes, I am not sure that this works well without the pidof trick; and
I don't want to rely on the pidof command in the official code.)
2. I'm also not sure I like the manner in which changes are flagged
alongside each operation (and some may be missing) and the autosave
feature is locked manually in some operations only (and some definitely
are missing). It would be cleaner to test whether things have changed by
checking the contents of ui.saved (remembering to delete the autosave or
update it when we actually save the file the normal way), and whether
there is an operation in progress by checking ui.cur_item_type ==
ITEM_NONE (perhaps also allow a few other item types that are not too
interactive if the save operation isn't mangled by them, e.g. what if
there is a text edition in progress?). Also, the timeout delay (2
seconds) should be customizable but deferrals should get handled on a
faster loop in case the user only wants to auto-save every 10 minutes
and we miss our turn.
Does this seem reasonable?
As you can tell, there'll be some more work needed. I'm game to try and
do this in the coming weeks (assuming no distractions from real-life
world to keep me away from xournal again) -- some confirmation that my
thinking on issue #1 above isn't crazy would be good.
Denis
On 05/28/2014 06:23 AM, Michael Cronenworth wrote:
> Hello,
>
> I updated a patch that implements auto-saving in Xournal and would like
> to see it implemented. Could you review the patch and make any comment?
> I have attached a patch to sourceforge made against git master.
>
> http://sourceforge.net/p/xournal/patches/62/
>
> Thanks,
> Michael
--
Denis Auroux aur...@math.berkeley.edu
University of California, Berkeley Tel: 510-642-4367
Department of Mathematics Fax: 510-642-8204
817 Evans Hall # 3840
Berkeley, CA 94720-3840
------------------------------------------------------------------------------
Time is money. Stop wasting it! Get your web API in 5 minutes.
www.restlet.com/download
http://p.sf.net/sfu/restlet
_______________________________________________
Xournal-devel mailing list
Xournal-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/xournal-devel
