On 05.09.18 17:48, Duy Nguyen wrote:
On Wed, Sep 5, 2018 at 10:56 AM Tim Schumacher <timschumi@xxxxxx> 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.
That would solve the issue, but I'm not sure if it is worth introducing
a new function exclusively for that. I didn't find anything about a
maximum indentation level in the code guidelines and since the new
parts stay within the width limit (and is imo still readable), would it
be ok to keep it like that?
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.
The alias system is using the default functions of the config system,
I assume that adding such a functionality is not possible, at least not
without breaking compatibility.
+ /* 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.
I'll remove them in v3.
+ 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.
I'll do that in v3 as well.
+ cmd_list[done_alias]);
+ }
+ }
+
Thanks for reviewing!
Tim
