[PATCH 1/2] alias: Rework comment about processing aliases

Tim Schumacher· Oct 21, 2018, 18:46 UTC · lore

The old comment's message wasn't really clear and it was in a weird location for it to talk about the alias handling process as a whole.

Rephrase and move it to the top of the while() loop to make the
message more clear.
---
 git.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Show changes to git.c +5 −5

diff --git a/git.c b/git.c
index 6a81ed6fc..0f77bce7d 100644
--- a/git.c
+++ b/git.c
@@ -718,6 +718,11 @@ static int run_argv(int *argcp, const char ***argv)
 	struct strbuf env = STRBUF_INIT;
 
 	init_cmd_history(&env, &cmd_list);
+
+	/*
+	 * Check if argv[0] is a command before seeing if it is an
+	 * alias to avoid taking over existing commands
+	 */
 	while (1) {
 		/*
 		 * If we tried alias and futzed with our environment,
@@ -753,11 +758,6 @@ static int run_argv(int *argcp, const char ***argv)
 
 		add_cmd_history(&env, &cmd_list, *argv[0]);
 
-		/*
-		 * It could be an alias -- this works around the insanity
-		 * of overriding "git log" with "git show" by having
-		 * alias.log = show
-		 */
 		if (!handle_alias(argcp, argv))
 			break;
 		done_alias = 1;

-- 
2.19.1.450.ga4b8ab536

Tim Schumacher· Oct 21, 2018, 18:46 UTC · re: Tim Schumacher · lore

[PATCH 2/2] alias: Move checking code into a seperate function

We can save a few indentations (and possibly brain cells of people that don't care about that code) by moving the code that checks for a looping alias (and that prints the error message if one is found) into a seperate function.

This restores a lot of readablility to the run_argv() function as
well, because it was only concerned with the high-level routing of
the command and alias logic before.
---
 git.c | 40 +++++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 17 deletions(-)

Show changes to git.c +23 −17

diff --git a/git.c b/git.c
index 0f77bce7d..b6fdd9708 100644
--- a/git.c
+++ b/git.c
@@ -710,11 +710,32 @@ static void add_cmd_history(struct strbuf *env, struct string_list *cmd_list,
 	setenv(COMMAND_HISTORY_ENVIRONMENT, env->buf, 1);
 }
 
+static void cmd_unique_or_die(struct string_list *cmd_list, const char *cmd)
+{
+	struct string_list_item *seen;
+
+	seen = unsorted_string_list_lookup(cmd_list, cmd);
+	if (!seen)
+		return;
+
+	int i;
+	struct strbuf sb = STRBUF_INIT;
+	for (i = 0; i < cmd_list->nr; i++) {
+		struct string_list_item *item = &cmd_list->items[i];
+		strbuf_addf(&sb, "\n  %s", item->string);
+		if (item == seen)
+			strbuf_addstr(&sb, " <==");
+		else if (i == cmd_list->nr - 1)
+			strbuf_addstr(&sb, " ==>");
+	}
+	die(_("alias loop detected: expansion of '%s' does not terminate:%s"),
+	    cmd_list->items[0].string, sb.buf);
+}
+
 static int run_argv(int *argcp, const char ***argv)
 {
 	int done_alias = 0;
 	struct string_list cmd_list = STRING_LIST_INIT_DUP;
-	struct string_list_item *seen;
 	struct strbuf env = STRBUF_INIT;
 
 	init_cmd_history(&env, &cmd_list);
@@ -739,22 +760,7 @@ static int run_argv(int *argcp, const char ***argv)
 		/* .. then try the external ones */
 		execv_dashed_external(*argv);
 
-		seen = unsorted_string_list_lookup(&cmd_list, *argv[0]);
-		if (seen) {
-			int i;
-			struct strbuf sb = STRBUF_INIT;
-			for (i = 0; i < cmd_list.nr; i++) {
-				struct string_list_item *item = &cmd_list.items[i];
-
-				strbuf_addf(&sb, "\n  %s", item->string);
-				if (item == seen)
-					strbuf_addstr(&sb, " <==");
-				else if (i == cmd_list.nr - 1)
-					strbuf_addstr(&sb, " ==>");
-			}
-			die(_("alias loop detected: expansion of '%s' does"
-			      " not terminate:%s"), cmd_list.items[0].string, sb.buf);
-		}
+		cmd_unique_or_die(&cmd_list, *argv[0]);
 
 		add_cmd_history(&env, &cmd_list, *argv[0]);

-- 
2.19.1.450.ga4b8ab536

patch, 2 partsalias: Rework comment about processing aliases