Re: Default completion bug

[Chet Ramey]

On 1/11/12 3:31 AM, Sung Pae wrote:
> Hello,
> 
> There is a small bug in the default completion system due to a
> disagreement between a few functions in what is considered a command
> delimiter. I have tested this on the current devel HEAD in the git repo
> (d00a2d6).

I think you've uncovered a potential problem, but not the one you think.
The important thing is that `>' is not a command separator and cannot be
a command name.  As such, it kind of slips between the cracks of the
completion system.

> 
> Define a default completion:
> 
>     _completion_loader() {
>         complete -W "COMPLETE ME" "$1" && return 124
>     }
> 
>     complete -F _completion_loader -D
> 
> Try to trigger completion for ">" (^I is literal tab):
> 
>     $ > ^I^I
>     $ complete -p
>     complete -F _completion_loader -D
>     complete -W 'COMPLETE ME' >
> 
> Check to make sure things are working:
> 
>     $ ls ^I^I
>     COMPLETE  ME
>     $ complete -p
>     complete -F _completion_loader -D
>     complete -W 'COMPLETE ME' >
>     complete -W 'COMPLETE ME' ls
> 
> Works just fine for "ls". Now, set a COMPSPEC for the empty string, "":
> 
>     $ complete -W "EMPTY STRING" ""
> 
> And try to complete ">" again:
> 
>     $ > ^I^I
>     EMPTY   STRING
>     $ complete -p
>     complete -W 'EMPTY STRING'
>     complete -F _completion_loader -D
>     complete -W 'COMPLETE ME' >
>     complete -W 'COMPLETE ME' ls
> 
> It works this time, but only because it is loading the COMPSPEC for the
> empty string, instead of for ">".
> 
> The reason for this is that gen_progcomp_completions() first searches
> the prog_completes table with the results from find_cmd_name() [1],
> which given a command line "> ", returns "" (the empty string).

That's because `>' is not part of a command name.  It *ends* a command
name.  It's a shell metacharacter, and unquoted metacharacters separate
words.

> 
> Failing this, it will call the default completion function
> _completion_loader(), and pass it a word list composed from the results
> of command_line_to_word_list(), which when given the _same_ command line
> "> ", returns the list [">", ""].

Yes, those are separate words.

> Our default completion function then faithfully inserts a COMPSPEC into
> prog_completes for the key ">" and returns 124 to restart the completion
> loop. However, since the prog_completes table is never looked up with
> the key ">", it is never actually found.

True, the completion system will allow you to define a compspec for
something that will never be used as a command name.

> Of course, once a COMPSPEC for the empty string is inserted into the
> table (which can also be done by trying to complete an empty command
> line), completing ">" works as if one was completing the empty string,
> which will most likely be assigned the same dynamic completion, thus
> having the unfortunate appearance of suddenly working as intended.

Completing `>' *is* the same as completing the empty string, because `>'
is not a command name, or part of a command name.  If you think about
how the shell parser operates, a sequence of characters like "foo>bar"
is three words: the command name `foo', the operator `>' and the word
`bar'.  Removing `foo' doesn't change the meaning of `>' and `bar'.

> 
> I have not attached a patch to this bug report, because I know
> the subtle differences of rl_basic_word_break_characters,
> COMMAND_SEPARATORS, and the value embedded in find_cmd_name(), are
> important, and that any changes, for better or worse, are scrupulously
> recorded for ALL TIME. I have implemented a couple of solutions, but who
> am _I_ to say what is a command delimiter?

Well, Posix says what is and what is not a command delimiter.  Let's see
if I can explain what these do a little better.

Readline has its own idea about completion: it breaks the command line
into units on which the user might find it convenient to perform
completion.  That's why characters like `:' and `@' are in readline's word
break characters when they are not shell metacharacters.

find_cmd_start and find_cmd_end are intended to find the boundaries of the
command containing the word to be completed.  That means find_cmd_start
uses characters after which it might be reasonable to find a command name.
That's why `{' appears in the value of COMMAND_SEPARATORS even though it's
not a shell operator.

find_cmd_end is where we might have a potential problem.  Like _start, it
should include characters that can reasonably end a command, so it should
include `)' instead of `('.  It all works out ok in the end, since `(' is
a character that starts the next command instead of one that ends the
current one, but there's potential improvement there.

find_cmd_name is different.  It uses word boundaries, not command
boundaries.  That's why it uses the shell break characters as delimiters
(look at syntax.h for the value of shell_break_characters), which are
those characters that delimit tokens when the shell is parsing input.

> So while a proper fix is complicated, there is a simple workaround
> that can be implemented in userland: simply define the completion for
> the empty string ("") for the keys that slip between the cracks of the
> different command delimiter lists (i.e. "<", ">", ">>"):

While we can talk about whether or not it's reasonable to extend the
completion system to incorporate redirections and other shell operators,
there's no bug in the way things currently work.

Chet
-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
                 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/