-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 22-May-2014 13:32, Bram Moolenaar wrote:
>
> Ingo Karkat wrote:
>
>> Hello Vim developers,
>>
>> I just noticed a regression when using a custom command's
>> complete function that is file-based. When I do :echo
>> glob('dir/*/'), I expect the result to only include directories,
>> because of the trailing "/" path separator (same when using "\"
>> btw.), but in recent Windows builds, it also includes *all
>> files*. For example, from the Vim repository root dir, I get:
>>
>> ,----[ bad ]---- | src\arabic.c\ | src\arabic.h\ | src\ascii.h\ |
>> src\auto\ | src\bigvim.bat\ | ... `----
>>
>> instead of
>>
>> ,----[ good ]---- | src\auto\ | ... `----
>>
>> With this command:
>>
>> src\vim.exe -N -u NONE -c "if len(glob('src/*/',0,1)) > 20 |
>> cquit | else | quit | endif"
>>
>> I've bisected this problem to the following patch:
>>
>> ,----[ bad change ]---- | 7.3.1182 'backupcopy' default on
>> MS-Windows does not work for links `----
>>
>> I still see this in the latest 7.4.295 (HUGE build, using
>> Make_mvc.mak with the Windows SDK 7.1 compiler) on Windows/x64.
>> No problems on Linux.
>
> Can you do some debugging to find out what part of that patch
> causes this? Perhaps one of the ways to expand a file name removes
> the trailing slash? Considering the output above, it looks like
> the slash is added back afterwards, also after normal files.I've finally found the problem and a possible fix. The root cause is that patch 7.3.1182 replaces the implementation of mch_getperm() in os_win32.c that uses the native GetFileAttributes() with a call to mch_stat(). That one (in vim_stat() from os_mswin.c) is documented to remove a trailing path separator. dos_expandpath() just checks for a successful stat() call, so it now accepts files just as well as directories. When reverting mch_getperm() to the previous implementation, the problem is gone, but I guess the change was done for a reason :-) While attempting to add filtering for actual directories, I happened upon a related comment "On Solaris stat() accepts "file/" as if it was "file"." in macros.h, and it appeared to me that applying the illegal_slash() function (adapted for Windows by also checking for a backslash) to the mch_stat() macro is the correct solution. Attached is a patch and a test for the problem. - -- regards, ingo - -- -- Ingo Karkat -- /^-- /^-- /^-- /^-- /^-- http://ingo-karkat.de/ -- -- http://vim.sourceforge.net/account/profile.php?user_id=9713 -- -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.17 (MingW32) iQEcBAEBAgAGBQJUmEK+AAoJEA7ziXlAzQ/vyZoH/iwBFvwCMHGD+xWvppCTT/Se UaDBTYUQtmjFK3rIG/Dhw10YosBbMpw3RDzb/QZplPPn4DNIKBzlBgSdejbT5Iv4 tz9nwSe4irr6RCB1zlLJ4zMx0P4E84MAonoIAYUPgcWww6KbZk/PNOxEFVYbws89 Dn/4KB9H6GfSGN80SNg63y2xvxkCBpIMY72LEoYsINfeU9HJyLC3efGXLGfjB6Fc q3Su/hR4DDZ0eWKqjON3p0P1GdifPiX21sNf9pgJOD46TQngtRcDJYY2yzzH1OZM Vg6ei/NT0qMIK0tJiHsExuUHnb8D1Rtfleovc9gJynqYnw9OdTJZy6ro6mT0K5Y= =jSNK -----END PGP SIGNATURE----- -- -- 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
diff --git a/src/macros.h b/src/macros.h
--- a/src/macros.h
+++ b/src/macros.h
@@ -188,7 +188,9 @@
# endif
# define mch_fstat(n, p) fstat((n), (p))
# ifdef MSWIN /* has it's own mch_stat() function */
-# define mch_stat(n, p) vim_stat((n), (p))
+ /* The Windows vim_stat() accepts "file/" as if it was "file". Return -1 if
+ * the name ends in "/" and it's not a directory. */
+# define mch_stat(n, p) (illegal_slash(n) ? -1 : vim_stat((n), (p)))
# else
# ifdef STAT_IGNORES_SLASH
/* On Solaris stat() accepts "file/" as if it was "file". Return -1 if
diff --git a/src/misc2.c b/src/misc2.c
--- a/src/misc2.c
+++ b/src/misc2.c
@@ -3367,7 +3367,7 @@ vim_chdirfile(fname)
}
#endif
-#if defined(STAT_IGNORES_SLASH) || defined(PROTO)
+#if defined(STAT_IGNORES_SLASH) || defined(MSWIN) || defined(PROTO)
/*
* Check if "name" ends in a slash and is not a directory.
* Used for systems where stat() ignores a trailing slash on a file name.
@@ -3379,7 +3379,11 @@ illegal_slash(name)
{
if (name[0] == NUL)
return FALSE; /* no file name is not illegal */
+#if defined(MSWIN)
+ if (name[strlen(name) - 1] != '/' && name[strlen(name) - 1] != '\\')
+#else
if (name[strlen(name) - 1] != '/')
+#endif
return FALSE; /* no trailing slash */
if (mch_isdir((char_u *)name))
return FALSE; /* trailing slash for a directory */
diff --git a/src/Makefile b/src/Makefile
--- a/src/Makefile
+++ b/src/Makefile
@@ -1898,6 +1898,7 @@ test1 \
test_close_count \
test_command_count \
test_eval \
+ test_glob_directories \
test_insertcount \
test_listlbr \
test_listlbr_utf8 \
diff --git a/src/testdir/Make_amiga.mak b/src/testdir/Make_amiga.mak
--- a/src/testdir/Make_amiga.mak
+++ b/src/testdir/Make_amiga.mak
@@ -43,6 +43,7 @@ SCRIPTS = test1.out test3.out test4.out
test_close_count.out \
test_command_count.out \
test_eval.out \
+ test_glob_directories.out \
test_insertcount.out \
test_listlbr.out \
test_listlbr_utf8.out \
@@ -182,6 +183,7 @@ test_changelist.out: test_changelist.in
test_close_count.out: test_close_count.in
test_command_count.out: test_command_count.in
test_eval.out: test_eval.in
+test_glob_directories.out: test_glob_directories.in
test_insertcount.out: test_insertcount.in
test_listlbr.out: test_listlbr.in
test_listlbr_utf8.out: test_listlbr_utf8.in
diff --git a/src/testdir/Make_dos.mak b/src/testdir/Make_dos.mak
--- a/src/testdir/Make_dos.mak
+++ b/src/testdir/Make_dos.mak
@@ -42,6 +42,7 @@ SCRIPTS = test3.out test4.out test5.out
test_close_count.out \
test_command_count.out \
test_eval.out \
+ test_glob_directories.out \
test_insertcount.out \
test_listlbr.out \
test_listlbr_utf8.out \
diff --git a/src/testdir/Make_ming.mak b/src/testdir/Make_ming.mak
--- a/src/testdir/Make_ming.mak
+++ b/src/testdir/Make_ming.mak
@@ -64,6 +64,7 @@ SCRIPTS = test3.out test4.out test5.out
test_close_count.out \
test_command_count.out \
test_eval.out \
+ test_glob_directories.out \
test_insertcount.out \
test_listlbr.out \
test_listlbr_utf8.out \
diff --git a/src/testdir/Make_os2.mak b/src/testdir/Make_os2.mak
--- a/src/testdir/Make_os2.mak
+++ b/src/testdir/Make_os2.mak
@@ -44,6 +44,7 @@ SCRIPTS = test1.out test3.out test4.out
test_close_count.out \
test_command_count.out \
test_eval.out \
+ test_glob_directories.out \
test_insertcount.out \
test_listlbr.out \
test_listlbr_utf8.out \
diff --git a/src/testdir/Make_vms.mms b/src/testdir/Make_vms.mms
--- a/src/testdir/Make_vms.mms
+++ b/src/testdir/Make_vms.mms
@@ -103,6 +103,7 @@ SCRIPT = test1.out test2.out test3.out
test_close_count.out \
test_command_count.out \
test_eval.out \
+ test_glob_directories.out \
test_insertcount.out \
test_listlbr.out \
test_listlbr_utf8.out \
diff --git a/src/testdir/Makefile b/src/testdir/Makefile
--- a/src/testdir/Makefile
+++ b/src/testdir/Makefile
@@ -40,6 +40,7 @@ SCRIPTS = test1.out test2.out test3.out
test_close_count.out \
test_command_count.out \
test_eval.out \
+ test_glob_directories.out \
test_insertcount.out \
test_listlbr.out \
test_listlbr_utf8.out \
diff --git a/src/testdir/test_glob_directories.in b/src/testdir/test_glob_directories.in
new file mode 100644
--- /dev/null
+++ b/src/testdir/test_glob_directories.in
@@ -0,0 +1,19 @@
+Test globbing of only directories by appending a trailing path separator
+
+STARTTEST
+:so small.vim
+:e! test.out
+:let cwd=getcwd()
+:cd ..
+:" Tests may be run from a shadow directory, so an extra cd needs to be done to
+:" get above src/
+:if fnamemodify(getcwd(), ':t') != 'src' | cd ../.. | else | cd .. | endif
+:%delete _
+:" Only src/testdir/ should match this glob, but there are files that match the pattern without the trailing slash, too.
+:$put =len(glob('src/*st*/', 0, 1))
+:$put =len(glob('src/*st*', 0, 1)) > 1
+:exe "cd" cwd
+:w
+:qa!
+ENDTEST
+
diff --git a/src/testdir/test_glob_directories.ok b/src/testdir/test_glob_directories.ok
new file mode 100644
--- /dev/null
+++ b/src/testdir/test_glob_directories.ok
@@ -0,0 +1,3 @@
+
+1
+1
glob-dir-extra-files.patch.sig
Description: Binary data
glob-dir-extra-files-test.patch.sig
Description: Binary data
