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

Re: Command line completion doesn't use . and ,, in path: Bug?

Expand Messages
  • Nazri Ramliy
    ... I found the problem and the solution. Problem summary: expand_path_option() leaves trailing path separators after expanding the path option. These expanded
    Message 1 of 16 , Apr 18, 2013
    • 0 Attachment
      On Tue, Apr 16, 2013 at 4:31 PM, Nazri Ramliy <ayiehere@...> wrote:
      > I can reproduce this with my gvim 7.3.46 (on linux vim 7.3.892 there's> no problem). I'll investigate and hopefully post a fix soon.

      I found the problem and the solution.

      Problem summary:

      expand_path_option() leaves trailing path separators after expanding the
      path option. These expanded paths are passed to globpath(), which then
      fed them to copy_option_part() as a comma-separated list of paths. On
      windows the presence of the trailing path separator before the comma
      causes copy_option_part() to not work as intended as it treats the
      backslash as escaping the commas.

      Solution:

      Fix expand_path_option to not leave a trailing path separators when
      expanding the 'path' option.

      Example problem:

      With 'path' set to ".,,", and editing the file c:\foo\bar.txt, from the
      directory c:\, and doing :find quu<tab>, expand_path_option() expands
      the ".,," to:

      c:\foo\,c:\

      Which is wrongly treated as a single option value by copy_option_part()

      Compare the same behavior on unix:

      when editing /foo/bar.txt and the current directory is /,
      expand_path_option() expands the ".,," to:

      /foo/,/

      This is seen as two option values by copy_option_part()

      Attached patch fixes this problem. It adds a test case that shows the
      problem - note that the problem only happen on windows, so the test
      fails on windows but ran successfully on unix.

      Nazri.

      --
      --
      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.
    • Ingo Karkat
      ... Please note that for the root directory, C: is different than C: (which means the current working directory of the C drive), so leaving off the trailing
      Message 2 of 16 , Apr 18, 2013
      • 0 Attachment
        On 18-Apr-2013 14:14 +0200, Nazri Ramliy wrote:

        > On Tue, Apr 16, 2013 at 4:31 PM, Nazri Ramliy <ayiehere@...> wrote:
        >> I can reproduce this with my gvim 7.3.46 (on linux vim 7.3.892 there's> no problem). I'll investigate and hopefully post a fix soon.
        >
        > I found the problem and the solution.
        >
        > Problem summary:
        >
        > expand_path_option() leaves trailing path separators after expanding the
        > path option. These expanded paths are passed to globpath(), which then
        > fed them to copy_option_part() as a comma-separated list of paths. On
        > windows the presence of the trailing path separator before the comma
        > causes copy_option_part() to not work as intended as it treats the
        > backslash as escaping the commas.

        > Solution:
        >
        > Fix expand_path_option to not leave a trailing path separators when
        > expanding the 'path' option.

        Please note that for the root directory, C:\ is different than C: (which
        means the current working directory of the C drive), so leaving off the
        trailing path separator may cause problems in this case. (I haven't
        checked your patch, but :echo globpath('C:', '*') returns different
        results than :echo globpath('C:\', '*') after :cd C:\windows.

        -- regards, ingo

        > Example problem:
        >
        > With 'path' set to ".,,", and editing the file c:\foo\bar.txt, from the
        > directory c:\, and doing :find quu<tab>, expand_path_option() expands
        > the ".,," to:
        >
        > c:\foo\,c:\
        >
        > Which is wrongly treated as a single option value by copy_option_part()
        >
        > Compare the same behavior on unix:
        >
        > when editing /foo/bar.txt and the current directory is /,
        > expand_path_option() expands the ".,," to:
        >
        > /foo/,/
        >
        > This is seen as two option values by copy_option_part()
        >
        > Attached patch fixes this problem. It adds a test case that shows the
        > problem - note that the problem only happen on windows, so the test
        > fails on windows but ran successfully on unix.
        >
        > Nazri.


        --
        --
        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
        ... That is interesting, because the echo globpath( C: , * ) result is rather useless here: ,---- ... `---- Note the missing slash. And C: here stands for
        Message 3 of 16 , Apr 18, 2013
        • 0 Attachment
          On Thu, April 18, 2013 15:06, Ingo Karkat wrote:
          > Please note that for the root directory, C:\ is different than C: (which
          > means the current working directory of the C drive), so leaving off the
          > trailing path separator may cause problems in this case. (I haven't
          > checked your patch, but :echo globpath('C:', '*') returns different
          > results than :echo globpath('C:\', '*') after :cd C:\windows.
          >

          That is interesting, because the echo globpath('C:', '*') result is
          rather useless here:

          ,----
          | C:111.txt
          | C:stats-sql.txt
          | C:test.txt
          `----

          Note the missing slash. And C: here stands for c:\temp, so the output
          is rather useless in this case.

          This looks like another bug here.

          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.
        • Ingo Karkat
          ... No, this is just the unusual but valid notation. Without the backslash after the drive letter, this means this path from the current working ...
          Message 4 of 16 , Apr 18, 2013
          • 0 Attachment
            On 18-Apr-2013 15:15 +0200, Christian Brabandt wrote:

            > On Thu, April 18, 2013 15:06, Ingo Karkat wrote:
            >> Please note that for the root directory, C:\ is different than C: (which
            >> means the current working directory of the C drive), so leaving off the
            >> trailing path separator may cause problems in this case. (I haven't
            >> checked your patch, but :echo globpath('C:', '*') returns different
            >> results than :echo globpath('C:\', '*') after :cd C:\windows.
            >>
            >
            > That is interesting, because the echo globpath('C:', '*') result is
            > rather useless here:
            >
            > ,----
            > | C:111.txt
            > | C:stats-sql.txt
            > | C:test.txt
            > `----
            >
            > Note the missing slash. And C: here stands for c:\temp, so the output
            > is rather useless in this case.
            >
            > This looks like another bug here.

            No, this is just the unusual but valid notation. Without the backslash
            after the drive letter, this means "this path from the current working
            directory of the preceding drive letter". Therefore, this works:

            :cd C:\Windows
            :edit C:win.ini
            " Existing file opens.
            :echo expand('%:p')
            C:\Windows\win.ini

            -- regards, ingo

            --
            --
            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
            ... I d still prefer to have the glob return an absolute path an not such a silly name. You can t even copy and paste that string. Would it hurt to have
            Message 5 of 16 , Apr 18, 2013
            • 0 Attachment
              On Thu, April 18, 2013 15:32, Ingo Karkat wrote:
              > On 18-Apr-2013 15:15 +0200, Christian Brabandt wrote:
              >
              >> On Thu, April 18, 2013 15:06, Ingo Karkat wrote:
              >>> Please note that for the root directory, C:\ is different than C:
              >>> (which
              >>> means the current working directory of the C drive), so leaving off the
              >>> trailing path separator may cause problems in this case. (I haven't
              >>> checked your patch, but :echo globpath('C:', '*') returns different
              >>> results than :echo globpath('C:\', '*') after :cd C:\windows.
              >>>
              >>
              >> That is interesting, because the echo globpath('C:', '*') result is
              >> rather useless here:
              >>
              >> ,----
              >> | C:111.txt
              >> | C:stats-sql.txt
              >> | C:test.txt
              >> `----
              >>
              >> Note the missing slash. And C: here stands for c:\temp, so the output
              >> is rather useless in this case.
              >>
              >> This looks like another bug here.
              >
              > No, this is just the unusual but valid notation. Without the backslash
              > after the drive letter, this means "this path from the current working
              > directory of the preceding drive letter". Therefore, this works:
              >
              > :cd C:\Windows
              > :edit C:win.ini
              > " Existing file opens.
              > :echo expand('%:p')
              > C:\Windows\win.ini
              >

              I'd still prefer to have the glob return an absolute path an not such
              a silly name. You can't even copy and paste that string.
              Would it hurt to have globpath() in that case return an
              absolute path?

              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.
            • Ingo Karkat
              ... It s not silly, you re just not used to it! (I started with CP/M and MS-DOS 5; maybe I have a different perspective.) It s a fixed-drive-relative-path; you
              Message 6 of 16 , Apr 18, 2013
              • 0 Attachment
                On 18-Apr-2013 15:15 +0200, Christian Brabandt wrote:

                > On Thu, April 18, 2013 15:32, Ingo Karkat wrote:
                >> On 18-Apr-2013 15:15 +0200, Christian Brabandt wrote:
                >>
                >>> On Thu, April 18, 2013 15:06, Ingo Karkat wrote:
                >>>> Please note that for the root directory, C:\ is different than C:
                >>>> (which
                >>>> means the current working directory of the C drive), so leaving off the
                >>>> trailing path separator may cause problems in this case. (I haven't
                >>>> checked your patch, but :echo globpath('C:', '*') returns different
                >>>> results than :echo globpath('C:\', '*') after :cd C:\windows.
                >>>>
                >>>
                >>> That is interesting, because the echo globpath('C:', '*') result is
                >>> rather useless here:
                >>>
                >>> ,----
                >>> | C:111.txt
                >>> | C:stats-sql.txt
                >>> | C:test.txt
                >>> `----
                >>>
                >>> Note the missing slash. And C: here stands for c:\temp, so the output
                >>> is rather useless in this case.
                >>>
                >>> This looks like another bug here.
                >>
                >> No, this is just the unusual but valid notation. Without the backslash
                >> after the drive letter, this means "this path from the current working
                >> directory of the preceding drive letter". Therefore, this works:
                >>
                >> :cd C:\Windows
                >> :edit C:win.ini
                >> " Existing file opens.
                >> :echo expand('%:p')
                >> C:\Windows\win.ini
                >>
                >
                > I'd still prefer to have the glob return an absolute path an not such
                > a silly name. You can't even copy and paste that string.
                > Would it hurt to have globpath() in that case return an
                > absolute path?

                It's not silly, you're just not used to it! (I started with CP/M and
                MS-DOS 5; maybe I have a different perspective.) It's a
                fixed-drive-relative-path; you wouldn't expect ../../foo to be
                copy&paste-able, neither, would you?

                globpath() does not return absolute paths for other more benign relative
                paths such as '.', so I'm against adding a special case. Sorry. Anyway,
                I'm more interested whether the proposed patch has a problem...

                -- regards, ingo

                --
                --
                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
                ... That is the case. I started with MS-DOS 4.X and have never come across that. But ok, may be it is just my expectation. regards, Christian -- -- You
                Message 7 of 16 , Apr 18, 2013
                • 0 Attachment
                  On Thu, April 18, 2013 15:45, Ingo Karkat wrote:
                  > It's not silly, you're just not used to it! (I started with CP/M and
                  > MS-DOS 5; maybe I have a different perspective.)

                  That is the case. I started with MS-DOS 4.X and have never come across
                  that. But ok, may be it is just my expectation.

                  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.
                • Nazri Ramliy
                  ... Thanks for the notice. I ll look into this again. My initial approach at solving this was to replace all backslashes in the expanded path parts into
                  Message 8 of 16 , Apr 18, 2013
                  • 0 Attachment
                    On Thu, Apr 18, 2013 at 9:06 PM, Ingo Karkat <swdev@...> wrote:
                    > Please note that for the root directory, C:\ is different than C: (which
                    > means the current working directory of the C drive), so leaving off the
                    > trailing path separator may cause problems in this case. (I haven't
                    > checked your patch, but :echo globpath('C:', '*') returns different
                    > results than :echo globpath('C:\', '*') after :cd C:\windows.

                    Thanks for the notice. I'll look into this again. My initial approach at
                    solving this was to replace all backslashes in the expanded path parts
                    into forward slashes but that seemed a bit too blankety of a solution
                    at that time.

                    Maybe it's not a bad idea after all.

                    nazri

                    --
                    --
                    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.
                  • Nazri Ramliy
                    ... Here s another attempt. This time we check if the expanded path ends with a path separator then we replace it with forward slash, and we do this only on
                    Message 9 of 16 , Apr 18, 2013
                    • 0 Attachment
                      On Thu, Apr 18, 2013 at 10:11 PM, Nazri Ramliy <ayiehere@...> wrote:
                      > Thanks for the notice. I'll look into this again. My initial approach at
                      > solving this was to replace all backslashes in the expanded path parts
                      > into forward slashes but that seemed a bit too blankety of a solution
                      > at that time.
                      >
                      > Maybe it's not a bad idea after all.

                      Here's another attempt. This time we check if the expanded path ends
                      with a path separator then we replace it with forward slash, and we do
                      this only on windows/msdos.

                      Updated patch attached.

                      nazri

                      --
                      --
                      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.
                    • Suresh Govindachar
                      ... I tried the patch to src/misc1.c (but not the patches for the automated test) and it appears to have fixed the issue. To recap the issue, here are the
                      Message 10 of 16 , Apr 21, 2013
                      • 0 Attachment
                        On 4/18/2013 7:50 AM, Nazri Ramliy wrote:
                        > Here's another attempt. This time we check if the expanded path ends
                        > with a path separator then we replace it with forward slash, and we do
                        > this only on windows/msdos.
                        >
                        > Updated patch attached.

                        I tried the patch to src/misc1.c (but not the patches for the automated test) and it appears to have fixed the issue. To recap the issue, here are the steps to test the issue manually:

                        On windows:
                        cd <to some_place>
                        mkdir -p foo/opt/vim
                        touch foo.txt
                        touch foo/foo.txt
                        touch foo/food.txt
                        touch foo/opt/vim/finger.txt
                        touch foo/opt/vim/fish.txt

                        start gvim -u NONE -U NONE
                        Commands inside gvim:

                        :pwd
                        "preceding will show <some_place>

                        :set cp wildchar=<Tab> wildmode=list:longest,full

                        "leave 'path' at default value of ".,,"

                        :find foo\opt\vim\fish.txt

                        " all of the above is set-up; now for the test

                        :find foo<hit the tab key>

                        Without the patch, hitting the tab key does nothing. With the patch,
                        hitting the tab key will find foo.txt and foo\

                        --Suresh

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