Re: [GSoC][PATCH 3/5 v3] submodule: port set_name_rev() from shell to C

Prathamesh Chavan [off-list ref] writes:

Hmph.

The only difference between the steps being what subcommand is run,
a better implementation might be to do

	static const char *describe_bare[] = {
		NULL
	};

	...

	static const char **describe_argv[] = {
		describe_bare, describe_tags, describe_contains, 
                describe_all_always, NULL
	};

	const char ***d;

	for (d = describe_argv; *d; d++) {
		argv_array_pushl(&cp.args, "describe");
		argv_array_pushv(&cp.args, *d);
		... do the thing ...
	}

but unfortunately C is a bit klunky to do so; we cannot easily make
the contents of describe_argv[] as anonymous arrays.  An otherwise
useless enum stil bothers me, but I do not think of anything better
offhand.

+
+static char *get_name_rev(const char *sub_path, const char* object_id)
+{
+	struct strbuf sb = STRBUF_INIT;
+	enum describe_step cur_step;
+
+	for (cur_step = step_bare; cur_step < step_end; cur_step++) {
+		struct child_process cp = CHILD_PROCESS_INIT;
+		prepare_submodule_repo_env(&cp.env_array);
+		cp.dir = sub_path;
+		cp.git_cmd = 1;
+		cp.no_stderr = 1;
+
+		switch (cur_step) {
+			case step_bare:
+				argv_array_pushl(&cp.args, "describe",
+						 object_id, NULL);
+				break;
+			case step_tags:	
+				argv_array_pushl(&cp.args, "describe",
+						 "--tags", object_id, NULL);
+				break;
+			case step_contains:
+				argv_array_pushl(&cp.args, "describe",
+						 "--contains", object_id,
+						 NULL);
+				break;
+			case step_all_always:
+				argv_array_pushl(&cp.args, "describe",
+						 "--all", "--always",
+						 object_id, NULL);
+				break;
+			default:
+				BUG("unknown describe step '%d'", cur_step);
+		}

Dedent the body of switch() by one level, i.e.

	switch (var) {
	case val1:
		do_this();
		break;
	case val2:
		do_that();
		...
	}

Otherwise looking good.