From: Mark Walters <markwalters1009@gmail.com>
To: Ethan Glasser-Camp <ethan.glasser.camp@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 5/8] notmuch-new: pull out useful bits of
	add_files_recursive
In-Reply-To: <1341160790-14525-6-git-send-email-ethan@betacantrips.com>
References:
 <CAOJ+Ob0MSOez2MvD2fCgF7t32kFPk4g2+xCud88QmBLt_b5pOA@mail.gmail.com>
	<1341160790-14525-1-git-send-email-ethan@betacantrips.com>
	<1341160790-14525-6-git-send-email-ethan@betacantrips.com>
User-Agent: Notmuch/0.13.2+70~gb6a56e7 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Sun, 01 Jul 2012 20:55:11 +0100
Message-ID: <87ehov2r4w.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Ethan Glasser-Camp <ethan@betacantrips.com>
Precedence: list

On Sun, 01 Jul 2012, Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> wrote:
> This patch pulls some bits out of add_files_recursive which will be
> useful for other backends: two reporting functions
> _report_before_adding_file and _report_added_file, as well as
> _add_message, which actually does the message adding.
>
> No functional changes.

This looks fine. Personally, I would split it into two or three bits so
it is easier to see that it is just code movement.

Best wishes

Mark

>
> Signed-off-by: Ethan Glasser-Camp <ethan@betacantrips.com>
> ---
>  notmuch-new.c |  192 +++++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 119 insertions(+), 73 deletions(-)
>
> diff --git a/notmuch-new.c b/notmuch-new.c
> index 8377750..5250562 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -239,6 +239,122 @@ _entry_in_ignore_list (const char *entry, add_files_state_t *state)
>      return FALSE;
>  }
>  
> +/* Progress-reporting function.
> + *
> + * Can be used by any mailstore-crawling function that wants to alert
> + * users what message it's about to add. Subsequent errors will be due
> + * to this message ;)
> + */
> +static void
> +_report_before_adding_file (add_files_state_t *state, const char *filename)
> +{
> +    state->processed_files++;
> +
> +    if (state->verbose) {
> +	if (state->output_is_a_tty)
> +	    printf("\r\033[K");
> +
> +	printf ("%i/%i: %s",
> +		state->processed_files,
> +		state->total_files,
> +		filename);
> +
> +	putchar((state->output_is_a_tty) ? '\r' : '\n');
> +	fflush (stdout);
> +    }
> +}
> +
> +/* Progress-reporting function.
> + *
> + * Call this to respond to the signal handler for SIGALRM.
> + */
> +static void
> +_report_added_file (add_files_state_t *state)
> +{
> +    if (do_print_progress) {
> +	do_print_progress = 0;
> +	generic_print_progress ("Processed", "files", state->tv_start,
> +				state->processed_files, state->total_files);
> +    }
> +}
> +
> +
> +/* Atomically handles adding a message to the database.
> + *
> + * Should be used by any mailstore-crawling function that finds a new
> + * message to add.
> + */
> +static notmuch_status_t
> +_add_message (add_files_state_t *state, notmuch_database_t *notmuch,
> +	      const char *filename)
> +{
> +    notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
> +    notmuch_message_t *message;
> +    const char **tag;
> +
> +    status = notmuch_database_begin_atomic (notmuch);
> +    if (status) {
> +	ret = status;
> +	goto DONE;
> +    }
> +
> +    status = notmuch_database_add_message (notmuch, filename, &message);
> +
> +    switch (status) {
> +    /* success */
> +    case NOTMUCH_STATUS_SUCCESS:
> +	state->added_messages++;
> +	notmuch_message_freeze (message);
> +	for (tag=state->new_tags; *tag != NULL; tag++)
> +	    notmuch_message_add_tag (message, *tag);
> +	if (state->synchronize_flags == TRUE)
> +	    notmuch_message_maildir_flags_to_tags (message);
> +	notmuch_message_thaw (message);
> +	break;
> +    /* Non-fatal issues (go on to next file) */
> +    case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
> +	if (state->synchronize_flags == TRUE)
> +	    notmuch_message_maildir_flags_to_tags (message);
> +	break;
> +    case NOTMUCH_STATUS_FILE_NOT_EMAIL:
> +	fprintf (stderr, "Note: Ignoring non-mail file: %s\n",
> +		 filename);
> +	break;
> +    /* Fatal issues. Don't process anymore. */
> +    case NOTMUCH_STATUS_READ_ONLY_DATABASE:
> +    case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
> +    case NOTMUCH_STATUS_OUT_OF_MEMORY:
> +	fprintf (stderr, "Error: %s. Halting processing.\n",
> +		 notmuch_status_to_string (status));
> +	ret = status;
> +	goto DONE;
> +    default:
> +    case NOTMUCH_STATUS_FILE_ERROR:
> +    case NOTMUCH_STATUS_NULL_POINTER:
> +    case NOTMUCH_STATUS_TAG_TOO_LONG:
> +    case NOTMUCH_STATUS_UNBALANCED_FREEZE_THAW:
> +    case NOTMUCH_STATUS_UNBALANCED_ATOMIC:
> +    case NOTMUCH_STATUS_LAST_STATUS:
> +	INTERNAL_ERROR ("add_message returned unexpected value: %d",  status);
> +	ret = status;
> +	goto DONE;
> +    }
> +
> +    status = notmuch_database_end_atomic (notmuch);
> +    if (status) {
> +	ret = status;
> +	goto DONE;
> +    }
> +
> +  DONE:
> +    if (message) {
> +	notmuch_message_destroy (message);
> +	message = NULL;
> +    }
> +
> +    return ret;
> +}
> +
>  /* Examine 'path' recursively as follows:
>   *
>   *   o Ask the filesystem for the mtime of 'path' (fs_mtime)
> @@ -290,7 +406,6 @@ add_files (notmuch_database_t *notmuch,
>      char *next = NULL;
>      time_t fs_mtime, db_mtime;
>      notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
> -    notmuch_message_t *message = NULL;
>      struct dirent **fs_entries = NULL;
>      int i, num_fs_entries = 0, entry_type;
>      notmuch_directory_t *directory;
> @@ -299,7 +414,6 @@ add_files (notmuch_database_t *notmuch,
>      time_t stat_time;
>      struct stat st;
>      notmuch_bool_t is_maildir;
> -    const char **tag;
>  
>      if (stat (path, &st)) {
>  	fprintf (stderr, "Error reading directory %s: %s\n",
> @@ -469,83 +583,15 @@ add_files (notmuch_database_t *notmuch,
>  	 * in the database, so add it. */
>  	next = talloc_asprintf (notmuch, "%s/%s", path, entry->d_name);
>  
> -	state->processed_files++;
> -
> -	if (state->verbose) {
> -	    if (state->output_is_a_tty)
> -		printf("\r\033[K");
> -
> -	    printf ("%i/%i: %s",
> -		    state->processed_files,
> -		    state->total_files,
> -		    next);
> -
> -	    putchar((state->output_is_a_tty) ? '\r' : '\n');
> -	    fflush (stdout);
> -	}
> +	_report_before_adding_file (state, next);
>  
> -	status = notmuch_database_begin_atomic (notmuch);
> +	status = _add_message (state, notmuch, next);
>  	if (status) {
>  	    ret = status;
>  	    goto DONE;
>  	}
>  
> -	status = notmuch_database_add_message (notmuch, next, &message);
> -	switch (status) {
> -	/* success */
> -	case NOTMUCH_STATUS_SUCCESS:
> -	    state->added_messages++;
> -	    notmuch_message_freeze (message);
> -	    for (tag=state->new_tags; *tag != NULL; tag++)
> -	        notmuch_message_add_tag (message, *tag);
> -	    if (state->synchronize_flags == TRUE)
> -		notmuch_message_maildir_flags_to_tags (message);
> -	    notmuch_message_thaw (message);
> -	    break;
> -	/* Non-fatal issues (go on to next file) */
> -	case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
> -	    if (state->synchronize_flags == TRUE)
> -		notmuch_message_maildir_flags_to_tags (message);
> -	    break;
> -	case NOTMUCH_STATUS_FILE_NOT_EMAIL:
> -	    fprintf (stderr, "Note: Ignoring non-mail file: %s\n",
> -		     next);
> -	    break;
> -	/* Fatal issues. Don't process anymore. */
> -	case NOTMUCH_STATUS_READ_ONLY_DATABASE:
> -	case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
> -	case NOTMUCH_STATUS_OUT_OF_MEMORY:
> -	    fprintf (stderr, "Error: %s. Halting processing.\n",
> -		     notmuch_status_to_string (status));
> -	    ret = status;
> -	    goto DONE;
> -	default:
> -	case NOTMUCH_STATUS_FILE_ERROR:
> -	case NOTMUCH_STATUS_NULL_POINTER:
> -	case NOTMUCH_STATUS_TAG_TOO_LONG:
> -	case NOTMUCH_STATUS_UNBALANCED_FREEZE_THAW:
> -	case NOTMUCH_STATUS_UNBALANCED_ATOMIC:
> -	case NOTMUCH_STATUS_LAST_STATUS:
> -	    INTERNAL_ERROR ("add_message returned unexpected value: %d",  status);
> -	    goto DONE;
> -	}
> -
> -	status = notmuch_database_end_atomic (notmuch);
> -	if (status) {
> -	    ret = status;
> -	    goto DONE;
> -	}
> -
> -	if (message) {
> -	    notmuch_message_destroy (message);
> -	    message = NULL;
> -	}
> -
> -	if (do_print_progress) {
> -	    do_print_progress = 0;
> -	    generic_print_progress ("Processed", "files", state->tv_start,
> -				    state->processed_files, state->total_files);
> -	}
> +	_report_added_file (state);
>  
>  	talloc_free (next);
>  	next = NULL;
> -- 
> 1.7.9.5
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch