On Fri, Mar 6, 2009 at 10:09 AM, Nazri Ramliy <[email protected]> wrote:
> On Thu, Mar 5, 2009 at 11:07 AM, Bram Moolenaar <[email protected]> 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
-~----------~----~----~----~------~----~------~--~---
Index: ex_docmd.c
===================================================================
--- ex_docmd.c (revision 1425)
+++ ex_docmd.c (working copy)
@@ -3422,6 +3422,11 @@
*/
switch (ea.cmdidx)
{
+ case CMD_find:
+ case CMD_sfind:
+ case CMD_tabfind:
+ xp->xp_context = EXPAND_FILES_IN_PATH;
+ break;
case CMD_cd:
case CMD_chdir:
case CMD_lcd:
Index: ex_getln.c
===================================================================
--- ex_getln.c (revision 1425)
+++ ex_getln.c (working copy)
@@ -4078,6 +4078,7 @@
char_u *tail;
if (context != EXPAND_FILES
+ && context != EXPAND_FILES_IN_PATH
&& context != EXPAND_SHELLCMD
&& context != EXPAND_DIRECTORIES)
{
@@ -4392,7 +4393,9 @@
if (options & WILD_SILENT)
flags |= EW_SILENT;
- if (xp->xp_context == EXPAND_FILES || xp->xp_context == EXPAND_DIRECTORIES)
+ if (xp->xp_context == EXPAND_FILES
+ || xp->xp_context == EXPAND_DIRECTORIES
+ || xp->xp_context == EXPAND_FILES_IN_PATH)
{
/*
* Expand file or directory names.
@@ -4422,6 +4425,8 @@
if (xp->xp_context == EXPAND_FILES)
flags |= EW_FILE;
+ else if (xp->xp_context == EXPAND_FILES_IN_PATH)
+ flags |= (EW_FILE | EW_PATH);
else
flags = (flags | EW_DIR) & ~EW_FILE;
ret = expand_wildcards(1, &pat, num_file, file, flags);
Index: misc1.c
===================================================================
--- misc1.c (revision 1425)
+++ misc1.c (working copy)
@@ -9101,7 +9101,200 @@
}
#endif
+#if defined(FEAT_SEARCHPATH)
/*
+ * Moves psep to the previous path separator in path, starting from the
+ * end of path. Returns FAIL is psep ends up at the beginning of path.
+ */
+ static int
+find_previous_pathsep(path, psep)
+ char_u *path;
+ char_u **psep;
+{
+ /*
+ * As we're looking for the previous path separator, skip the current
+ * separator.
+ */
+ if (vim_ispathsep(**psep))
+ (*psep)--;
+
+ while (*psep >= path && !vim_ispathsep(**psep))
+ (*psep)--;
+
+ if (*psep != path && vim_ispathsep(**psep))
+ return OK;
+
+ return FAIL;
+}
+
+/*
+ * Returns TRUE if maybe_unique is unique wrt other_paths in gap. maybe_unique
+ * is the end portion of ((char_u **)gap->ga_data)[i].
+ */
+ static int
+is_unique(maybe_unique, gap, i)
+ char_u *maybe_unique;
+ garray_T *gap;
+ int i;
+{
+ int j;
+ int candidate_len;
+ int other_path_len;
+ char_u *rival;
+ char_u **other_paths;
+
+ other_paths = (gap->ga_data != NULL) ? (char_u **)gap->ga_data : (char_u **)"";
+
+ for (j = 0; j < gap->ga_len && !got_int; j++)
+ {
+ ui_breakcheck();
+ /* Don't compare it with itself */
+ if(j == i)
+ continue;
+
+ candidate_len = STRLEN(maybe_unique);
+ other_path_len = STRLEN(other_paths[j]);
+
+ if(other_path_len < candidate_len)
+ /* It's different, */
+ continue;
+
+ rival = other_paths[j] + other_path_len - candidate_len;
+
+ if (fnamecmp(maybe_unique, rival) == 0)
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+/*
+ * Sorts, removes duplicates and modifies all the fullpath names in gap so that
+ * they are unique with respect to each other while conserving the part that
+ * matches the pattern. Beware, this is at least O(n^2) wrt gap->ga_len.
+ */
+ static void
+uniquefy_paths(gap, pattern)
+ garray_T *gap;
+ char_u *pattern;
+{
+ int i;
+ int j;
+ int len;
+ char_u *pathsep_p;
+ char_u *path;
+ char_u **fnames = (char_u **) gap->ga_data;
+
+ int sort_again = 0;
+ char_u *pat;
+ char_u *file_pattern;
+ regmatch_T regmatch;
+
+ /* Remove duplicate entries */
+ sort_strings(fnames, gap->ga_len);
+ for (i = 0; i < gap->ga_len - 1; i++)
+ if (fnamecmp(fnames[i], fnames[i+1]) == 0)
+ {
+ vim_free(fnames[i]);
+ for (j = i+1; j < gap->ga_len; j++)
+ fnames[j-1] = fnames[j];
+ gap->ga_len--;
+ i--;
+ }
+
+ /*
+ * We need to prepend a '*' at the beginning of file_pattern so that the
+ * regex matches anywhere in the path. FIXME: is this valid for all
+ * possible pattern?
+ */
+ len = STRLEN(pattern);
+ file_pattern = alloc(len + 2);
+ file_pattern[0] = '*';
+ file_pattern[1] = '\0';
+ STRCAT(file_pattern, pattern);
+ pat = file_pat_to_reg_pat(file_pattern, NULL, NULL, TRUE);
+ vim_free(file_pattern);
+ regmatch.rm_ic = TRUE; /* Always ignore case */
+
+ if (pat != NULL)
+ {
+ regmatch.regprog = vim_regcomp(pat, RE_MAGIC + RE_STRING);
+ vim_free(pat);
+ }
+ if (pat == NULL || regmatch.regprog == NULL)
+ return;
+
+ for (i = 0; i < gap->ga_len; i++)
+ {
+ path = fnames[i];
+ len = STRLEN(path);
+
+ /* We start at the end of the path */
+ pathsep_p = path + len - 1;
+
+ while (find_previous_pathsep(path, &pathsep_p))
+ if (vim_regexec(®match, pathsep_p + 1, (colnr_T)0) &&
+ is_unique(pathsep_p, gap, i))
+ {
+ sort_again = 1;
+ mch_memmove(path, pathsep_p + 1, STRLEN(pathsep_p));
+ break;
+ }
+ }
+
+ if (sort_again)
+ sort_strings(fnames, gap->ga_len);
+}
+
+/*
+ * Calls globpath with 'path' values for the given pattern and stores
+ * the result in gap. Returns the total number of match.
+ */
+ static int
+expand_in_path(gap, pattern, flags)
+ garray_T *gap;
+ char_u *pattern;
+ int flags; /* EW_* flags */
+{
+ int c = 0;
+ char_u *path_option = *curbuf->b_p_path == NUL ? p_path : curbuf->b_p_path;
+ char_u *files;
+ char_u *s; /* start */
+ char_u *e; /* end */
+
+ files = globpath(path_option, pattern, 0);
+ if (files == NULL)
+ return 0;
+
+ /* Copy each path in files into gap */
+ s = e = files;
+ while (*s != '\0')
+ {
+ while (*e != '\n' && *e != '\0')
+ e++;
+ if (*e == '\0')
+ {
+ addfile(gap, s, flags);
+ break;
+ }
+ else
+ {
+ /* *e is '\n' */
+ *e = '\0';
+ addfile(gap, s, flags);
+ e++;
+ s = e;
+ }
+ }
+
+ c = gap->ga_len;
+ vim_free(files);
+
+ return c;
+}
+#endif
+
+/*
* Generic wildcard expansion code.
*
* Characters in "pat" that should not be expanded must be preceded with a
@@ -9209,7 +9402,14 @@
* when EW_NOTFOUND is given.
*/
if (mch_has_exp_wildcard(p))
- add_pat = mch_expandpath(&ga, p, flags);
+ {
+#if defined(FEAT_SEARCHPATH)
+ if (*p != '.' && !vim_ispathsep(*p) && flags & EW_PATH)
+ add_pat = expand_in_path(&ga, p, flags);
+ else
+#endif
+ add_pat = mch_expandpath(&ga, p, flags);
+ }
}
if (add_pat == -1 || (add_pat == 0 && (flags & EW_NOTFOUND)))
@@ -9232,6 +9432,11 @@
vim_free(p);
}
+#if defined(FEAT_SEARCHPATH)
+ if (flags & EW_PATH)
+ uniquefy_paths(&ga, p);
+#endif
+
*num_file = ga.ga_len;
*file = (ga.ga_data != NULL) ? (char_u **)ga.ga_data : (char_u **)"";
Index: vim.h
===================================================================
--- vim.h (revision 1425)
+++ vim.h (working copy)
@@ -709,6 +709,7 @@
#define EXPAND_USER_LIST 31
#define EXPAND_SHELLCMD 32
#define EXPAND_CSCOPE 33
+#define EXPAND_FILES_IN_PATH 34
/* Values for exmode_active (0 is no exmode) */
#define EXMODE_NORMAL 1
@@ -740,6 +741,7 @@
#define EW_KEEPALL 0x10 /* keep all matches */
#define EW_SILENT 0x20 /* don't print "1 returned" from shell */
#define EW_EXEC 0x40 /* executable files */
+#define EW_PATH 0x80 /* search in 'path' too */
/* Note: mostly EW_NOTFOUND and EW_SILENT are mutually exclusive: EW_NOTFOUND
* is used when executing commands and EW_SILENT for interactive expanding. */
