« Return to Thread: WIP Note Directory Watcher Patch (was Watch for Changes Patch)

Re: WIP Note Directory Watcher Patch (was Watch for Changes Patch)

by Michael Fletcher-3 :: Rate this Message:

> * A lot of try/catching and error-checking to catch bad note file
> edits (needs more testing)

> * Add/Delete notes using proper Tomboy API

> * Use LoadForeignNoteXml to update more than just note title and content
> * Save DateTimes for Note.Saved events, use them to ignore file
> changes that happen within 3 seconds of such an event. I didn't do
> this with Note deletion events because they error out pretty
> gracefully already.
> * Prefix all log statements with "NoteDirectoryWatcher: "
>
> This will be part of Monday's release, so if you have time to review
> that would rock.
>
> Thanks,
> Sandy
>

> public class NoteDirectoryWatcherApplicationAddin : ApplicationAddin
> {
>+ // TODO: Use environment variable here?
> private static bool VERBOSE_LOGGING = false;
Um either way is good. It will spit out a ton of log messages if you
set this to true.

> + private void OnNoteSaved (Note note)
> + {
> + note_save_times [note.Id] = DateTime.Now;
> + }
Do you want to call this HandleNoteSaved?

> + // TODO: Why are we throwing an exception here?
> + // What are the repercussions of this?
> throw new Exception (message);
I threw an exception because it should not be possible. I have
handled all the values of enum WatcherChangeType.

Its a pattern I usually follow: if something impossible happens then
die gracefully instead of continuing on. Especially when you are
modifying data :).

> + if (!File.Exists (note_path)) {
> + // TODO: Any need to handle a deletion here?
> ... snipped ...
> + return;
> + }
Its very unlikely this would happen. And you have handled this in the
try-catch below.

> + string noteXml = null;
> + try {
> ... snipped ...
> + } catch (Exception e) {
> + Logger.Error ("NoteDirectoryWatcher: Update aborted, error reading {0}: {1}", note_path, e);
> + }
We need to return after logging this error. noteXml is set to null
and will throw a NPE shortly.

> + if (string.IsNullOrEmpty (noteXml)) {
> ... snipped ...
> + return;
> + }
Very good idea.

I wonder if we shouldn't put a try-catch around the call to
AddOrUpdateNote() and then eliminate some of the error handling code.
I think we could get rid of three exception handlers and a couple of
result checks.
_______________________________________________
Tomboy-list mailing list
Tomboy-list@...
http://lists.beatniksoftware.com/listinfo.cgi/tomboy-list-beatniksoftware.com

« Return to Thread: WIP Note Directory Watcher Patch (was Watch for Changes Patch)