Re: [PATCH] completion: bash: support recursive aliases
Taylor Blau <me@ttaylorr.com> writes:
Show 19 quoted lines
> 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?
Junio C Hamano <gitster@pobox.com>