Re: [RFC PATCH 1/1] completion: load completion file for external subcommand

["Florian Gamböck" <mail@floga.de>]

On 2018-04-09 18:36, Junio C Hamano wrote:
> Florian Gamböck <mail@floga.de> writes:
> 
> > Good point. I could go even further and ditch the if-construct:
> >
> >    ! declare -f $completion_func && declare -f __load_completion &&
> >        __load_completion "git-$command"
> 
> I personally find that a lot harder to read than if/then/fi.

Then I mis-understood you the first time. It sounded a bit as if you
wanted to avoid if-fi. After all, the rest of this code block also uses
quite long &&-chains.

Then we are back at the first question:

> >  	local completion_func="_git_${command//-/_}"
> > +	if ! declare -f $completion_func >/dev/null 2>/dev/null; then
> > +		declare -f __load_completion >/dev/null 2>/dev/null &&
> > +			__load_completion "git-$command"
> 
> wouldn't the above be easier to read if it were
> 
> 	if ! declare ... $completion_func ... && declare -f __load_completion
> 	then
> 		__load_completion "git-$command"
> 	fi
> 
> or is there a reason why it is better to &&-chain the check for
> __load_completion with its use?

As for readability, I would prefer my first approach then with the
following reasoning: Checking the existence of a function and actually
calling it can be seen as a unit. Either the function does not exist or
you call it. You could even create a function like "call_if_exists",
that does exactly this. Either way, at the end of the line you are
smarter than before. As for the if-statement, this describes the reason
why you even want to load an external file. If the function in question
($completion_func) already exists, we do not want to load it again. If
you chain the statement with the existence check of __load_completion,
you make it look like those two functions are somehow related, which is
not the case.

To put it in words: "If $completion_func does not already exist, then
load another file (if you know how to do that)." versus "If
$completion_func does not yet exist and you know how to load another
file, then load another file." The difference is subtle, but I think the
first sentence better describes the intention.

Does my reasoning make sense? I mean, the result will be exactly the
same, we are clearly only talking about readability here.