Re: [PATCH v4 09/24] ls-files.c: use index api
- From
Thomas Gummerer <t.gummerer@gmail.com>
- Date
- Nov 30, 2013, 10:30 UTC
- Message-ID
- <87zjomxjul.fsf@gmail.com>
- In-Reply-To
- <CACsJy8D6q5Y4uLTcxD+9pY7U9_2qkO6o-_JeciWWAwJFrsqrzQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
Show 48 quoted lines
> On Wed, Nov 27, 2013 at 7:00 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
>> @@ -447,6 +463,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
>> struct dir_struct dir;
>> struct exclude_list *el;
>> struct string_list exclude_list = STRING_LIST_INIT_NODUP;
>> + struct filter_opts *opts = xmalloc(sizeof(*opts));
>> struct option builtin_ls_files_options[] = {
>> { OPTION_CALLBACK, 'z', NULL, NULL, NULL,
>> N_("paths are separated with NUL character"),
>> @@ -512,9 +529,6 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
>> prefix_len = strlen(prefix);
>> git_config(git_default_config, NULL);
>>
>> - if (read_cache() < 0)
>> - die("index file corrupt");
>> -
>> argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
>> ls_files_usage, 0);
>> el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
>> @@ -550,6 +564,24 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
>> PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
>> prefix, argv);
>>
>> + if (!with_tree && !needs_trailing_slash_stripped()) {
>> + memset(opts, 0, sizeof(*opts));
>> + opts->pathspec = &pathspec;
>> + opts->read_staged = 1;
>> + if (show_resolve_undo)
>> + opts->read_resolve_undo = 1;
>> + if (read_cache_filtered(opts) < 0)
>> + die("index file corrupt");
>> + } else {
>> + if (read_cache() < 0)
>> + die("index file corrupt");
>> + parse_pathspec(&pathspec, 0,
>> + PATHSPEC_PREFER_CWD |
>> + PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
>> + prefix, argv);
>
> So we ran parse_pathspec() once (not shown in the context), if found
> trailing slashes, we read full cache and rerun parse_pathspec()
> because the index is not loaded on the first run.
>
> This is fine. Just a note for future improvement: as _SLASH_CHEAP only
> needs to look at a few entries with cache_name_pos(), we could take
> advantage of v5 to peek individual entries (or in v2, load full cache
> first). Nothing needs to be done now, I think we have not decided
> whether to combine _SLASH_CHEAP and _SLASH_EXPENSIVE into one.Yes that makes sense. Adding the ability to search for path entries without reading the whole or part of the index was something I was thinking about, but didn't have time to do so yet. I'll add this to my list of possible future improvements.
Show 7 quoted lines
>> + } >> + >> /* Find common prefix for all pathspec's */ >> max_prefix = common_prefix(&pathspec); >> max_prefix_len = max_prefix ? strlen(max_prefix) : 0; > -- > Duy