From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Sun, 12 May 2013 16:54:37 GMT
Subject: Re: [PATCH v7 01/10] Add support for -i/--interactive to git-clean
Message-ID: <vpqvc6oi1f6.fsf@grenoble-inp.fr>
In-Reply-To: <ad26375dece854339d64dcb82c17f19f8edccf48.1368011946.git.worldhello.net@gmail.com>

Jiang Xin <worldhello.net@gmail.com> writes:

> +			putchar('\n');
> +
> +			/* Display dels in "Would remove ..." format */
> +			for_each_string_list_item(item, &del_list) {
> +				qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
> +				printf(_(msg_would_remove), qname);
> +			}
> +			putchar('\n');


> +		putchar('\n');
> +
> +		/* Display dels in "Would remove ..." format */
> +		for_each_string_list_item(item, &del_list) {
> +			qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
> +			printf(_(msg_would_remove), qname);
> +		}
> +		putchar('\n');

These two pieces of code are surprisingly similar ... Shouldn't they be
factored into a small helper function?

> +		/* Confirmation dialog */
> +		printf(_("Remove ([y]es/[n]o/[e]dit) ? "));

To be more consistent with "git add -p", this should use [] instead of
(), and have no space before "?".

> +			die(_("clean.requireForce defaults to true and neither -i, -n nor -f given; "
>  				  "refusing to clean"));

That makes it a 85 characters message, and we usually break lines before
80. Adding \n after ";" (instead of a space) would be better IMO.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/