-
Notifications
You must be signed in to change notification settings - Fork 400
-
The xfuncs we currently have that operate on COMPREPLY have inconsistent behavior with regards to whether they append or replace to the target. Would be good to be consistent with this.
If we are to choose between always append or replace, I think I'd go with append, as it's easier to work around at call sites if replace semantics are preferred.
Then again, I suppose we could look into providing an -a option to _comp_xfunc for controlling this, but I'm not sure if that's worth the trouble.
Beta Was this translation helpful? Give feedback.
All reactions
Replies: 2 comments 6 replies
-
I had a related idea, though I haven't thought deeply about whether it's really worth it: Maybe it is useful to provide a way to adjust the behavior through _comp_compgen also for exported functions that generate completions.
For example, when the _comp_compgen call has the following form (with a slash in 1ドル):
_comp_compgen [-aR|-v var|-c cur] CMD/NAME args...
it internally calls _comp_xfunc CMD compgen_NAME instead of _comp_compgen_NAME, where the xfunc is defined with the name _comp_xfunc_CMD_compgen_NAME.
Beta Was this translation helpful? Give feedback.
All reactions
-
In this way, one can implement _comp_xfunc_CMD_compgen_NAME using _comp_comgen and let the caller decide whether to replace or append. It replaces by default the same as the other _comp_compgen_NAME. If one wants to append the new completions, one can call it using _comp_compgen -a CMD/NAME args....
Beta Was this translation helpful? Give feedback.
All reactions
-
Sounds good to me in principle, although the CMD/NAME syntax feels a bit magical. How about moving the xfunc cmd to [-x CMD]?
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
|
So, exported generator functions have the name
Also, it might be better to make a naming rule for the internal generator functions defined in specific completion files. I'd like to make use of
|
Beta Was this translation helpful? Give feedback.
All reactions
-
2 with -i looks best to me of the options so far. Given green field, -c would look even better (-x ~ xfunc, -c ~ cmd) but it's taken for cur already. Perhaps -m ~ cMd could be worth considering, but is quite far fetched.
In a sense I like 3 too, it would kind of make it clearer that it's really for the same cmd namespace and not to be used from anywhere else, but... dunno. Calling from somewhere else breaks immediately because there's no sourcing mechanism with it, so not that much of a problem.
Beta Was this translation helpful? Give feedback.
All reactions
-
Thoughts on the above 2 vs 3 consideration? In particular, I'm wondering if the redundant CMD with -i is worth the symmetry.
Beta Was this translation helpful? Give feedback.
All reactions
-
I realized that omitting CMD can be a problem when the completion is loaded through a symbolic link, in which case BASH_SOURCE will tell the path of the symbolic link. For this reason (in addition to the symmetry), I currently favor option 2.
We might still call readlink against "${BASH_SOURCE[1]}", but the implementation would become complicated and it requires spawning a process.
- First,
readlinkis not standardized in POSIX. For this problem, we can extract the link target from the result ofls -l <file>instead of relying onreadlink. - Even if
readlinkis available, we might need to resolve recursive links sincereadlink -forrealpathis not universally available. That said, we may not need to care about recursive links forcompletions/*. - To reduce the spawning cost, we might first check
[[ -h ${BASH_SOURCE[1]} ]]before callingls -land only callls -lwhen it is a symbolic link. For the second call of the same internal generator, we might create a delegating function_comp_cmd_LINK__compgen_NAME() { _comp_cmd_CMD__compgen_NAME "$@"; }on the fly. - Also,
BASH_SOURCE[1]can contain a relative path to the file from the working directory when the file was loaded. We can make sure to use the absolute path in__load_completion.
Beta Was this translation helpful? Give feedback.
All reactions
-
I agree, thanks for taking the time to explain 👍
Beta Was this translation helpful? Give feedback.