refactor: { => _comp_compgen}_filedir and _comp_compgen [opts] NAME

[akinomyoga]

I came up with a new idea for #948 (comment). This PR implements the idea using _filedir as an example.

1. First, the function _filedir is renamed to _comp_compgen_filedir. This function can be directly called by _comp_compgen_filedir ... as we have been doing so far but can also be called as _comp_compgen filedir ....

$ COMPREPLY=(1 2 3); cur=c
$ _comp_compgen_filedir; echo "${COMPREPLY[*]}"
1 2 3 configure completions config.log config.status configure~ configure.ac

$ COMPREPLY=(1 2 3); cur=c
$ _comp_compgen filedir; echo "${COMPREPLY[*]}"
1 2 3 configure completions config.log config.status configure~ configure.ac

In the above, _comp_compgen redirects the call _comp_compgen NAME to the shell function _comp_compgen_NAME in a similar way as xfunc. Note that this is not ambiguous with the call _comp_compgen -- <comgpen args> because all the arguments to the compgen builtin start with - while our function names do not contain -. When the first (non-option) argument of _comp_compgen does not start with -, _comp_compgen redirects the call to "_comp_compgen_$1". Otherwise, _comp_compgen can consider the arguments to be passed to the compgen builtin.

2. The options specified to _comp_compgen will take effect in the behavior of the redirected _comp_compgen_filedir. For example, _comp_compgen_filedir stores the result in COMPREPLY by default, but we can instead store the results to a specified array arr by _comp_compgen -v arr filedir. This is done by recording the parsed options in local variables _comp_compgen__* and letting the child calls of _comp_compgen inside _comp_compgen_filedir reference the local variables.

$ COMPREPLY=(1 2 3); arr=(); cur=c
$ _comp_compgen -v arr filedir
$ echo "COMPREPLY: ${COMPREPLY[*]}"
COMPREPLY: 1 2 3
$ echo "arr: ${arr[*]}")
arr: configure completions config.log config.status configure~ configure.ac

Commit fcf9825 implements the above two extensions of _comp_compgen. Actually, the implementation is simple.

---

The modification at the _filedir side is minimal as far as the completions are generated through _comp_compgen. Only the codes that directly reference cur and COMPREPLY need to be updated.

edit: I also changed the behavior of _comp_compgen_filedir so that it replaces the existing COMPREPLY by default. To make it append to the array, one can specify the -a option as _comp_compgen -a filedir. Currently, many similar functions just replace the existing contents, but only a part of them try to append to the array. I'd like to make the default behaviors of similar functions consistent and allow appending as an option -a.

[scop]

Oh, very nice!

I did not look into usages yet, but I thought I'd just ask -- in the vast majority of cases we have I believe _filedir's behavior of appending to COMPREPLY isn't required and we could replace instead. Usually superflous append does no harm either though, because COMPREPLY is in most cases empty at that time. What do you think, while we're working on this, should we also review each call site and remove the append behavior where unnecessary? I think doing so would reveal and fix some bugs, but this is quite a bit of work for a smallish gain. We can also postpone to later if seen useful but not something we'd like to do now.

[akinomyoga]

in the vast majority of cases we have I believe _filedir's behavior of appending to COMPREPLY isn't required and we could replace instead.

I agree with it, but we need to carefully check each case not to break existing behavior. In removing flag -a, we need to make sure that COMPREPLY is empty at that point (or we do want to discard the existing elements of COMPREPLY). I expect most of them would be easy, but I'm afraid that there would be not a small number of cases that it is subtle.

What do you think, while we're working on this, should we also review each call site and remove the append behavior where unnecessary?

Actually, at the current stage, the new implementation of _comp_compgen_filedir is incomplete. For example, _tilde called from _comp_compgen_filedir should also be upgraded to the _comp_compgen_tilde style for the sake of completeness. Let me first try to make _comp_compgen_filedir complete by upgrading related functions.

[akinomyoga]

The function _comp_cmd_lintian is suspicious.

bash-completion/completions/lintian

Lines 70 to 145 in a26a2a9

	_comp_cmd_lintian()
	{
	local cur prev words cword comp_args
	_comp_initialize -- "$@" || return
	local lint_actions general_opts behaviour_opts configuration_opts
	lint_actions="--setup-lab --remove-lab --check --check-part --tags
	--tags-from-file --ftp-master-rejects --dont-check-part --unpack
	--remove"
	general_opts="--help --version --print-version --verbose --debug --quiet"
	behaviour_opts="--info --display-info --display-experimental --pedantic
	--display-level --suppress-tags --suppress-tags-from-file --no-override
	--show-overrides --color --unpack-info --md5sums --checksums
	--allow-root --fail-on-warnings --keep-lab"
	configuration_opts="--cfg --lab --archivedir --dist --area --section --arch
	--root"
	if [[ $prev == -* ]]; then
	case $prev in
	-C | --check-part | -X | --dont-check-part)
	_comp_cmd_lintian__checks
	;;
	-T | --tags | --suppress-tags)
	_comp_cmd_lintian__tags
	;;
	--tags-from-file | --suppress-tags-from-file | --cfg | -p | \
	--packages-file)
	_filedir
	;;
	--lab | --archivedir | --dist | --root)
	_filedir -d
	;;
	--color)
	COMPREPLY=($(compgen -W "never always auto html" -- "$cur"))
	;;
	-U | --unpack-info)
	_comp_cmd_lintian__infos
	;;
	--area | --section)
	COMPREPLY=($(compgen -W "main contrib non-free" -- "$cur"))
	;;
	--arch) ;;
	esac
	fi
	case "$cur" in
	--*)
	COMPREPLY=($(compgen -W "$lint_actions $general_opts
	$behaviour_opts $configuration_opts" -- "$cur"))
	;;
	*,)
	# If we're here, the user is trying to complete on
	# --action tag,tag,<TAB>
	# Only few actions permit that, re-complete them now.
	case "$prev" in
	-C | --check-part | -X | --dont-check-part)
	_comp_cmd_lintian__checks
	;;
	-T | --tags | --suppress-tags)
	_comp_cmd_lintian__tags
	;;
	-U | --unpack-info)
	_comp_cmd_lintian__infos
	;;
	esac
	;;
	*)
	# in Ubuntu, dbgsym packages end in .ddeb, lintian >= 2.57.0 groks
	_filedir '@(?(u|d)deb|changes|dsc|buildinfo)'
	;;
	esac
	return 0
	} &&
	complete -F _comp_cmd_lintian lintian

Even when completions are generated in if [[ $prev == -* ]]; then, it goes through the case "$cur" in block. For example, _filedir would be called twice for lintian -p [TAB]. Maybe

diff --git a/completions/lintian b/completions/lintian
index 0769fb4b..2175c720 100644
--- a/completions/lintian
+++ b/completions/lintian
@@ -89,28 +89,36 @@ _comp_cmd_lintian()
         case $prev in
             -C | --check-part | -X | --dont-check-part)
                 _comp_cmd_lintian__checks
+                return
                 ;;
             -T | --tags | --suppress-tags)
                 _comp_cmd_lintian__tags
+                return
                 ;;
             --tags-from-file | --suppress-tags-from-file | --cfg | -p | \
                 --packages-file)
                 _filedir
+                return
                 ;;
             --lab | --archivedir | --dist | --root)
                 _filedir -d
+                return
                 ;;
             --color)
                 COMPREPLY=($(compgen -W "never always auto html" -- "$cur"))
+                return
                 ;;
             -U | --unpack-info)
                 _comp_cmd_lintian__infos
+                return
                 ;;
             --area | --section)
                 COMPREPLY=($(compgen -W "main contrib non-free" -- "$cur"))
+                return
+                ;;
+            --arch)
+                return
                 ;;
-            --arch) ;;
-
         esac
     fi

[akinomyoga]

The function _comp_cmd_python is also suspicious.

bash-completion/completions/python

Lines 64 to 74 in a26a2a9

	!(?(*/)python*([0-9.])|?(*/)pypy*([0-9.])|-?))
	[[ $cword -lt 2 || ${words[cword - 2]} != -[QWX] ]] && _filedir
	;;
	esac
	# if -c or -m is already given, complete all kind of files.
	if [[ ${words[*]::cword} == *\ -[cm]\ * ]]; then
	_filedir
	elif [[ $cur != -* ]]; then
	_filedir '@(py?([cowz])|zip)'
	else

Case !(?(*/)python*([0-9.])|?(*/)pypy*([0-9.])|-?) fall backs to the general case, then _filedir might be called multiple times. But I'm not sure the intent of this code, so maybe this is intended.

[akinomyoga]

This branch of _comp_cmd_smartctl is also suspicious.

bash-completion/completions/smartctl

Lines 55 to 57 in a26a2a9

	--quietmode | -${noargopts}q)
	COMPREPLY=($(compgen -W 'errorsonly silent noserial' -- "$cur"))
	;;

[akinomyoga]

It is subtle whether to make _comp_xfunc_ssh_identityfile replace existing elements of COMPREPLY or append to them. The calls in the bash_completion codebase seem to be in the context where COMPREPLY is empty, so we can make the function replace existing elements, but possible external completions that use _comp_xfunc_ssh_identityfile might assume it to append the completions. Now I'm letting it append to existing elements.

bash-completion/completions/ssh

Lines 251 to 261 in a26a2a9

	# With $1 set, look for public key files, else private
	# shellcheck disable=SC2120
	_comp_xfunc_ssh_identityfile()
	{
	[[ ! $cur && -d ~/.ssh ]] && cur=~/.ssh/id
	_filedir
	if ((${#COMPREPLY[@]} > 0)); then
	COMPREPLY=($(compgen -W '"${COMPREPLY[@]}"' \
	-X "${1:+!}*.pub" -- "$cur"))
	fi
	}

[scop]

Great stuff. Left a few comments inline in response to separate comments, pre-approved with them addressed the way you see best.

[akinomyoga]

Added partial reverts 83d6ea1 3d6cbfc

[akinomyoga]

Partly squashed