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

au_exists bug, was Re: Test Existence of Autocommand?

Expand Messages
  • Bob Hiestand
    ... Here s a probably better patch, and a test case to show the behavior I expect from exists() with respect to autocommands: diff --git a/src/fileio.c
    Message 1 of 6 , Aug 31, 2009
    • 0 Attachment
      On Tue, Aug 25, 2009 at 11:38 AM, Ben Fritz<fritzophrenic@...> wrote:
      >> On Fri, Aug 14, 2009 at 6:38 PM, Edward Beach<beach9...@...> wrote:
      >> > I'm trying to write a script that will toggle an autocommand on/off
      >> > and I'm looking for a function to return it's existence.  I've found
      >> > the built-in exists function and from the help it gives me this:
      >> >                        #group#event    autocommand defined for this group and event.
      >> > So from my command line I enter in the following
      >> >        :aug TestGroup | aug END | echo exists("#TestGroup#BufWritePost")
      >> > and it comes back 1 -- but shouldn't it be 0 since I haven't created a
      >> > BufWritePost event and since
      >> >        :au TestGroup
      >> > returns nothing?
      >>
      >> It looks like the #group#event code doesn't check for an autocommand
      >> for that event being defined *within* the group, just whether both the
      >> group and the autocommand exist.  The #group#event#pattern does check
      >> for an autocommand defined within the specified group, though, so that
      >> may be helpful to you.

      > Since this is a patch for some VERY unexpected behavior, I'm cc'ing
      > vim_dev where such things are normally discussed.
      >
      > My opinion is that this is a bug, if it can be reliably reproduced. I
      > can't imagine any situation where the described behavior would be
      > helpful.

      Here's a probably better patch, and a test case to show the behavior I
      expect from exists() with respect to autocommands:

      diff --git a/src/fileio.c b/src/fileio.c
      index a9552b5..5da1a96 100644
      --- a/src/fileio.c
      +++ b/src/fileio.c
      @@ -9498,15 +9498,10 @@ au_exists(arg)
      ap = first_autopat[(int)event];
      if (ap == NULL)
      goto theend;
      - if (pattern == NULL)
      - {
      - retval = TRUE;
      - goto theend;
      - }

      /* if pattern is "<buffer>", special handling is needed which
      uses curbuf */
      /* for pattern "<buffer=N>, fnamecmp() will work fine */
      - if (STRICMP(pattern, "<buffer>") == 0)
      + if (pattern != NULL && STRICMP(pattern, "<buffer>") == 0)
      buflocal_buf = curbuf;

      /* Check if there is an autocommand with the given pattern. */
      @@ -9515,9 +9510,10 @@ au_exists(arg)
      /* For buffer-local autocommands, fnamecmp() works fine. */
      if (ap->pat != NULL && ap->cmds != NULL
      && (group == AUGROUP_ALL || ap->group == group)
      - && (buflocal_buf == NULL
      - ? fnamecmp(ap->pat, pattern) == 0
      - : ap->buflocal_nr == buflocal_buf->b_fnum))
      + && (pattern == NULL
      + || (buflocal_buf == NULL
      + ? fnamecmp(ap->pat, pattern) == 0
      + : ap->buflocal_nr == buflocal_buf->b_fnum)))
      {
      retval = TRUE;
      break;



      unpatched testcase results:

      $ /usr/bin/vim -u ~/testcase.vim
      BufEnter is a valid event (should be true) : 1
      BufEnter defined generally (should be false) : 0
      BufEnter defined generally (should be true) : 1
      BufEnter defined in 'auexists' group (should be false) : 1
      BufEnter defined in 'auexists' group (should be true) : 1
      BufEnter defined for *.test (should be false) : 0
      BufEnter defined for *.test (should be true) : 1
      BufEnter defined for local buffer (should be false) : 0
      BufEnter defined for local buffer (should be true) : 1


      patched testcase results:

      $ vim -u ~/testcase.vim
      BufEnter is a valid event (should be true) : 1
      BufEnter defined generally (should be false) : 0
      BufEnter defined generally (should be true) : 1
      BufEnter defined in 'auexists' group (should be false) : 0
      BufEnter defined in 'auexists' group (should be true) : 1
      BufEnter defined for *.test (should be false) : 0
      BufEnter defined for *.test (should be true) : 1
      BufEnter defined for local buffer (should be false) : 0
      BufEnter defined for local buffer (should be true) : 1

      testcase.vim:

      augroup auexists
      augroup END
      echomsg "BufEnter is a valid event (should be true) : " exists("##BufEnter")
      echomsg "BufEnter defined generally (should be false) : " exists("#BufEnter")
      au BufEnter * let g:entered=1
      echomsg "BufEnter defined generally (should be true) : " exists("#BufEnter")
      echomsg "BufEnter defined in 'auexists' group (should be false) : "
      exists("#auexists#BufEnter")
      augroup auexists
      au BufEnter * let g:entered=1
      augroup END
      echomsg "BufEnter defined in 'auexists' group (should be true) : "
      exists("#auexists#BufEnter")
      echomsg "BufEnter defined for *.test (should be false) : "
      exists("#BufEnter#*.test")
      au BufEnter *.test let g:entered=1
      echomsg "BufEnter defined for *.test (should be true) : "
      exists("#BufEnter#*.test")
      silent edit testfile.test
      echomsg "BufEnter defined for local buffer (should be false) : "
      exists("#BufEnter#<buffer>")
      au BufEnter <buffer> let g:entered=1
      echomsg "BufEnter defined for local buffer (should be true) : "
      exists("#BufEnter#<buffer>")


      Thank you,

      bob

      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_dev" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---
    • Bram Moolenaar
      ... Thanks for the patch and testcase script. Would you be able to turn the script into a test for src/testdir? That will greatly help avoid regressions. --
      Message 2 of 6 , Sep 3, 2009
      • 0 Attachment
        Bob Hiestand wrote:

        > On Tue, Aug 25, 2009 at 11:38 AM, Ben Fritz<fritzophrenic@...> wrote:
        > >> On Fri, Aug 14, 2009 at 6:38 PM, Edward Beach<beach9...@...> wrote:
        > >> > I'm trying to write a script that will toggle an autocommand on/off
        > >> > and I'm looking for a function to return it's existence. I've found
        > >> > the built-in exists function and from the help it gives me this:
        > >> > #group#event autocommand defined for this group and event.
        > >> > So from my command line I enter in the following
        > >> > :aug TestGroup | aug END | echo exists("#TestGroup#BufWritePost")
        > >> > and it comes back 1 -- but shouldn't it be 0 since I haven't created a
        > >> > BufWritePost event and since
        > >> > :au TestGroup
        > >> > returns nothing?
        > >>
        > >> It looks like the #group#event code doesn't check for an autocommand
        > >> for that event being defined *within* the group, just whether both the
        > >> group and the autocommand exist. The #group#event#pattern does check
        > >> for an autocommand defined within the specified group, though, so that
        > >> may be helpful to you.
        >
        > > Since this is a patch for some VERY unexpected behavior, I'm cc'ing
        > > vim_dev where such things are normally discussed.
        > >
        > > My opinion is that this is a bug, if it can be reliably reproduced. I
        > > can't imagine any situation where the described behavior would be
        > > helpful.
        >
        > Here's a probably better patch, and a test case to show the behavior I
        > expect from exists() with respect to autocommands:

        Thanks for the patch and testcase script. Would you be able to turn the
        script into a test for src/testdir? That will greatly help avoid
        regressions.

        --
        hundred-and-one symptoms of being an internet addict:
        178. You look for an icon to double-click to open your bedroom window.

        /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
        /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
        \\\ download, build and distribute -- http://www.A-A-P.org ///
        \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

        --~--~---------~--~----~------------~-------~--~----~
        You received this message from the "vim_dev" maillist.
        For more information, visit http://www.vim.org/maillist.php
        -~----------~----~----~----~------~----~------~--~---
      • Bob Hiestand
        ... Sure, something like the following? diff --git a/src/testdir/Makefile b/src/testdir/Makefile index 50386ec..8f9fd13 100644 ... +++ b/src/testdir/Makefile
        Message 3 of 6 , Sep 3, 2009
        • 0 Attachment
          On Thu, Sep 3, 2009 at 6:13 AM, Bram Moolenaar<Bram@...> wrote:

          >> Here's a probably better patch, and a test case to show the behavior I
          >> expect from exists() with respect to autocommands:
          >
          > Thanks for the patch and testcase script.  Would you be able to turn the
          > script into a test for src/testdir?  That will greatly help avoid
          > regressions.

          Sure, something like the following?

          diff --git a/src/testdir/Makefile b/src/testdir/Makefile
          index 50386ec..8f9fd13 100644
          --- a/src/testdir/Makefile
          +++ b/src/testdir/Makefile
          @@ -22,7 +22,7 @@ SCRIPTS = test1.out test2.out test3.out test4.out
          test5.out test6.out \
          test48.out test49.out test51.out test52.out test53.out \
          test54.out test55.out test56.out test57.out test58.out \
          test59.out test60.out test61.out test62.out test63.out \
          - test64.out test65.out test66.out
          + test64.out test65.out test66.out test67.out

          SCRIPTS_GUI = test16.out

          diff --git a/src/testdir/test67.in b/src/testdir/test67.in
          new file mode 100644
          index 0000000..46221ea
          --- /dev/null
          +++ b/src/testdir/test67.in
          @@ -0,0 +1,32 @@
          +Test that groups and patterns are tested correctly when calling
          exists() for autocommands.
          +
          +STARTTEST
          +:so small.vim
          +:let results=[]
          +:augroup auexists
          +:augroup END
          +:call add(results, exists('##BufEnter'))
          +:call add(results, exists("#BufEnter"))
          +:au BufEnter * let g:entered=1
          +:call add(results, exists("#BufEnter"))
          +:call add(results, exists("#auexists#BufEnter"))
          +:augroup auexists
          +:au BufEnter * let g:entered=1
          +:augroup END
          +:call add(results, exists("#auexists#BufEnter"))
          +:call add(results, exists("#BufEnter#*.test"))
          +:au BufEnter *.test let g:entered=1
          +:call add(results, exists("#BufEnter#*.test"))
          +:edit testfile.test
          +:call add(results, exists("#BufEnter#<buffer>"))
          +:au BufEnter <buffer> let g:entered=1
          +:call add(results, exists("#BufEnter#<buffer>"))
          +:edit testfile2.test
          +:call add(results, exists("#BufEnter#<buffer>"))
          +:e test.out
          +:call append(0, results)
          +:$d
          +:w
          +:qa!
          +ENDTEST
          +
          diff --git a/src/testdir/test67.ok b/src/testdir/test67.ok
          new file mode 100644
          index 0000000..61e9344
          --- /dev/null
          +++ b/src/testdir/test67.ok
          @@ -0,0 +1,10 @@
          +1
          +0
          +1
          +0
          +1
          +0
          +1
          +0
          +1
          +0

          --~--~---------~--~----~------------~-------~--~----~
          You received this message from the "vim_dev" maillist.
          For more information, visit http://www.vim.org/maillist.php
          -~----------~----~----~----~------~----~------~--~---
        • Bram Moolenaar
          ... Looks good. To make it a bit easier to locate any exists() that doesn t do what was ... -- I m in shape. Round IS a shape. /// Bram Moolenaar --
          Message 4 of 6 , Sep 6, 2009
          • 0 Attachment
            Bob Hiestand wrote:

            > On Thu, Sep 3, 2009 at 6:13 AM, Bram Moolenaar<Bram@...> wrote:
            >
            > >> Here's a probably better patch, and a test case to show the behavior I
            > >> expect from exists() with respect to autocommands:
            > >
            > > Thanks for the patch and testcase script. Would you be able to turn the
            > > script into a test for src/testdir? That will greatly help avoid
            > > regressions.
            >
            > Sure, something like the following?

            Looks good.

            To make it a bit easier to locate any exists() that doesn't do what was
            expected, can you make it:

            :call add(results, '##BufEnter: ' + exists('##BufEnter'))


            --
            I'm in shape. Round IS a shape.

            /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
            /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
            \\\ download, build and distribute -- http://www.A-A-P.org ///
            \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

            --~--~---------~--~----~------------~-------~--~----~
            You received this message from the "vim_dev" maillist.
            For more information, visit http://www.vim.org/maillist.php
            -~----------~----~----~----~------~----~------~--~---
          • Bob Hiestand
            ... Sure, a revised patch is attached. Thank you, bob --~--~---------~--~----~------------~-------~--~----~ You received this message from the vim_dev
            Message 5 of 6 , Sep 13, 2009
            • 0 Attachment
              On Sun, Sep 6, 2009 at 12:11 PM, Bram Moolenaar <Bram@...> wrote:

              Bob Hiestand wrote:

              > On Thu, Sep 3, 2009 at 6:13 AM, Bram Moolenaar<Bram@...> wrote:
              >
              > >> Here's a probably better patch, and a test case to show the behavior I
              > >> expect from exists() with respect to autocommands:
              > >
              > > Thanks for the patch and testcase script.  Would you be able to turn the
              > > script into a test for src/testdir?  That will greatly help avoid
              > > regressions.
              >
              > Sure, something like the following?

              Looks good.

              To make it a bit easier to locate any exists() that doesn't do what was
              expected, can you make it:

               :call add(results, '##BufEnter: ' + exists('##BufEnter'))


              Sure, a revised patch is attached.

              Thank you,

              bob


              --~--~---------~--~----~------------~-------~--~----~
              You received this message from the "vim_dev" maillist.
              For more information, visit http://www.vim.org/maillist.php
              -~----------~----~----~----~------~----~------~--~---

            • Bram Moolenaar
              ... I had already done this myself, since it was simple. It is in patch 7.2.259. Please have a look and let me know if something can be improved. -- Marriage
              Message 6 of 6 , Sep 15, 2009
              • 0 Attachment
                Bot Hiestand wrote:

                > > > On Thu, Sep 3, 2009 at 6:13 AM, Bram Moolenaar<Bram@...>
                > > wrote:
                > > >
                > > > >> Here's a probably better patch, and a test case to show the behavior I
                > > > >> expect from exists() with respect to autocommands:
                > > > >
                > > > > Thanks for the patch and testcase script. Would you be able to turn
                > > the
                > > > > script into a test for src/testdir? That will greatly help avoid
                > > > > regressions.
                > > >
                > > > Sure, something like the following?
                > >
                > > Looks good.
                > >
                > > To make it a bit easier to locate any exists() that doesn't do what was
                > > expected, can you make it:
                > >
                > > :call add(results, '##BufEnter: ' + exists('##BufEnter'))
                > >
                > >
                > Sure, a revised patch is attached.

                I had already done this myself, since it was simple. It is in patch
                7.2.259. Please have a look and let me know if something can be
                improved.

                --
                Marriage isn't a word. It's a sentence.

                /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
                /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
                \\\ download, build and distribute -- http://www.A-A-P.org ///
                \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

                --~--~---------~--~----~------------~-------~--~----~
                You received this message from the "vim_dev" maillist.
                For more information, visit http://www.vim.org/maillist.php
                -~----------~----~----~----~------~----~------~--~---
              Your message has been successfully submitted and would be delivered to recipients shortly.