Re: [PATCH 4/4] Add a --cover-letter option to format-patch

From: Peter Oberndorfer <kumbayo84@arcor.de>
Date: Feb 6, 2008, 18:21 UTC
Message-ID: <200802061921.58282.kumbayo84@arcor.de>
In-Reply-To: <alpine.LNX.1.00.0802061141410.13593@iabervon.org>

On Mittwoch 06 Februar 2008, Daniel Barkalow wrote:

Show 27 quoted lines

> If --cover-letter is provided, generate a cover letter message before
> the patches, numbered 0.
> 
> Original patch thanks to Johannes Schindelin
> 
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
> +static int reopen_stdout(const char *oneline, int nr, int total)
>  {
>  	char filename[PATH_MAX];
> -	char *sol;
>  	int len = 0;
>  	int suffix_len = strlen(fmt_patch_suffix) + 1;
>  
>  	if (output_directory) {
> -		if (strlen(output_directory) >=
> +		len = snprintf(filename, sizeof(filename), "%s",
> +				output_directory);
> +		if (len >=
>  		    sizeof(filename) - FORMAT_PATCH_NAME_MAX - suffix_len)
>  			return error("name of output directory is too long");
> -		strlcpy(filename, output_directory, sizeof(filename) - suffix_len);
> -		len = strlen(filename);
>  		if (filename[len - 1] != '/')
>  			filename[len++] = '/';
>  	}
>

*snip*

> +	if (!filename)
> +		len += sprintf(filename + len, "%d", nr);

maybe this should be !oneline instead?

Show 6 quoted lines

> +	else {
> +		len += sprintf(filename + len, "%04d-", nr);
> +		len += snprintf(filename + len, sizeof(filename) - len - 1
> +				- suffix_len, "%s", oneline);
>  		strcpy(filename + len, fmt_patch_suffix);
>  	}

Show 9 quoted lines

> +static void make_cover_letter(struct rev_info *rev,
> +		int use_stdout, int numbered, int numbered_files,
> +			      struct commit *origin, struct commit *head)
> +{
> +	const char *committer;
> +	const char *origin_sha1, *head_sha1;
> +	const char *argv[7];
> +	const char *subject_start = NULL;
> +	const char *body = "*** SUBJECT HERE ***\n\n\n*** BLURB HERE ***\n";

I don't know git policy but maybe use const char body[] = "*** SUBJECT HERE ***\n\n\n*** BLURB HERE ***\n"; since you don't change the pointer. (or remove the variable)

> +	const char *msg;
> +	const char *extra_headers = rev->extra_headers;
> +	struct strbuf sb;
> +	const char *encoding = "utf-8";

same here

> +
> +	if (rev->commit_format != CMIT_FMT_EMAIL)
> +		die("Cover letter needs email format");

It might be useful to split the reopen_stdout/get_oneline_for_filename into a separate patch.

When i tried to do this --cover-letter function i went the way to create a empty fake commit and let log_tree_commit do all the formating stuff for me. But i don't know if which way is preferred...

Does you patch set up a reply to chain, so patches are in reply to cover letter? I remember battling a bit to set it up reasonably.

Greetings Peter Who tried to create this --cover-letter function but gave up silently when his patch mails never reached the ML :-(

I don't know if my patches are of any use (do not apply cleanly anymore, reading cover letter message from a file does not honor encoding in any way) But i can send them to you if you want...