Re: [PATCH 2/2] commit: add --skip-intent-to-add to allow commit with i-t-a entries in index

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 11, 2012, 23:55 UTC
Message-ID: <7vipkh6bg8.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1326275982-29866-3-git-send-email-pclouds@gmail.com>

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

Show 24 quoted lines

> ---
>  builtin/commit.c      |   10 +++++++---
>  cache-tree.c          |    8 +++++---
>  cache-tree.h          |    1 +
>  t/t2203-add-intent.sh |   17 +++++++++++++++++
>  4 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index bf42bb3..021206e 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -86,6 +86,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;
>  static int edit_flag = -1; /* unspecified */
>  static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
>  static int no_post_rewrite, allow_empty_message;
> +static int cache_tree_flags, skip_intent_to_add;
>  static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
>  static char *sign_commit;
>  
> @@ -170,6 +171,7 @@ static struct option builtin_commit_options[] = {
>  	OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"),
>  	OPT_BOOLEAN(0, "no-post-rewrite", &no_post_rewrite, "bypass post-rewrite hook"),
>  	{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, "mode", "show untracked files, optional modes: all, normal, no. (Default: all)", PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
> +	OPT_BOOL(0, "skip-intent-to-add", &skip_intent_to_add, "allow intent-to-add entries in index"),

This is more like "ignore", not "allow", from end user's point of view, no? The user earlier said "I cannot decide what contents to put in the commit yet for this path", and normally we catch it and remind the user that she needs to decide. This option gives her a quick way to say "I decide that I do not want to add this path at all to this commit I am creating, so please ignore it in the meantime."

Show 6 quoted lines

> @@ -1088,6 +1090,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
>  		cleanup_mode = CLEANUP_ALL;
>  	else
>  		die(_("Invalid cleanup mode %s"), cleanup_arg);
> +	if (skip_intent_to_add)
> +		cache_tree_flags = WRITE_TREE_INTENT_TO_ADD_OK;

The name WRITE_TREE_INTENT_TO_ADD_OK says "it is OK to call write-tree with i-t-a entries in the index, please do not barf", but I think "when writing a tree, ignore i-t-a entries" would be a more appropriate way to say the same thing, i.e. WRITE_TREE_IGNORE_INTENT_TO_ADD.

Other than that, I do not see an issue in the implementation of the patch. It is a separate design level issue if we want to worsen proliferation of the options, though.

Thanks.