Re: PATCH: fix complist interactive mode overwriting buffer
Mikael Magnusson <[email protected]>
| Newsgroups | gmane.comp.shells.zsh.devel |
|---|---|
| Message-ID | <CAHYJk3T4yZafOyDHKSkvm5enRXRaEmX72u3B8qTup44a6D8DAQ@mail.gmail.com> |
I don't normally use interactive menu selection, but I played around
with it a bit more and realized the accept-and-hold path needs a
similar fix. This one needs to restore minfo.cur afterwards because it
does some other stuff afterwards that dereference it. That's not
needed on the paths in the previous patch.
diff --git a/Src/Zle/complist.c b/Src/Zle/complist.c
index f2462c259e..1ee3b74de2 100644
--- a/Src/Zle/complist.c
+++ b/Src/Zle/complist.c
@@ -2823,8 +2823,12 @@ domenuselect(Hookdef dummy, Chdata dat)
Menustack s = (Menustack) zhalloc(sizeof(*s));
int ol;
- if (mode == MM_INTER)
- do_single(*minfo.cur);
+ if (mode == MM_INTER) {
+ Cmatch *cur = minfo.cur;
+ minfo.cur = NULL;
+ do_single(*cur);
+ minfo.cur = cur;
+ }
mode = 0;
s->prev = u;
u = s;
And then I also noticed another problem,
% : <cursor here>; foobar
start interactive menu selection, type doc/z/ and hit tab, then select
one of the files, you end up with
% : Doc/Zsh/arith.yo<cursor here>[suffix space]h/; foobar
And it's fixed by this little change:
diff --git a/Src/Zle/complist.c b/Src/Zle/complist.c
index 1ee3b74de2..f4b4df6c21 100644
--- a/Src/Zle/complist.c
+++ b/Src/Zle/complist.c
@@ -3284,6 +3284,7 @@ domenuselect(Hookdef dummy, Chdata dat)
strncpy(zlemetaline, origline, origll);
zlemetacs = origcs;
minfo.len = modelen;
+ we = wb + modelen;
} else {
mode = 0;
comprecursive = 1;
If someone who uses interactive menu selection a lot feels like
testing these out, that would be helpful.
On Sat, May 23, 2026 at 8:37 AM Mikael Magnusson <[email protected]> wrote:
>
> This has been discussed a few times starting with 49242, and continued
> at 50412, 50422, 52312 and 52657.
>
> reproduce recipe:
> % zmodload zsh/complist
> % bindkey '^I' menu-select
> % MENUMODE=interactive
> % touch test{1,2}
> % : ; foobar
> ^ 1. Type the above line in its entirety.
> 2. Place the cursor before the ;
> 3. Press Tab.
> 4. Press Enter.
> % : test1 bar
> ^ Completion is written over existing buffer contents.
>
> The main confusing thing to note is that do_single looks at minfo.cur,
> whether you pass it or not, because minfo is a global variable. When it
> is not NULL, it uses minfo.pos and minfo.len which are stale in these
> particular situations. When it is instead NULL, it recalculates them
> correctly for us, which is what we want.
>
> The two different hunks are separate code paths for accepting the
> completion or navigating to another match.
> ---
> Src/Zle/complist.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/Src/Zle/complist.c b/Src/Zle/complist.c
> index 6a46497954..f2462c259e 100644
> --- a/Src/Zle/complist.c
> +++ b/Src/Zle/complist.c
> @@ -2640,6 +2640,7 @@ domenuselect(Hookdef dummy, Chdata dat)
> }
> do_last_key = 0;
>
> + int was_inter = (mode == MM_INTER);
> if (!cmd || cmd == Th(z_sendbreak)) {
> zbeep();
> molbeg = -1;
> @@ -3407,6 +3408,8 @@ domenuselect(Hookdef dummy, Chdata dat)
> acc = 1;
> break;
> }
> + if (was_inter)
> + minfo.cur = NULL;
> do_single(**p);
> mselect = (**p)->gnum;
> }
> @@ -3423,7 +3426,12 @@ domenuselect(Hookdef dummy, Chdata dat)
> clearlist = listshown = 1;
> if (acc && validlist && minfo.cur) {
> menucmp = lastambig = hasoldlist = 0;
> - do_single(*(minfo.cur));
> + if (mode == MM_INTER) {
> + Cmatch *cur = minfo.cur;
> + minfo.cur = NULL;
> + do_single(*cur);
> + } else
> + do_single(*(minfo.cur));
> }
> if (wasnext || broken) {
> menucmp = 1;
> --
> 2.38.1
>
--
Mikael Magnusson