[tomboy-list] WIP Note Directory Watcher Patch (was Watch for Changes Patch)

[Michael Fletcher]

> * 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.