Taylor Blau <me@xxxxxxxxxxxx> writes: > I am not an expert or user of the Bash completion scripts in contrib, so > I'll refrain from reviewing that portion of the patch. > > I would, however, recommend that you avoid the word 'recursive' here. > Git rightly detects and rejects recursive and looping aliases. In fact, > the example that you give below: > >> l = log --oneline >> lg = l --graph > > Is not even recursive. I would instead recommend calling 'lg' a "nested" > alias. > > You could argue about whether it is "l", "lg", or both that are nested, > but I think renaming the patch to "completion: bash: support nested > aliases" and then a s/recursive/nested throughout the patch message > would be sufficient. > >> So the completion should detect such aliases as well. Two comments. - on design, is it possible to make a set of aliases that form a cycle? do we need to worry about such case? what does the current implementation do for an "alias" in such a cycle? - on implementation, it is done as a recursive call to the same function, but a loop that naturally maps tail recursion would also be a trivial implementation. is it worth rewriting the recursive calls into a loop? if we need to solve the circular references (above) by say limiting the length of the cycle, would such a rewrite make sense as a way to help implementation?
