Subject: Re: [PATCH/WIP v2 07/19] am: extract patch, message and authorship with git-mailinfo

From: Junio C Hamano <[email-masked]>
Date: Sun, 14 Jun 2015 15:10:24 -0700

Paul Tan <[email-masked]> writes:

> For the purpose of applying the patch and committing the results,
> implement extracting the patch data, commit message and authorship from
> an e-mail message using git-mailinfo.
>
> git-mailinfo is run as a separate process, but ideally in the future,
> we should be be able to access its functionality directly without
> spawning a new process.
>
> Helped-by: Junio C Hamano <[email-masked]>
> Helped-by: Jeff King <[email-masked]>
> Signed-off-by: Paul Tan <[email-masked]>
> ---
>
> Notes:
>     v2
>     
>     * use die_errno()
>     
>     * use '%*d' as the format specifier for msgnum()
>
>  builtin/am.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 228 insertions(+)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index 7379b97..a1db474 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -9,6 +9,23 @@
>  #include "parse-options.h"
>  #include "dir.h"
>  #include "run-command.h"
> +#include "quote.h"
> +
> +/**
> + * Returns 1 if the file is empty or does not exist, 0 otherwise.
> + */
> +static int is_empty_file(const char *filename)
> +{
> +	struct stat st;
> +
> +	if (stat(filename, &st) < 0) {
> +		if (errno == ENOENT)
> +			return 1;
> +		die_errno(_("could not stat %s"), filename);
> +	}
> +
> +	return !st.st_size;
> +}
>  
>  enum patch_format {
>  	PATCH_FORMAT_UNKNOWN = 0,
> @@ -23,6 +40,12 @@ struct am_state {
>  	int cur;
>  	int last;
>  
> +	/* commit message and metadata */
> +	struct strbuf author_name;
> +	struct strbuf author_email;
> +	struct strbuf author_date;
> +	struct strbuf msg;
> +
>  	/* number of digits in patch filename */
>  	int prec;
>  };
> @@ -35,6 +58,10 @@ static void am_state_init(struct am_state *state)
>  	memset(state, 0, sizeof(*state));
>  
>  	strbuf_init(&state->dir, 0);
> +	strbuf_init(&state->author_name, 0);
> +	strbuf_init(&state->author_email, 0);
> +	strbuf_init(&state->author_date, 0);
> +	strbuf_init(&state->msg, 0);
>  	state->prec = 4;
>  }
>  
> @@ -44,6 +71,10 @@ static void am_state_init(struct am_state *state)
>  static void am_state_release(struct am_state *state)
>  {
>  	strbuf_release(&state->dir);
> +	strbuf_release(&state->author_name);
> +	strbuf_release(&state->author_email);
> +	strbuf_release(&state->author_date);
> +	strbuf_release(&state->msg);
>  }
>  
>  /**
> @@ -93,6 +124,95 @@ static int read_state_file(struct strbuf *sb, const char *file, size_t hint, int
>  }
>  
>  /**
> + * Parses the "author script" `filename`, and sets state->author_name,
> + * state->author_email and state->author_date accordingly. We are strict with
> + * our parsing, as the author script is supposed to be eval'd, and loosely
> + * parsing it may not give the results the user expects.
> + *
> + * The author script is of the format:
> + *
> + * 	GIT_AUTHOR_NAME='$author_name'

It seems that you have SP * SP TAB GIT_AUTHOR_... here; lose SP
before the TAB?

> +	if (!skip_prefix(sb.buf, "GIT_AUTHOR_NAME=", (const char**) &value))

Style:

	if (!skip_prefix(sb.buf, "GIT_AUTHOR_NAME=", (const char **)&value))