`_command_offset` allows completions on bash built-ins

[scop]

`_command_offset` allows completions on bash built-ins

[scop]

I'm not sure I understand what you mean, could you elaborate with some specific examples? If you mean e.g. that hitting TAB after e.g. ccache should not offer read among possible "wrapped" commands to further complete, then I must say that first, while on first thought it seems to make sense, I'm not 100% certain that a case where that would be useful doesn't exist. And second, I don't know how to sanely filter out builtins from list of commands to complete. (Nor am I at all sure what you mean by "filtering out -b".)

[rrthomas]

Your interpretation is correct; I mean that for example typing ccache rea and pressing TAB gives me ccache read, though there is no binary that can be wrapped like that.

The completion files that currently use _command_offset are all ones that wrap commands like this, so I think the onus is on finding an example where completing built-ins like this is useful, rather than finding one where it isn't! (If it were found to be useful in some cases, an option could be added to include built-ins).

My assumption (which perhaps is mistaken) is that the builtins were included in the list of commands to complete by passing the -b option to compgen, so if _command_offset were to filter out this option, then it would not complete built-ins. _command_offset already fiddles with the options, so this (it seemed to me) should not be hard to achieve.

The motivation for this was the following exchange:

brummer10/nosuspend@e5eb83d#commitcomment-19712085

where someone decided to write a bash completion without using _command_offset because of this bug. That seemed to me unfortunate.

[scop]

I don't see where _command_offset would be currently passing -b to compgen.

[rrthomas]

It gets the current options from complete -p, stores them in cspec, and later passes them on. It already treats -F specially.

[scop]

Sure, but are there any commands that actually have -b in their completion options so that complete -p on them would end up returning it? ccache or sudo don't, haven't checked others.

$ complete -p ccache
complete -F _ccache ccache
$ complete -p sudo
complete -F _sudo sudo

[rrthomas]

OK, so why is it completing built-ins then? Is this just the default, as when starting with an empty line?

[scop]

The code branch where -F is examined is already completing on the secondary command, too late to do anything there. The secondary command names to offer as completions are retrieved in an earlier code branch, using compgen -d -c. And I think -c just includes all commands, including builtins, without any further options we could use to tell it to omit builtins.

[rrthomas]

The method used by the code I link to is:

executables=$({ compgen -c; compgen -abkA function; } | sort | uniq -u)

i.e. it finds all the commands which are not executables, and then only takes the executables, which appear only once in the list (i.e. they are not builtins, functions etc.).

[rrthomas]

@scop, you've replied to my two bug reports, but you've not responded further to my updated PR#86. I wondered whether this was an oversight? Feel free to delete this comment once you've read it!

[rrthomas]

@brummer10 comments:

A much faster version to create a list of (only) the available executables will be

executables=$(echo -n "$PATH" | xargs -d: -I{} -r -- find -L {} -maxdepth 1 -mindepth 1 -type f -executable -printf '%P\n' 2>/dev/null | sort -u)

He quotes timings at the above-linked bug report.

[scop]

No oversights, I'm just very very low on time to spend on bash-completion these days.

Just a first impression on the latter implementation of grabbing the executables; it uses several arguments to find which aren't portable (at least -maxdepth, -mindepth, -executable, -printf don't exist in POSIX find), and I'm not sure we want to depend on xargs in the first place, or if its usage here is portable. Search for POSIX in CONTRIBUTING.md for more info.

[rrthomas]

No worries, I just wondered because you'd replied to my issues, but not to the PR.

Since the faster version is non-portable, I refer you to the previous, portable version.