Re: rl_complete(0, '?') should never insert into the line buffer

[Pip Cet]

On Thu, May 23, 2024 at 8:41 PM Pip Cet <pipcet@gmail.com> wrote:
>
> On Thu, May 23, 2024 at 3:48 PM Chet Ramey <chet.ramey@case.edu> wrote:
> Please forgive me for reordering your email a little, but I think it's
> best if we start with this:
>
> > The appropriate fix is to separate the behavior of
> > `possible-completions' and consecutive invocations of `complete' so that
> > using M-? doesn't result in a completion being inserted.
> > That will be
> > in the next version of bash/readline, and it's in the respective devel
> > git branches now. I'm not sure that's going to help your particular use
> > case, though.
>
> If M-? is fixed never to insert text, that addresses this bug. There
> appears to be a separate and very recent readline bug which makes TAB
> after a failed completion list completions rather than insert a unique
> completion, but I hadn't even noticed that one; it doesn't appear with
> my system version of readline, and it doesn't appear with the devel
> branch of readline, but it does appear with the master branch of
> readline.
>
> I think you're saying both the bug I reported and the separate bug
> introduced in readline fairly recently will be fixed? That's great! If
> that's true, please feel free to disregard the rest of this email.
>
> > On 5/22/24 8:03 AM, Pip Cet wrote:
> > > x x BACKSPACE x x
> > >
> > > with automatic completions before and after the BACKSPACE, will result
> > > in the line buffer showing "xyz xx". The expected output is "xxx".
> > >
> > > It's this code in complete.c:
> > >
> > >      case '?':
> > >        /* Let's try to insert a single match here if the last completion 
> > > failed
> > >       but this attempt returned a single match. */
> > >        if (saved_last_completion_failed && matches[0] && *matches[0] &&
> > > matches[1] == 0)
> > >      {
> > >        insert_match (matches[0], start, matches[1] ? MULT_MATCH :
> > > SINGLE_MATCH, &quote_char);
> > >        append_to_match (matches[0], delimiter, quote_char, 
> > > nontrivial_lcd);
> > >        break;
> > >      }
> > >
> > > The code in question seems extremely questionable to me: '?' is meant
> > > to LIST completions (non-destructively), not insert a completion.
> > > Again, with most applications, it's valid and useful to continue
> > > typing in something that doesn't match a completion.
> >
> > Consider the case that this is designed to address:
>
> Can you expand that example a little? I think there's a
> misunderstanding here somewhere, and I strongly suspect it's on my
> part.
>
> Here are the two interpretations I have of what you describe, using
> bash and default key bindings:
>
> 1. user is in a directory with no x* files.
> 2. user enters c a t > x TAB
> 3. no completions appear
> 4. user goes to another window and creates "xyz"
> 5. user enters TAB again
> 6. The command line now reads "cat>xyz"
>
> That interpretation works with and without the code in question, using
> my system version of readline and bash. It appears to fail with the
> current master branch of readline, but that's a separate bug entirely.
>
> The other one is:
>
> 1. user is in a directory with no x* files.
> 2. user enters c a t > x TAB
> 3. no completions appear
> 4. user goes to another window and creates "xyz"
> 5. user enters M-?
> 6. The command line now reads "cat>xyz"
>
> > They then go off to another window and fix whatever the problem was, and
> > come back and try completion again.
>
> Except they don't "try completion": they invoke a command specifically
> documented to LIST available completions, not insert them at point.
> Using default key bindings, they made the choice not to use the TAB
> key, but to hit M-? instead.
>
> > Since the last readline command was
> > attempted completion,
> > the default behavior is to list possible completions,
>
> the behavior of TAB or M-? ? The code I referenced does not depend in
> any way on what the last readline command was, it just checks the last
> completion command, which may have been a while ago, failed.
>
> Consider this:
>
> 0. you're in a fresh shell (this is important)
> 1. you're in a directory containing a single file called "xyz"
> 2. you enter c a t SPACE a b c RET
> 3. the file doesn't exist and an error message is printed
> 4. you enter c a t SPACE x M-?
> 5. you see a single completion listed, "xyz"
> 6. the command line still reads "cat x"
>
> versus this:
>
> 1. you're in a directory containing a single file called "xyz"
> 2. you enter c a t SPACE a b c TAB RET
> 3. the file doesn't exist and an error message is printed
> 4. you enter c a t SPACE x M-?
> 5. you see no completions
> 6. the command line reads "cat abc xyz"
>
> So to know what M-? does, the user has to remember the result of the
> last attempted completion, no matter how long ago that was, and how
> many intervening completion-free commands were executed.
>
> > but in this case, listing a single completion that the user knows is there
> > is pretty hostile.
>
> I struggle to follow this. Maybe it's the default keybindings
> confusing me, but I assume most users who have M-? rebound to TAB
> specifically decided they don't want TAB to insert a completion, ever.
>
> > It's better to just insert the single completion, since
> > there is one, and go on.
>
> It may lead to the production of a command line which would remove or
> modify a file you never intended to, which is why you used M-?, not
> TAB. I fail to see how that is better, in general.
>
> > > Let's just remove this special case: it's undocumented, violates the
> > > distinction between text-modifying and non-destructive calls of
> > > rl_complete, it's useless for applications where non-completed words
> > > are still valid input, and it complicates things.
> >
> > No, it's valuable.
>
> You're saying it's valuable for M-? to insert completions at point?
> That's the behavior I want to remove.
>
> I did briefly try the devel version of readline, but it didn't work at
> all for my application, most likely because I'm doing something wrong,
> so I couldn't check whether the bug still existed there.

After investigating, I've installed the attached patch
(readline-001.diff), and it allows me to run my application with the
"devel" branch of readline. It's not the proper fix, obviously, but
maybe it helps explain the problem I'm running into.

And I can run bash linked against it, which does indeed reproduce the
"hostile" behavior you described above. Here's what I'm doing:

1. enter a directory with two files, xya and xyb
2. enter c a t SPACE x y TAB
3. in another window, remove the file "xyb"
4. enter TAB

I would expect the command line now to read "cat xyb", but it's still
"cat xy", and the single completion is listed.

I've attached a second patch (readline-002.diff) which fixes this
particular case and makes rl_complete_internal behave as documented,
but may very well break other applications. Maybe it's better to swap
'?' and '2' in the patch?

Thanks again,
Pip

readline-002.diff
Description: Text Data

readline-001.diff
Description: Text Data