Re: [PATCH v3] Allow aliases that include other aliases
- Date: Thu, 6 Sep 2018 10:59:42 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH v3] Allow aliases that include other aliases
On Thu, Sep 06, 2018 at 12:16:58PM +0200, Tim Schumacher wrote:
> @@ -691,17 +692,23 @@ static int run_argv(int *argcp, const char ***argv)
> /* .. then try the external ones */
> - /* It could be an alias -- this works around the insanity
> + if (string_list_has_string(&cmd_list, *argv))
> + die(_("loop alias: %s is called twice"), *argv);
I pointed this out in my response to Ævar, but I want to make sure it
gets seen. This call assumes the list is sorted, but...
> + string_list_append(&cmd_list, *argv);
This will create an unsorted list. You'd have to use
string_list_insert() here for a sorted list, or
unsorted_string_list_has_string() in the earlier call.
It's unfortunate that string_list makes this so easy to get wrong.
> + /*
> + * It could be an alias -- this works around the insanity
> * of overriding "git log" with "git show" by having
> * alias.log = show
> - if (done_alias)
> - break;
> if (!handle_alias(argcp, argv))
> - done_alias = 1;
> + done_alias++;
I don't think anybody cares about done_alias being an accurate count.
Should we just leave this as-is?