{"thread":{"id":"38800","subject":"Re: [PATCH 01/16] list-files: command skeleton","startedAt":"2015-03-14T11:13:42Z","lastAt":"2015-03-14T11:13:42Z","messageCount":1,"participants":["Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":16},"messages":[{"id":"257671","messageId":"CACsJy8Bj4impHg8qnytgsf9Tt-oHXzxMcgDLX3UcJ7gfb8sdqQ@mail.gmail.com","threadId":"38800","inReplyTo":null,"subject":"Re: [PATCH 01/16] list-files: command skeleton","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-03-14T11:13:42Z","receivedAt":"2015-03-14T11:13:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 13, 2015 at 4:02 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> list-files is supposed to be the user friendly version of ls-files, or\n>> an alternative to git-status. Nothing fancy in this patch yet.\n>\n> The result of applying this patch alone will not give us anything\n> fancy, but the patch itself is interesting ;-)\n\nThat's the point. Fancy stuff comes as separate, bite-size patches.\n\n>> +static void populate_cached_entries(struct string_list *result,\n>> +                                 const struct index_state *istate)\n>> +{\n>> +     int i;\n>> +\n>> +     for (i = 0; i < istate->cache_nr; i++) {\n>> +             const struct cache_entry *ce = istate->cache[i];\n>> +\n>> +             if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),\n>> +                                 0, NULL,\n>> +                                 S_ISDIR(ce->ce_mode) ||\n>> +                                 S_ISGITLINK(ce->ce_mode)))\n>\n> Because we won't tell the user \"You gave me Mkaefile but that did\n> not match\" when \"git list-files Mkaefile\" does not produce anything,\n> we do not need to pass seen[] down from here.\n\nWe probably want that feature back. Not implemented yet though.\n\n>> +     prefix = cmd_prefix;\n>> +     if (prefix)\n>> +             prefix_length = strlen(prefix);\n>> +\n>> +     if (read_cache() < 0)\n>> +             die(_(\"index file corrupt\"));\n>> +\n>> +     git_config(ls_config, NULL);\n>> +\n>> +     argc = parse_options(argc, argv, prefix, ls_options, ls_usage, 0);\n>> +\n>> +     parse_pathspec(&pathspec, 0,\n>> +                    PATHSPEC_PREFER_CWD |\n>> +                    PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,\n>> +                    cmd_prefix, argv);\n>> +     pathspec.max_depth = 0;\n>> +     pathspec.recursive = 1;\n>> +\n>> +     refresh_index(&the_index, REFRESH_QUIET | REFRESH_UNMERGED,\n>> +                   &pathspec, NULL, NULL);\n>\n> It would be better to do read-cache-preload, instead of read-cache,\n> if you are going to immediately refresh.  That is what \"git status\"\n> does.\n\nparse_options(), or *_SLASH_CHEAP to be exact, needs the index being\nloaded (but not necesarily refreshed), so we can't simply move it\ncloser to refresh_index() after we have got the pathspec. And doing\nread-cache-preload() without pathspec where read_cache() is seems a\nwaste of lstat() because the default mode is only look at cwd and\nsubdirs, not full worktree like git-status.\n\nI have a patch (not sent yet) that adds read-cache-preload back. But\nthis 01/16 will likely stay simple and unoptimized.\n-- \nDuy\n"}]}