Re: [PATCH v3 3/3] checkout: reorder option handling

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 7, 2012, 20:19 UTC
Message-ID: <7vpq5xwppc.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1346330750-17937-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> +	if (opts->track != BRANCH_TRACK_UNSPECIFIED)
> +		die(_("%s cannot be used with updating paths"), "--track");

I think most of the places we try to enclose these literals inside quotes, so I'd squash in a patch to make this:

		die(_("'%s' cannot be used with updating paths"), "--track");

and update others to match.

> +	if (new->name && !new->commit)
> +		die(_("Cannot switch branch to a non-commit '%s'."),
> +		    new->name);

This is one of the only few places that end a sentence with a full-stop.

> +		/* Try to give more helpful suggestion, new_branch &&
> +		   argc > 1 will be caught later */

Style;

Show 7 quoted lines

> +		if (opts.new_branch && argc == 1)
> +			die(_("Cannot update paths and switch to branch '%s' at the same time.\n"
> +			      "Did you intend to checkout '%s' which can not be resolved as commit?"),
> +			    opts.new_branch, argv[0]);
>  
>  		if (opts.force_detach)
>  			die(_("git checkout: --detach does not take a path argument"));

I tried this codepath and the message left me feeling uneasy.  I'd do:

		die(_("git checkout: --detach does not take a path argument '%s'"),
			argv[0]);

Other than these, I didn't find anything obviously wrong with my final review before merging to 'next', so I'll merge it soonish.

Thanks.