Re: [RFC PATCH v2] Allow aliases that include other aliases

[Duy Nguyen <pclouds@gmail.com>]

On Wed, Sep 5, 2018 at 10:56 AM Tim Schumacher <timschumi@gmx.de> wrote:
>
> Aliases can only contain non-alias git commands and their
> arguments, not other user-defined aliases. Resolving further
> (nested) aliases is prevented by breaking the loop after the
> first alias was processed. Git then fails with a command-not-found
> error.
>
> Allow resolving nested aliases by not breaking the loop in
> run_argv() after the first alias was processed. Instead, continue
> incrementing `done_alias` until `handle_alias()` fails, which means that
> there are no further aliases that can be processed. Prevent looping
> aliases by storing substituted commands in `cmd_list` and checking if
> a command has been substituted previously.
> ---
>
> This is what I've come up with to prevent looping aliases. I'm not too
> happy with the number of indentations needed, but this seemed to be the
> easiest way to search an array for a value.

You can just make all the new code a separate function, which reduces
indentation.

There's another thing I wanted (but probably a wrong thing to want):
if I define alias 'foo' in ~/.gitconfig, then I'd like to modify it in
some project by redefining it as alias.foo='foo --something' in
$GIT_DIR/config. This results in alias loop, but the loop is broken by
looking up 'foo' from a higher level config file instead.

This is not easy to do, and as I mentioned, I'm not even sure if it's
a sane thing to do.

> +               /* Increase the array size and add the current
> +                * command to it.
> +                */

I think this is pretty clear from the code, you don't need to add a
comment to explain how the next few lines work. Same comment for the
next comment block.

> +               cmd_list_alloc += strlen(*argv[0]) + 1;
> +               REALLOC_ARRAY(cmd_list, cmd_list_alloc);
> +               cmd_list[done_alias] = *argv[0];
> +
> +               /* Search the array for occurrences of that command,
> +                * abort if something has been found.
> +                */
> +               for (int i = 0; i < done_alias; i++) {
> +                       if (!strcmp(cmd_list[i], *argv[0])) {
> +                               die("loop alias: %s is called twice",

Please wrap the string in _() so that it can be translated in
different languages.

> +                                   cmd_list[done_alias]);
> +                       }
> +               }
> +
-- 
Duy