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

[Tim Schumacher]

On 05.09.18 19:12, Junio C Hamano wrote:

Tim Schumacher <timsch...@gmx.de> writes:

@@ -691,17 +693,34 @@ static int run_argv(int *argcp, const char ***argv)
                /* .. then try the external ones */
                execv_dashed_external(*argv);

+ /* Increase the array size and add the current

+                * command to it.
+                */
+               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",
+                                   cmd_list[done_alias]);
+                       }
+               }
+

Wouldn't all of the above become three or four lines that is so
clear that there is no need for any comment if you used string-list,
perhaps?

Whoops, I didn't know that string-list existed. I'll try reworking the
code to use that. Concerning the comments: I planned to remove them
anyways since the code should be simple enough to be understood without
them already.

                /* It could be an alias -- this works around the insanity
                 * of overriding "git log" with "git show" by having
                 * alias.log = show
                 */

        /*
         * Style: our multi-line comment begins with and ends with
         * slash-asterisk and asterisk-slash on their own lines.
         */

I wasn't sure if I should have changed that (because I didn't introduce
that comment), but I can fix it in v3.

-               if (done_alias)
-                       break;
                if (!handle_alias(argcp, argv))
                        break;
-               done_alias = 1;
+               done_alias++;
        }

+ free(cmd_list);

+
        return done_alias;
  }