Re: [PATCH 2/5] _hg: declare appropriate local parameters for ->string form

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Mon, 6 Aug 2018 09:20:32 +0000
Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:

> Anton Shestakov wrote on Mon, Aug 06, 2018 at 15:28:10 +0800:
> > && ret=0 is also needed (not sure why, the help page doesn't explain
> > it), otherwise completing `hg diff -<TAB>` doesn't look right.  
> 
> Completion functions are supposed to return 0 if they added at least
> one match and non-zero otherwise.  It's common for completion
> functions to have the following form:
> 
>     local ret=1
>     _foo && ret=0
>     return ret
> 
> (usually with some code before/after the call to _foo)
> 
> Simply adding '&&ret=0' without declaring 'ret' local is a bug: it
> sets the variable in the caller's scope.  Furthermore, if 'ret' were
> declared local, setting it to 0 without using it afterwards would
> naturally have no effect.
> 
> I'm not sure what the correct fix is.

I don't see why it isn't just a local question in _hg_cmd_revert?

It needs to declare ret and combine the effect with the stuff in the
following block, then return ret explicitly.  I wouldn't be surprised if
_hg_cmd_resolve just above needed something similar.

> To -workers, couldn't we write a wrapper such as the following:
> 
>     _added_matches_p() {
>         local nmatches=$compstate[nmatches]
>         "$@"
>         (( nmatches < $compstate[nmatches] ))
>     }
> 
> to reduce the need to manage $ret in every single layer of the
> callstack?

Are you suggesting avoiding looking at the return value, but looking at
what its effect would have been at the point where you would have looked
it?

pws