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

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

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.

>

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?

>

Cheers,

Daniel

> @@ -715,14 +717,14 @@ _hg_cmd_resolve() {
>  }
>  
>  _hg_cmd_revert() {
> -  local context state line
> +  local context state state_descr line
>    typeset -A opt_args
>  
>    _arguments -s : $_hg_global_opts $_hg_pat_opts $_hg_dryrun_opts \
>    '(--all -a :)'{-a,--all}'[revert all changes when no arguments given]' \
>    '(--rev -r)'{-r+,--rev=}'[revision to revert to]:revision:_hg_tags' \
>    '--no-backup[do not save backup copies of files]' \
> -  '*:file:->diff_files'
> +  '*:file:->diff_files' && ret=0
>  
>    if [[ $state == 'diff_files' ]]
>    then