Use _comp_split and _comp_compgen in completions/*

[scop]

Now we have a utility function _comp_split to safely split strings (including the output of the compgen builtin and other commands) without being affected by pathname expansions, unexpected word splitting, etc. [...]
The changes to completions/* are left for later PRs.

_Originally posted by @akinomyoga in #919

[akinomyoga]

Naming of functions that generate completions in COMPREPLY

Ref: Originally raised as a part of #539 (comment)

I would like to discuss the naming of a particular class of functions (including _filedir, _parse_help, and _parse_usage).

For this rewriting, we will switch from COMPREPLY=($(compgen ...)), etc. to use _comp_compgen .... One of the frequent cases among them is COMPREPLY=($(compgen -W '$(_parse_help ...)' -- "$cur")). I would like to rewrite them to use new versions of _parse_help or _parse_usage that directly assign results to COMPREPLY (or a specified array).

Then there is a related discussion that I wanted to open. Currently, there are utility functions of the name _<CATEGORY> that generate and store completions of the category in COMPREPLY, such as _filedir, _ip_addresses, and _pids. The functions _parse_help and _parse_usage also behave in the same way after the suggested change to store results in arrays. If we just follow the new naming convention, the simple prefixing of _comp works for all of them (such as _comp_filedir).

• _comp_filedir, _comp_ip_addresses, _comp_pids, ...

But I think there is also a possibility to introduce a namespace or a naming rule for this class of functions, such as

• _comp_generate_filedir, _comp_generate_ip_addresses, _comp_generate_pids, ...
• _comp_gen_filedir, _comp_gen_ip_addresses, _comp_gen_pids, ... (shorter name)
• _comp_yield_filedir, _comp_yield_ip_addresses, _comp_yield_pids, ... (typical word for generator/coroutine)
• _comp_reply_filedir, _comp_reply_ip_addresses, _comop_reply_pids, ... (inspired by the variable name COMPREPLY)
• _comp_action_filedir, _comp_action_ip_addresses, _comp_action_pids, ... (inspired by compgen -A <action>)
• _comp_compgen_filedir, _comp_compgen_ip_addresses, _comp_compgen_pids, ...
  Note: we already use this namespace as the function _comp_compgen, but the default behavior of _comp_compgen is different from existing _filedir, etc.: E.g., _comp_compgen requires an array name and doesn't filter completions by -- "$cur" by default. Maybe we could also adjust the default behavior of _comp_compgen this time.
• or other ways

What do you think?

edit: Now I'm leaning to _comp_compgen_* with the _comp_compgen interface (#951). #952 is the basic implementation, and _parse_{help,usage} are also implemented (#954) based on #952.

Interface change of _comp_compgen (#951)

edit: I'm currently thinking of changing the interface of _comp_compgen so that it behaves similarly to _filedir, _ip_addresses, _pids, etc., i.e., it stores the results to COMPREPLY by default and filters the completions by -- "${cur-}". This is because the most of the calls of _comp_compgen currently in master and in the planned PRs have the pattern _comp_compgen COMPREPLY ... -- "$cur".

# currently
_comp_compgen [-al|-F sep] ARRAY COMPGEN_ARGS... -- CUR

# suggestion (By default, ARRAY is _comp_compgen, and CUR is "${cur-}")
_comp_compgen [-al|-F sep|-c CUR|-v ARRAY] -- COMPGEN_ARGS...

[akinomyoga]

Also, PR #560 by @algorythmic is related to _parse_help, which would need to be first merged before applying the interface change of _parse_help. I have rebased #560. Could you first check #560?

[akinomyoga]

I added another PR #950, which I'd like to address before _parse_{help,usage}.

---

I'd like to separate related PRs for easier review. There are some dependencies between the planned changes.

• _parse_help-3 refactor: remaining _comp_compgen_{help,usage} changes #971
• _parse_help-2 refactor: use _comp_compgen_{help,usage} with non-trivial rewriting #969
  • _parse_help-1 refactor: _parse_{help,usage} => _comp_compgen_help #954
    • curl: fix option listing in newer versions #560
    • fix: add misc fixes before _parse_{help,usage} changes #950
    • fix(_comp_compgen): change interface #951
    • naming of functions that generate completions in COMPREPLY #948 (comment)
    • refactor: { => _comp_compgen}_filedir and _comp_compgen [opts] NAME #952
• fix: use _comp_compgen for COMPREPLY=($(compgen ...)) #955
• fix: use _comp_compgen for multiline COMPREPLY=($(compgen ...)) #970
• fix: use _comp_compgen for simple cases #972
• fix: use _comp_compgen with API changes #973
• fix: add misc fixes before _comp_compgen_split #985
• feat(_comp_compgen_split): use _comp_compgen_split for -W '$(...)' #989
  • Resolve Over-eager positional param check in _comp_compgen #958
• fix: use _comp_compgen for the remaining cases #994

[scop]

@akinomyoga what are your thoughts on the status of this? For example a quick git grep -F "COMPREPLY=(" reveals a few dozen hits I think we could look into before 2.12.

[akinomyoga]

As I have replied once in #530 (reply in thread), I wouldn't request it to be part of 2.12. However, if you like it, I'll try to look at it to see whether it is easy to apply. If it would cause again a bunch of discussions, we can probably postpone applying it not to delay the new release further.

[akinomyoga]

I have submitted PRs that replace unquoted array assignments.

• fix(ri): fix several problems and use _comp_split #1083
• fix: fix misc problems before applying _comp_split changes #1084
  • fix: replace unquoted array assignments with _comp_{split,compgen_split,expand_glob} #1085
    • fix: refactor unquoted array assignments #1086
    • fix: refactor array assignments with manual cur filtering #1087

[scop]

Removing this from the 2.12 TODO list, this is not an API issue nor essential for the release, and could take a bit of time.

[akinomyoga]

Thank you for reviewing all those issues!

I actually thought the PRs mentioned above were the last ones for replacing arr=($word) and compgen with _comp_split and _comp_compgen. The generators of the form _comp_compgen NAME were not initially intended for this issue because the generators were introduced in #952 after this issue was raised. I feel we can now close this issue. But maybe the confirmation itself will take time, so we can remove this from the ToDo list anyway.

[scop]

Thanks for the feedback, I'll close this one now per that. I'd like to be able to reduce the number of open issues and PR's to ones for which there is clear intent to work on addressing something in the foreseeable future.