Re: [PATCH 2/3] add--interactive: add builtin helper for interactive add

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, May 5, 2017 at 8:43 PM, Daniel Ferreira <bnmvco@gmail.com> wrote:
> Create a builtin helper for git-add--interactive, which right now is
> only able to reproduce git-add--interactive.perl's status_cmd()
> function, providing a summarized diff numstat to the user.

I'm not a user of add -i and didn't review the main logic in detail,
but I ran this, and aside from two issues this LGTM:

 * You missed a trailing \n in the output, so your formatting is
   different from the current behavior.

 * You should be using the getopt API instead of rolling your own.

Fixes for both below, thanks a lot for hacking on this. As you pointed
out this doesn't help with removing any of the perl code for now, but
after replacing a few more command modes we can start chopping away at
the perl code.

diff --git a/builtin/add-interactive--helper.c b/builtin/add-interactive--helper.c
index 97ca1b38dc..ea0f790bd3 100644
--- a/builtin/add-interactive--helper.c
+++ b/builtin/add-interactive--helper.c
@@ -226,6 +226,7 @@ static void print_modified(void)
 		printf(modified_fmt, index_changes, worktree_changes, f.path);
 		printf("\n");
 	}
+	printf("\n");
 }
 
 static void status_cmd(void)
@@ -233,26 +234,31 @@ static void status_cmd(void)
 	print_modified();
 }
 
-static const char add_interactive_helper_usage[] =
-"git add-interactive--helper <command>";
+static const char * const builtin_add_interactive_helper_usage[] = {
+	N_("git add-interactive--helper <command>"),
+	NULL
+};
 
 int cmd_add_interactive__helper(int argc, const char **argv, const char *prefix)
 {
-	int i, found_opt = 0;
-
-	git_config(git_add_interactive_config, NULL);
+	int opt_status = 0;
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
+	struct option options[] = {
+		OPT_BOOL(0, "status", &opt_status,
+			 N_("print status information with diffstat")),
+		OPT_END()
+	};
 
-		if (!strcmp(arg, "--status")) {
-			status_cmd();
-			found_opt = 1;
-		}
-	}
-
-	if (!found_opt)
-		usage(add_interactive_helper_usage);
+	git_config(git_add_interactive_config, NULL);
+	argc = parse_options(argc, argv, NULL, options,
+			     builtin_add_interactive_helper_usage,
+			     PARSE_OPT_KEEP_ARGV0);
+
+	if (opt_status)
+		status_cmd();
+	else
+		usage_with_options(builtin_add_interactive_helper_usage,
+				   options);
 
 	return 0;
 }
-- 
2.11.0