Loading ...
Sorry, an error occurred while loading the content.
 

Re: Wrong completion entry selection after backspace

Expand Messages
  • Christian Brabandt
    Hi Olivier! ... The problem is, hitting backspace changes the leader and on the next invocation of ins_complete() compl_curr_match() will be set wrongly
    Message 1 of 8 , Feb 26, 2013
      Hi Olivier!

      On Mo, 25 Feb 2013, Olivier Teulière wrote:

      > Hi Christian,
      >
      > On Saturday, February 16, 2013 10:03:05 PM UTC+1, Christian Brabandt wrote:
      > > Check this patch:
      > > [...]
      >
      > Your patch solves the problem, but maybe not in the best way. Now, after typing 'f' to shrink the list, the selection is lost in the popup. This means that I cannot simply type Enter to insert the selected entry and close the popup at the same time, which was very practical.
      >
      > I think it would make more sense to keep the selection, and to return to one of the following states after <bs>:
      > - foofoobar1 is selected (both visually and "internally"), because it was selected before typing <bs>
      > - foobar is selected (both visually and "internally"), because it was selected before typing the letter erased by <bs>
      > Whichever is easier to implement :) Currently, it seems to be in the first state visually, and in the second one internally.

      The problem is, hitting backspace changes the leader and on the next
      invocation of ins_complete() compl_curr_match() will be set wrongly
      because ins_compl_next is called.

      This patch should work better:

      diff --git a/src/edit.c b/src/edit.c
      --- a/src/edit.c
      +++ b/src/edit.c
      @@ -93,6 +93,7 @@
      static compl_T *compl_first_match = NULL;
      static compl_T *compl_curr_match = NULL;
      static compl_T *compl_shown_match = NULL;
      +static int did_bs = FALSE;

      /* After using a cursor key <Enter> selects a match in the popup menu,
      * otherwise it inserts a line break. */
      @@ -3380,6 +3381,7 @@
      if (compl_leader != NULL)
      {
      ins_compl_new_leader();
      + did_bs = TRUE;
      return NUL;
      }
      return K_BS;
      @@ -5363,7 +5365,10 @@
      * Find next match (and following matches).
      */
      save_w_wrow = curwin->w_wrow;
      - n = ins_compl_next(TRUE, ins_compl_key2count(c), ins_compl_use_match(c));
      + /* don't add completions, after hitting backspace and the leader changed */
      + if (!did_bs)
      + n = ins_compl_next(TRUE, ins_compl_key2count(c), ins_compl_use_match(c));
      + did_bs = FALSE;

      /* may undisplay the popup menu */
      ins_compl_upd_pum();


      Mit freundlichen Grüßen
      Christian
      --
      Was man erfindet, tut man mit Liebe, was man gelernt hat, mit
      Sicherheit.
      -- Goethe, Maximen und Reflektionen, Nr. 1068

      --
      --
      You received this message from the "vim_dev" maillist.
      Do not top-post! Type your reply below the text you are replying to.
      For more information, visit http://www.vim.org/maillist.php

      ---
      You received this message because you are subscribed to the Google Groups "vim_dev" group.
      To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
      For more options, visit https://groups.google.com/groups/opt_out.
    • Olivier Teuliere
      Hi Christian, Sorry, but there doesn t seem to be any difference. And there is a compilation warning: edit.c: In function ‘ins_complete’: edit.c:5377:16:
      Message 2 of 8 , Feb 26, 2013
        Hi Christian,

        Sorry, but there doesn't seem to be any difference.
        And there is a compilation warning:
        edit.c: In function ‘ins_complete’:
        edit.c:5377:16: warning: ‘n’ may be used uninitialized in this
        function [-Wmaybe-uninitialized]
        edit.c:4944:9: note: ‘n’ was declared here

        Best regards,
        Olivier

        On Tue, Feb 26, 2013 at 11:10 PM, Christian Brabandt <cblists@...> wrote:
        > Hi Olivier!
        >
        > On Mo, 25 Feb 2013, Olivier Teulière wrote:
        >
        >> Hi Christian,
        >>
        >> On Saturday, February 16, 2013 10:03:05 PM UTC+1, Christian Brabandt wrote:
        >> > Check this patch:
        >> > [...]
        >>
        >> Your patch solves the problem, but maybe not in the best way. Now, after typing 'f' to shrink the list, the selection is lost in the popup. This means that I cannot simply type Enter to insert the selected entry and close the popup at the same time, which was very practical.
        >>
        >> I think it would make more sense to keep the selection, and to return to one of the following states after <bs>:
        >> - foofoobar1 is selected (both visually and "internally"), because it was selected before typing <bs>
        >> - foobar is selected (both visually and "internally"), because it was selected before typing the letter erased by <bs>
        >> Whichever is easier to implement :) Currently, it seems to be in the first state visually, and in the second one internally.
        >
        > The problem is, hitting backspace changes the leader and on the next
        > invocation of ins_complete() compl_curr_match() will be set wrongly
        > because ins_compl_next is called.
        >
        > This patch should work better:
        >
        > diff --git a/src/edit.c b/src/edit.c
        > --- a/src/edit.c
        > +++ b/src/edit.c
        > @@ -93,6 +93,7 @@
        > static compl_T *compl_first_match = NULL;
        > static compl_T *compl_curr_match = NULL;
        > static compl_T *compl_shown_match = NULL;
        > +static int did_bs = FALSE;
        >
        > /* After using a cursor key <Enter> selects a match in the popup menu,
        > * otherwise it inserts a line break. */
        > @@ -3380,6 +3381,7 @@
        > if (compl_leader != NULL)
        > {
        > ins_compl_new_leader();
        > + did_bs = TRUE;
        > return NUL;
        > }
        > return K_BS;
        > @@ -5363,7 +5365,10 @@
        > * Find next match (and following matches).
        > */
        > save_w_wrow = curwin->w_wrow;
        > - n = ins_compl_next(TRUE, ins_compl_key2count(c), ins_compl_use_match(c));
        > + /* don't add completions, after hitting backspace and the leader changed */
        > + if (!did_bs)
        > + n = ins_compl_next(TRUE, ins_compl_key2count(c), ins_compl_use_match(c));
        > + did_bs = FALSE;
        >
        > /* may undisplay the popup menu */
        > ins_compl_upd_pum();
        >
        >
        > Mit freundlichen Grüßen
        > Christian
        > --
        > Was man erfindet, tut man mit Liebe, was man gelernt hat, mit
        > Sicherheit.
        > -- Goethe, Maximen und Reflektionen, Nr. 1068
        >
        > --
        > --
        > You received this message from the "vim_dev" maillist.
        > Do not top-post! Type your reply below the text you are replying to.
        > For more information, visit http://www.vim.org/maillist.php
        >
        > ---
        > You received this message because you are subscribed to the Google Groups "vim_dev" group.
        > To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
        > For more options, visit https://groups.google.com/groups/opt_out.
        >
        >



        --
        Olivier

        --
        --
        You received this message from the "vim_dev" maillist.
        Do not top-post! Type your reply below the text you are replying to.
        For more information, visit http://www.vim.org/maillist.php

        ---
        You received this message because you are subscribed to the Google Groups "vim_dev" group.
        To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
        For more options, visit https://groups.google.com/groups/opt_out.
      • Christian Brabandt
        Hi Olivier! ... Yes. n needs to be initialized to -1. It fixes the problem you described in your first message for me. regards, Christian -- -- You received
        Message 3 of 8 , Feb 26, 2013
          Hi Olivier!

          On Di, 26 Feb 2013, Olivier Teuliere wrote:

          > Hi Christian,
          >
          > Sorry, but there doesn't seem to be any difference.
          > And there is a compilation warning:
          > edit.c: In function ‘ins_complete’:
          > edit.c:5377:16: warning: ‘n’ may be used uninitialized in this
          > function [-Wmaybe-uninitialized]
          > edit.c:4944:9: note: ‘n’ was declared here

          Yes. n needs to be initialized to -1. It fixes the problem you described
          in your first message for me.

          regards,
          Christian

          --
          --
          You received this message from the "vim_dev" maillist.
          Do not top-post! Type your reply below the text you are replying to.
          For more information, visit http://www.vim.org/maillist.php

          ---
          You received this message because you are subscribed to the Google Groups "vim_dev" group.
          To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
          For more options, visit https://groups.google.com/groups/opt_out.
        • Olivier Teuliere
          Hi Christian, ... Initializing n doesn t fix the initial problem either. I don t know if I was supposed to revert your first patch before applying the second
          Message 4 of 8 , Feb 26, 2013
            Hi Christian,

            On Tue, Feb 26, 2013 at 11:56 PM, Christian Brabandt <cblists@...> wrote:
            > Yes. n needs to be initialized to -1. It fixes the problem you described
            > in your first message for me.

            Initializing n doesn't fix the initial problem either.
            I don't know if I was supposed to revert your first patch before
            applying the second one, but neither way works for me:
            - if I apply only the second one, the behaviour described in my first
            email is still there
            - if I apply both, it's the same as when applying only the first one:
            the selection is lost after typing the 'f'

            Did you use the latest sources from hg for your tests?

            Best regards,
            --
            Olivier

            --
            --
            You received this message from the "vim_dev" maillist.
            Do not top-post! Type your reply below the text you are replying to.
            For more information, visit http://www.vim.org/maillist.php

            ---
            You received this message because you are subscribed to the Google Groups "vim_dev" group.
            To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
            For more options, visit https://groups.google.com/groups/opt_out.
          • Christian Brabandt
            ... Hi Olivier, I used something like 7.3.840 patch yesterday night to test it and as I said, it works for me, but let me look into it again later today.
            Message 5 of 8 , Feb 27, 2013
              On Wed, February 27, 2013 08:35, Olivier Teuliere wrote:
              > Hi Christian,
              >
              > On Tue, Feb 26, 2013 at 11:56 PM, Christian Brabandt <cblists@...>
              > wrote:
              >> Yes. n needs to be initialized to -1. It fixes the problem you described
              >> in your first message for me.
              >
              > Initializing n doesn't fix the initial problem either.
              > I don't know if I was supposed to revert your first patch before
              > applying the second one, but neither way works for me:
              > - if I apply only the second one, the behaviour described in my first
              > email is still there
              > - if I apply both, it's the same as when applying only the first one:
              > the selection is lost after typing the 'f'
              >
              > Did you use the latest sources from hg for your tests?

              Hi Olivier,
              I used something like 7.3.840 patch yesterday night to test it and as
              I said, it works for me, but let me look into it again later today.

              regards,
              Christian

              --
              --
              You received this message from the "vim_dev" maillist.
              Do not top-post! Type your reply below the text you are replying to.
              For more information, visit http://www.vim.org/maillist.php

              ---
              You received this message because you are subscribed to the Google Groups "vim_dev" group.
              To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
              For more options, visit https://groups.google.com/groups/opt_out.
            Your message has been successfully submitted and would be delivered to recipients shortly.