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

Re: PATCH: Completion for find and sfind arguments (todo.txt)

Expand Messages
  • Nazri Ramliy
    ... I ve thought about this too. On linux, hard links is not an issue since it is only limited to files (on my linux, at least, I m assuming this is true for
    Message 1 of 18 , Feb 26, 2009
      On Thu, Feb 26, 2009 at 11:49 PM, Ben Fritz <fritzophrenic@...> wrote:
      >> Attached patch makes it skip symbolic links to directory on unix platform.
      >> I don't know how to do the same for other platforms (amiga?/windows?),
      >> sorry :)
      >>
      >
      > Well...Windows doesn't have symbolic links. But, it does have the
      > possibility of hard links. Are those handled, too, on Unix?

      I've thought about this too.

      On linux, hard links is not an issue since it is only limited to files
      (on my linux, at least, I'm assuming this is true for all UNIX). It's
      links to directories that may be problematic due to possible cycle.

      > Maybe you just need some sort of limit on the depth of the search,
      > possibly controlled by an option.

      Limiting the depth is not the solution [1], IMHO, as the cycle may be
      present at any depth levels.

      nazri

      [1] It's already done in {unix,dos}_expandpath functions in misc1.c. The
      hard limit is 100.

      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_dev" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---
    • Nazri Ramliy
      ... Thanks, Craig. IIRC on windows platform symbolic link support comes with the NTFS filesystem so it s been there since NT. Attached patch (untested!) skips
      Message 2 of 18 , Feb 26, 2009
        On Fri, Feb 27, 2009 at 3:00 AM, Craig Barkhouse <craigba@...> wrote:
        > Windows definitely does have symbolic links, starting with Vista.
        >  (See: mklink /?)  There are also junctions, which are like symbolic
        > links but only for directories, starting with Win2K.
        >
        > Both of these are examples of reparse points.  To detect if a file or
        > directory is a reparse point, call GetFileAttributes or
        > GetFileAttributesEx, and see if the FILE_ATTRIBUTE_REPARSE_POINT flag
        > is set.

        Thanks, Craig.

        IIRC on windows platform symbolic link support comes with the NTFS
        filesystem so it's been there since NT.

        Attached patch (untested!) skips symbolic links to directory when
        completing filenames in the 'path' setting to avoid cycle on windows
        platform.

        If anyone could verify that it works fine on windows too that would be
        super.

        Test setup:

        1. Apply the patch to vim (svn r1390) and compile
        2. Create this directory hierarchy: c:\a\b\c\d
        3. Create a file in the d directory, say c:\a\b\c\d\file.txt
        4. Create a link c:\a\b\d that links to its parent directory (..)
        5. vim -u NONE
        :set path=c:/a/**
        :find f<C-e><C-e><C-e>

        That is hit ctrl+e to trigger the expansion multiple times. It
        should complete to file.txt ONLY (the correct behavior).

        Now test that without this patch it breaks:
        6. vim src/misc.c, search for nextfile, and comment out the three lines
        starting from "if (flags & EW_NOSYMLINK..." test for triggering the
        'goto nextfile;' (in dos_expandpath function).
        7. Compile and repeat step 5 and things should go haywire

        nazri.

        --~--~---------~--~----~------------~-------~--~----~
        You received this message from the "vim_dev" maillist.
        For more information, visit http://www.vim.org/maillist.php
        -~----------~----~----~----~------~----~------~--~---
      • Ben Fritz
        ... Well, then...I stand corrected. I ve never worked with Vista so I can be excused on that count. Hard link creation was difficult enough to find, but I had
        Message 3 of 18 , Feb 27, 2009
          On Feb 26, 1:00 pm, Craig Barkhouse <crai...@...> wrote:

          > Windows definitely does have symbolic links, starting with Vista.  (See: mklink /?)  There are also junctions, which are like symbolic links but only for directories, starting with Win2K.
          >

          Well, then...I stand corrected.

          I've never worked with Vista so I can be excused on that count. Hard
          link creation was difficult enough to find, but I had no idea about
          the existence of junctions (probably because Microsoft provides no
          utilities to create/work with them). Thanks for the enlightenment!

          Time to go try out some nifty junction creation.
          --~--~---------~--~----~------------~-------~--~----~
          You received this message from the "vim_dev" maillist.
          For more information, visit http://www.vim.org/maillist.php
          -~----------~----~----~----~------~----~------~--~---
        • Bram Moolenaar
          ... Cycles in symbolic links are rare. The code should be able to handle this, in a way that it doesn t drop all results that contain a symbolic link. Thus
          Message 4 of 18 , Mar 4, 2009
            Nazri Ramliy wrote:

            > On Fri, Feb 20, 2009 at 9:22 PM, Nazri Ramliy <ayiehere@...> wrote:
            > > There's one major outstanding issue that I'm aware of - it does not
            > > skip symbolic links. This may result in infinite cycle if there exists
            > > a symbolic link that creates a cycle in the file system.
            >
            > Attached patch makes it skip symbolic links to directory on unix platform.
            > I don't know how to do the same for other platforms (amiga?/windows?),
            > sorry :)

            Cycles in symbolic links are rare. The code should be able to handle
            this, in a way that it doesn't drop all results that contain a symbolic
            link. Thus symbolic links should be followed, unless getting to a
            directory that was previously visited.

            > > I don't have access to a windows platform so I can't test the patch on
            > > windows. It may have problems with those different drive letters.
            >
            > Upon further thoughts (on my limited brain capacity :), I think that it won't
            > have any problem when doing the expansion and uniquefying of the files
            > in the 'path' option.
            >
            > For example with 'path' is set to C:/**,D:/**, doing ":find file1<tab>",
            > matching files like
            >
            > C:\a\file1.txt and
            > D:\a\file1.txt
            >
            > would be listed using their full path names in the wildmenu listing while
            > files like
            >
            > C:\project1\a\file1.txt and
            > D:\project2\a\file2.txt
            >
            > would be shortened to
            >
            > project1\a\file1.txt and
            > project2\a\file2.txt
            >
            > If anybody would verify that it works fine on windows platform (and/or any
            > other platform for that matter), that would be nice.
            >
            > I've sort of stress-tested this feature by setting my path to /** (the
            > whole root
            > directory, up to the default 30 directories deep), and then searching for some
            > known file using part of its filename. I did an strace on the vim process (to
            > see the chdir system calls that it made) and also looked at its memory usage
            > in top during the whole searching process. All seemed to be fine.

            Anyway, the patch adds a lot of code. I'm wondering if we can reuse
            more of the existing code. How about using globpath()? Or use
            find_file_in_path(), like ex_find() does (also accepting directories
            though). The last one also takes care of removing duplicates.

            If your code is to be used, the chdir() calls are not safe. Look at the
            mch_FullName() function in os_unix.c, lots of remarks about this.
            Yet another reason to use as much of the existing code, that has been
            debugged for a long time, instead of introducing new code which new
            problems.

            Also, the functions potentially take a lot of time, so they should check
            for CTRL-C now and then. That means calling ui_breakcheck() and
            checking the got_int flag.

            --
            "Hit any key to continue" does _not_ mean you can hit the on/off button!

            /// 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
            -~----------~----~----~----~------~----~------~--~---
          • Nazri Ramliy
            ... My initial plan was to do that but I dropped it since I thought that it might incur more processing. Seems like hash_* in hashtab.c might help here. ... I
            Message 5 of 18 , Mar 5, 2009
              On Thu, Mar 5, 2009 at 11:07 AM, Bram Moolenaar <Bram@...> wrote:
              > Cycles in symbolic links are rare.  The code should be able to handle
              > this, in a way that it doesn't drop all results that contain a symbolic
              > link.  Thus symbolic links should be followed, unless getting to a
              > directory that was previously visited.

              My initial plan was to do that but I dropped it since I thought that it
              might incur more processing. Seems like hash_* in hashtab.c might help
              here.

              > Anyway, the patch adds a lot of code.  I'm wondering if we can reuse
              > more of the existing code.  How about using globpath()?  Or use
              > find_file_in_path(), like ex_find() does (also accepting directories
              > though).  The last one also takes care of removing duplicates.
              >
              > If your code is to be used, the chdir() calls are not safe.  Look at the
              > mch_FullName() function in os_unix.c, lots of remarks about this.
              > Yet another reason to use as much of the existing code, that has been
              > debugged for a long time, instead of introducing new code which new
              > problems.

              I agree with all of the above. Thanks for the hints and comments.

              > Also, the functions potentially take a lot of time, so they should check
              > for CTRL-C now and then.  That means calling ui_breakcheck() and
              > checking the got_int flag.

              Currently it checks only for the got_int flag. I'll include
              ui_breakcheck too, later.

              There's a couple more issues that I found out while using :find with the
              patch:

              1. It doesn't honor the 'wildignore' setting in cases where there
              are more than one files matching the pattern and at least one of
              them would be excluded by 'wildignore'. The current behavior is
              that it will list all the files that match the pattern whether or
              not it matches 'wildignore'.

              2. Doing :find a/*/hello.html gets uniquefied to hello.html, which
              may not be what we want as there might be other hello.html in
              'path'. The solution to this might be to uniquefy the name based
              on the pattern, not by the fullpath alone.

              I'll look into this when I got another wave of free time.

              nazri

              --~--~---------~--~----~------------~-------~--~----~
              You received this message from the "vim_dev" maillist.
              For more information, visit http://www.vim.org/maillist.php
              -~----------~----~----~----~------~----~------~--~---
            • Nazri Ramliy
              ... [snip] ... [snip] ... [snip] ... Here s the updated patch with all the above issues addressed: 1. Cycle resulting from symbolic links - I m using
              Message 6 of 18 , Mar 27, 2009
                On Fri, Mar 6, 2009 at 10:09 AM, Nazri Ramliy <ayiehere@...> wrote:
                > On Thu, Mar 5, 2009 at 11:07 AM, Bram Moolenaar <Bram@...> wrote:
                >> Cycles in symbolic links are rare. The code should be able to handle
                >> this, in a way that it doesn't drop all results that contain a symbolic
                >> link. Thus symbolic links should be followed, unless getting to a
                >> directory that was previously visited.

                [snip]

                >> Anyway, the patch adds a lot of code. I'm wondering if we can reuse
                >> more of the existing code. How about using globpath()? Or use
                >> find_file_in_path(), like ex_find() does (also accepting directories
                >> though). The last one also takes care of removing duplicates.
                >>
                >> If your code is to be used, the chdir() calls are not safe. Look at the
                >> mch_FullName() function in os_unix.c, lots of remarks about this.
                >> Yet another reason to use as much of the existing code, that has been
                >> debugged for a long time, instead of introducing new code which new
                >> problems.

                [snip]

                >> Also, the functions potentially take a lot of time, so they should check
                >> for CTRL-C now and then. That means calling ui_breakcheck() and
                >> checking the got_int flag.

                [snip]

                > There's a couple more issues that I found out while using :find with the
                > patch:
                >
                > 1. It doesn't honor the 'wildignore' setting in cases where there
                > are more than one files matching the pattern and at least one of
                > them would be excluded by 'wildignore'. The current behavior is
                > that it will list all the files that match the pattern whether or
                > not it matches 'wildignore'.
                >
                > 2. Doing :find a/*/hello.html gets uniquefied to hello.html, which
                > may not be what we want as there might be other hello.html in
                > 'path'. The solution to this might be to uniquefy the name based
                > on the pattern, not by the fullpath alone.

                Here's the updated patch with all the above issues addressed:

                1. Cycle resulting from symbolic links - I'm using globpath() and it
                seems that this is no longer an issue anymore.

                2. No more chdir() calls.

                3. Call to ui_breakcheck() is done and got_int flag is checked
                accordingly.

                4. wildignore setting is already handled by globpath()

                5. uniquefy_paths() uniquefies and shortens the fullpaths based on the
                given pattern. Please check up on the FIXME note that I added in the
                patch.

                If anyone could test the patch on other OS that would be great. I've
                tested it on my linux and all seemed well.

                nazri

                --~--~---------~--~----~------------~-------~--~----~
                You received this message from the "vim_dev" maillist.
                For more information, visit http://www.vim.org/maillist.php
                -~----------~----~----~----~------~----~------~--~---
              • Bram Moolenaar
                ... Thanks! I ll look into it later. -- In his lifetime van Gogh painted 486 oil paintings. Oddly enough, 8975 of them are to be found in the United States.
                Message 7 of 18 , Mar 28, 2009
                  Nazri Ramliy wrote:

                  > On Fri, Mar 6, 2009 at 10:09 AM, Nazri Ramliy <ayiehere@...> wrote:
                  > > On Thu, Mar 5, 2009 at 11:07 AM, Bram Moolenaar <Bram@...> wrote:
                  > >> Cycles in symbolic links are rare. The code should be able to handle
                  > >> this, in a way that it doesn't drop all results that contain a symbolic
                  > >> link. Thus symbolic links should be followed, unless getting to a
                  > >> directory that was previously visited.
                  >
                  > [snip]
                  >
                  > >> Anyway, the patch adds a lot of code. I'm wondering if we can reuse
                  > >> more of the existing code. How about using globpath()? Or use
                  > >> find_file_in_path(), like ex_find() does (also accepting directories
                  > >> though). The last one also takes care of removing duplicates.
                  > >>
                  > >> If your code is to be used, the chdir() calls are not safe. Look at the
                  > >> mch_FullName() function in os_unix.c, lots of remarks about this.
                  > >> Yet another reason to use as much of the existing code, that has been
                  > >> debugged for a long time, instead of introducing new code which new
                  > >> problems.
                  >
                  > [snip]
                  >
                  > >> Also, the functions potentially take a lot of time, so they should check
                  > >> for CTRL-C now and then. That means calling ui_breakcheck() and
                  > >> checking the got_int flag.
                  >
                  > [snip]
                  >
                  > > There's a couple more issues that I found out while using :find with the
                  > > patch:
                  > >
                  > > 1. It doesn't honor the 'wildignore' setting in cases where there
                  > > are more than one files matching the pattern and at least one of
                  > > them would be excluded by 'wildignore'. The current behavior is
                  > > that it will list all the files that match the pattern whether or
                  > > not it matches 'wildignore'.
                  > >
                  > > 2. Doing :find a/*/hello.html gets uniquefied to hello.html, which
                  > > may not be what we want as there might be other hello.html in
                  > > 'path'. The solution to this might be to uniquefy the name based
                  > > on the pattern, not by the fullpath alone.
                  >
                  > Here's the updated patch with all the above issues addressed:
                  >
                  > 1. Cycle resulting from symbolic links - I'm using globpath() and it
                  > seems that this is no longer an issue anymore.
                  >
                  > 2. No more chdir() calls.
                  >
                  > 3. Call to ui_breakcheck() is done and got_int flag is checked
                  > accordingly.
                  >
                  > 4. wildignore setting is already handled by globpath()
                  >
                  > 5. uniquefy_paths() uniquefies and shortens the fullpaths based on the
                  > given pattern. Please check up on the FIXME note that I added in the
                  > patch.
                  >
                  > If anyone could test the patch on other OS that would be great. I've
                  > tested it on my linux and all seemed well.

                  Thanks! I'll look into it later.

                  --
                  In his lifetime van Gogh painted 486 oil paintings. Oddly enough, 8975
                  of them are to be found in the United States.

                  /// 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.