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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.