Re: [PATCH] Do not generate full commit log message if it not going to be used

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Tue, 27 Nov 2007, Alex Riesen wrote:

> Could not stop myself. Hopefully didn't beat anyone to it :)
> Almost all code shamelessly stolen from builtin-diff-index.c.

Then I have to wonder if it would not be a better idea to refactor the 
code, so that other people do not have to steal the code again, but are 
able to reuse it ;-)

> Preprocessor trickery in DIFF_OPT_* macros is disgusting, it breaks Vim 
> word completion and trying to use many flags in one expression looks 
> just ugly.

How does it break Vim word completion?  And why should something like

		DIFF_OPT_SET(&rev.diffopt, QUIET | EXIT_WITH_STATUS);

look ugly?  I find it highly readable.

> +	if (no_edit) {
> +		static const char *argv[] = { NULL, "HEAD", NULL };
> +		struct rev_info rev;
> +		unsigned char sha1[40];
> +		int is_initial;
> +
> +		fclose(fp);
> +
> +		if (!active_nr && read_cache() < 0)
> +			die("Cannot read index");
> +
> +		if (get_sha1("HEAD", sha1) != 0)
> +			return !!active_nr;

Don't want to be anal here, but are there possibly reasons (read "possible 
errors") other than an empty repo where this triggers?

> +
> +		init_revisions(&rev, "");
> +		rev.abbrev = 0;
> +		(void)setup_revisions(2, argv, &rev, NULL);

(void)?

Besides, would this not be more elegant as

		setup_revisions(0, NULL, &rev, "HEAD");

Hmm?

> +		(void)run_diff_index(&rev, 1 /* cached */);

(void)?

Other than that (including my remark about refactoring that piece of 
code), I like it.

Thanks,
Dscho