Denis Auroux wrote:
> 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:
Thanks for the review. I stumbled upon the original patch on
freedomsponsors.org
and took a few minutes to update the patch without extensive review of the
feature on my part.
>
> 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.
Yes, calling "pidof" is not the best solution here and I didn't see this.
Spawning a duplicate file is one solution, typically handed with a special
character or hidden file (FOO.xoj~, or .FOO.xoj).
>
> 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.)
Xournal does not crash for me, and I don't recall users saying it crashes for
them. I assume, same as an office suite, auto-saving is for non-Xournal crash
cases - kernel crash, video driver crash, or power outages.
>
> 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?
Yes, I will defer to your expertise here.
>
> 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.
Don't feel this patch is high priority, but it would add some value. If you
want
to push the work back to me I'll lend a hand.
Thanks,
Michael
------------------------------------------------------------------------------
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
